Skip to content

Commit 97b322d

Browse files
Michal Klocekpatricia-gallardo
authored andcommitted
Fix crashes in user resource controller when single process
Mojo interface when running in single process was not correctly destructed, since we used user resource controller as global static object. Move user resource controller to content render client. Change-Id: I219510c9bc382545174aa5aae99ac8282a2049e6 Reviewed-by: Allan Sandfeld Jensen <[email protected]>
1 parent da55882 commit 97b322d

File tree

5 files changed

+52
-32
lines changed

5 files changed

+52
-32
lines changed

src/core/renderer/content_renderer_client_qt.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -134,11 +134,12 @@ void ContentRendererClientQt::RenderThreadStarted()
134134
{
135135
content::RenderThread *renderThread = content::RenderThread::Get();
136136
m_renderThreadObserver.reset(new RenderThreadObserverQt());
137+
m_userResourceController.reset(new UserResourceController());
137138
m_visitedLinkReader.reset(new visitedlink::VisitedLinkReader);
138139
m_webCacheImpl.reset(new web_cache::WebCacheImpl());
139140

140141
renderThread->AddObserver(m_renderThreadObserver.data());
141-
renderThread->AddObserver(UserResourceController::instance());
142+
renderThread->AddObserver(m_userResourceController.data());
142143

143144
#if QT_CONFIG(webengine_spellchecker)
144145
if (!m_spellCheck)
@@ -191,7 +192,7 @@ void ContentRendererClientQt::RenderViewCreated(content::RenderView *render_view
191192
{
192193
// RenderViewObservers destroy themselves with their RenderView.
193194
new RenderViewObserverQt(render_view);
194-
UserResourceController::instance()->renderViewCreated(render_view);
195+
m_userResourceController->renderViewCreated(render_view);
195196
}
196197

197198
void ContentRendererClientQt::RenderFrameCreated(content::RenderFrame *render_frame)
@@ -203,7 +204,7 @@ void ContentRendererClientQt::RenderFrameCreated(content::RenderFrame *render_fr
203204
new WebChannelIPCTransport(render_frame);
204205
#endif
205206

206-
UserResourceController::instance()->renderFrameCreated(render_frame);
207+
m_userResourceController->renderFrameCreated(render_frame);
207208

208209
new QtWebEngineCore::ContentSettingsObserverQt(render_frame);
209210

@@ -234,7 +235,7 @@ void ContentRendererClientQt::RunScriptsAtDocumentEnd(content::RenderFrame *rend
234235
RenderFrameObserverQt *render_frame_observer = RenderFrameObserverQt::Get(render_frame);
235236

236237
if (render_frame_observer && !render_frame_observer->isFrameDetached())
237-
UserResourceController::instance()->RunScriptsAtDocumentEnd(render_frame);
238+
m_userResourceController->RunScriptsAtDocumentEnd(render_frame);
238239

239240
#if BUILDFLAG(ENABLE_EXTENSIONS)
240241
ExtensionsRendererClientQt::GetInstance()->RunScriptsAtDocumentEnd(render_frame);

src/core/renderer/content_renderer_client_qt.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,10 +73,11 @@ namespace content {
7373
struct WebPluginInfo;
7474
}
7575

76+
class UserResourceController;
77+
7678
namespace QtWebEngineCore {
7779

7880
class RenderThreadObserverQt;
79-
8081
class ContentRendererClientQt
8182
: public content::ContentRendererClient
8283
, public service_manager::LocalInterfaceProvider
@@ -148,6 +149,7 @@ class ContentRendererClientQt
148149
const error_page::Error &error, std::string *errorHtml);
149150

150151
QScopedPointer<RenderThreadObserverQt> m_renderThreadObserver;
152+
QScopedPointer<UserResourceController> m_userResourceController;
151153
QScopedPointer<visitedlink::VisitedLinkReader> m_visitedLinkReader;
152154
QScopedPointer<web_cache::WebCacheImpl> m_webCacheImpl;
153155
#if QT_CONFIG(webengine_spellchecker)

src/core/renderer/render_thread_observer_qt.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,15 +54,11 @@ void RenderThreadObserverQt::RegisterMojoInterfaces(blink::AssociatedInterfaceRe
5454
{
5555
associated_interfaces->AddInterface(
5656
base::Bind(&RenderThreadObserverQt::OnRendererConfigurationAssociatedRequest, base::Unretained(this)));
57-
associated_interfaces->AddInterface(
58-
base::Bind(&UserResourceController::BindReceiver,
59-
base::Unretained(UserResourceController::instance())));
6057
}
6158

6259
void RenderThreadObserverQt::UnregisterMojoInterfaces(blink::AssociatedInterfaceRegistry *associated_interfaces)
6360
{
6461
associated_interfaces->RemoveInterface(qtwebengine::mojom::RendererConfiguration::Name_);
65-
associated_interfaces->RemoveInterface(qtwebengine::mojom::UserResourceController::Name_);
6662
}
6763

6864
void RenderThreadObserverQt::SetInitialConfiguration(bool is_incognito_process)

src/core/renderer/user_resource_controller.cpp

Lines changed: 39 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,6 @@
6565

6666
#include <bitset>
6767

68-
Q_GLOBAL_STATIC(UserResourceController, qt_webengine_userResourceController)
69-
7068
static content::RenderView *const globalScriptsIndex = nullptr;
7169

7270
// Scripts meant to run after the load event will be run 500ms after DOMContentLoaded if the load event doesn't come within that delay.
@@ -142,7 +140,8 @@ class UserResourceController::RenderFrameObserverHelper
142140
public qtwebengine::mojom::UserResourceControllerRenderFrame
143141
{
144142
public:
145-
RenderFrameObserverHelper(content::RenderFrame *render_frame);
143+
RenderFrameObserverHelper(content::RenderFrame *render_frame,
144+
UserResourceController *controller);
146145
void BindReceiver(
147146
mojo::PendingAssociatedReceiver<qtwebengine::mojom::UserResourceControllerRenderFrame>
148147
receiver);
@@ -161,38 +160,44 @@ class UserResourceController::RenderFrameObserverHelper
161160
class Runner;
162161
QScopedPointer<Runner> m_runner;
163162
mojo::AssociatedReceiver<qtwebengine::mojom::UserResourceControllerRenderFrame> m_binding;
163+
UserResourceController *m_userResourceController;
164164
};
165165

166166
// Helper class to create WeakPtrs so the AfterLoad tasks can be canceled and to
167167
// avoid running scripts more than once per injection point.
168168
class UserResourceController::RenderFrameObserverHelper::Runner : public base::SupportsWeakPtr<Runner>
169169
{
170170
public:
171-
explicit Runner(blink::WebLocalFrame *frame) : m_frame(frame) {}
171+
explicit Runner(blink::WebLocalFrame *frame, UserResourceController *controller)
172+
: m_frame(frame), m_userResourceController(controller)
173+
{
174+
}
172175

173176
void run(QtWebEngineCore::UserScriptData::InjectionPoint p)
174177
{
175178
DCHECK_LT(p, m_ran.size());
176179
if (!m_ran[p]) {
177-
UserResourceController::instance()->runScripts(p, m_frame);
180+
m_userResourceController->runScripts(p, m_frame);
178181
m_ran[p] = true;
179182
}
180183
}
181184

182185
private:
183186
blink::WebLocalFrame *m_frame;
184187
std::bitset<3> m_ran;
188+
UserResourceController *m_userResourceController;
185189
};
186190

187191
// Used only for script cleanup on RenderView destruction.
188192
class UserResourceController::RenderViewObserverHelper : public content::RenderViewObserver
189193
{
190194
public:
191-
RenderViewObserverHelper(content::RenderView *render_view);
195+
RenderViewObserverHelper(content::RenderView *render_view, UserResourceController *controller);
192196

193197
private:
194198
// RenderViewObserver implementation.
195199
void OnDestruct() override;
200+
UserResourceController *m_userResourceController;
196201
};
197202

198203
void UserResourceController::runScripts(QtWebEngineCore::UserScriptData::InjectionPoint p,
@@ -230,16 +235,20 @@ void UserResourceController::RunScriptsAtDocumentEnd(content::RenderFrame *rende
230235
}
231236

232237
UserResourceController::RenderFrameObserverHelper::RenderFrameObserverHelper(
233-
content::RenderFrame *render_frame)
234-
: content::RenderFrameObserver(render_frame), m_binding(this)
238+
content::RenderFrame *render_frame, UserResourceController *controller)
239+
: content::RenderFrameObserver(render_frame)
240+
, m_binding(this)
241+
, m_userResourceController(controller)
235242
{
236243
render_frame->GetAssociatedInterfaceRegistry()->AddInterface(
237244
base::BindRepeating(&UserResourceController::RenderFrameObserverHelper::BindReceiver,
238245
base::Unretained(this)));
239246
}
240247

241-
UserResourceController::RenderViewObserverHelper::RenderViewObserverHelper(content::RenderView *render_view)
242-
: content::RenderViewObserver(render_view)
248+
UserResourceController::RenderViewObserverHelper::RenderViewObserverHelper(
249+
content::RenderView *render_view, UserResourceController *controller)
250+
: content::RenderViewObserver(render_view), m_userResourceController(controller)
251+
243252
{}
244253

245254
void UserResourceController::RenderFrameObserverHelper::BindReceiver(
@@ -259,7 +268,7 @@ void UserResourceController::RenderFrameObserverHelper::DidCommitProvisionalLoad
259268
// process has been notified of the DidCommitProvisionalLoad event to ensure
260269
// that the WebChannelTransportHost is ready to receive messages.
261270

262-
m_runner.reset(new Runner(render_frame()->GetWebFrame()));
271+
m_runner.reset(new Runner(render_frame()->GetWebFrame(), m_userResourceController));
263272

264273
base::ThreadTaskRunnerHandle::Get()->PostTask(
265274
FROM_HERE,
@@ -302,7 +311,7 @@ void UserResourceController::RenderViewObserverHelper::OnDestruct()
302311
{
303312
// Remove all scripts associated with the render view.
304313
if (content::RenderView *view = render_view())
305-
UserResourceController::instance()->renderViewDestroyed(view);
314+
m_userResourceController->renderViewDestroyed(view);
306315
delete this;
307316
}
308317

@@ -311,27 +320,22 @@ void UserResourceController::RenderFrameObserverHelper::AddScript(
311320
{
312321
if (content::RenderFrame *frame = render_frame())
313322
if (content::RenderView *view = frame->GetRenderView())
314-
UserResourceController::instance()->addScriptForView(script, view);
323+
m_userResourceController->addScriptForView(script, view);
315324
}
316325

317326
void UserResourceController::RenderFrameObserverHelper::RemoveScript(
318327
const QtWebEngineCore::UserScriptData &script)
319328
{
320329
if (content::RenderFrame *frame = render_frame())
321330
if (content::RenderView *view = frame->GetRenderView())
322-
UserResourceController::instance()->removeScriptForView(script, view);
331+
m_userResourceController->removeScriptForView(script, view);
323332
}
324333

325334
void UserResourceController::RenderFrameObserverHelper::ClearScripts()
326335
{
327336
if (content::RenderFrame *frame = render_frame())
328337
if (content::RenderView *view = frame->GetRenderView())
329-
UserResourceController::instance()->clearScriptsForView(view);
330-
}
331-
332-
UserResourceController *UserResourceController::instance()
333-
{
334-
return qt_webengine_userResourceController();
338+
m_userResourceController->clearScriptsForView(view);
335339
}
336340

337341
void UserResourceController::BindReceiver(
@@ -352,13 +356,13 @@ UserResourceController::UserResourceController() : m_binding(this)
352356
void UserResourceController::renderFrameCreated(content::RenderFrame *renderFrame)
353357
{
354358
// Will destroy itself when the RenderFrame is destroyed.
355-
new RenderFrameObserverHelper(renderFrame);
359+
new RenderFrameObserverHelper(renderFrame, this);
356360
}
357361

358362
void UserResourceController::renderViewCreated(content::RenderView *renderView)
359363
{
360364
// Will destroy itself when the RenderView is destroyed.
361-
new RenderViewObserverHelper(renderView);
365+
new RenderViewObserverHelper(renderView, this);
362366
}
363367

364368
void UserResourceController::renderViewDestroyed(content::RenderView *renderView)
@@ -419,3 +423,16 @@ void UserResourceController::ClearScripts()
419423
{
420424
clearScriptsForView(globalScriptsIndex);
421425
}
426+
427+
void UserResourceController::RegisterMojoInterfaces(
428+
blink::AssociatedInterfaceRegistry *associated_interfaces)
429+
{
430+
associated_interfaces->AddInterface(
431+
base::Bind(&UserResourceController::BindReceiver, base::Unretained(this)));
432+
}
433+
434+
void UserResourceController::UnregisterMojoInterfaces(
435+
blink::AssociatedInterfaceRegistry *associated_interfaces)
436+
{
437+
associated_interfaces->RemoveInterface(qtwebengine::mojom::UserResourceController::Name_);
438+
}

src/core/renderer/user_resource_controller.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,6 @@ class UserResourceController : public content::RenderThreadObserver,
6262
{
6363

6464
public:
65-
static UserResourceController *instance();
6665
UserResourceController();
6766
void renderFrameCreated(content::RenderFrame *);
6867
void renderViewCreated(content::RenderView *);
@@ -78,6 +77,11 @@ class UserResourceController : public content::RenderThreadObserver,
7877
private:
7978
Q_DISABLE_COPY(UserResourceController)
8079

80+
// content::RenderThreadObserver:
81+
void RegisterMojoInterfaces(blink::AssociatedInterfaceRegistry *associated_interfaces) override;
82+
void
83+
UnregisterMojoInterfaces(blink::AssociatedInterfaceRegistry *associated_interfaces) override;
84+
8185
class RenderFrameObserverHelper;
8286
class RenderViewObserverHelper;
8387

0 commit comments

Comments
 (0)