From 1fe372cdbdf6a6960e46c7555a9698c6b5d7e758 Mon Sep 17 00:00:00 2001 From: Michal Klocek Date: Wed, 7 Oct 2020 13:05:00 +0200 Subject: 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 --- src/core/renderer/content_renderer_client_qt.cpp | 9 ++-- src/core/renderer/content_renderer_client_qt.h | 4 +- src/core/renderer/render_thread_observer_qt.cpp | 4 -- src/core/renderer/user_resource_controller.cpp | 61 +++++++++++++++--------- src/core/renderer/user_resource_controller.h | 6 ++- 5 files changed, 52 insertions(+), 32 deletions(-) (limited to 'src') diff --git a/src/core/renderer/content_renderer_client_qt.cpp b/src/core/renderer/content_renderer_client_qt.cpp index 9f1f5bcf3..a762c6fbb 100644 --- a/src/core/renderer/content_renderer_client_qt.cpp +++ b/src/core/renderer/content_renderer_client_qt.cpp @@ -134,11 +134,12 @@ void ContentRendererClientQt::RenderThreadStarted() { content::RenderThread *renderThread = content::RenderThread::Get(); m_renderThreadObserver.reset(new RenderThreadObserverQt()); + m_userResourceController.reset(new UserResourceController()); m_visitedLinkReader.reset(new visitedlink::VisitedLinkReader); m_webCacheImpl.reset(new web_cache::WebCacheImpl()); renderThread->AddObserver(m_renderThreadObserver.data()); - renderThread->AddObserver(UserResourceController::instance()); + renderThread->AddObserver(m_userResourceController.data()); #if QT_CONFIG(webengine_spellchecker) if (!m_spellCheck) @@ -191,7 +192,7 @@ void ContentRendererClientQt::RenderViewCreated(content::RenderView *render_view { // RenderViewObservers destroy themselves with their RenderView. new RenderViewObserverQt(render_view); - UserResourceController::instance()->renderViewCreated(render_view); + m_userResourceController->renderViewCreated(render_view); } void ContentRendererClientQt::RenderFrameCreated(content::RenderFrame *render_frame) @@ -203,7 +204,7 @@ void ContentRendererClientQt::RenderFrameCreated(content::RenderFrame *render_fr new WebChannelIPCTransport(render_frame); #endif - UserResourceController::instance()->renderFrameCreated(render_frame); + m_userResourceController->renderFrameCreated(render_frame); new QtWebEngineCore::ContentSettingsObserverQt(render_frame); @@ -234,7 +235,7 @@ void ContentRendererClientQt::RunScriptsAtDocumentEnd(content::RenderFrame *rend RenderFrameObserverQt *render_frame_observer = RenderFrameObserverQt::Get(render_frame); if (render_frame_observer && !render_frame_observer->isFrameDetached()) - UserResourceController::instance()->RunScriptsAtDocumentEnd(render_frame); + m_userResourceController->RunScriptsAtDocumentEnd(render_frame); #if BUILDFLAG(ENABLE_EXTENSIONS) ExtensionsRendererClientQt::GetInstance()->RunScriptsAtDocumentEnd(render_frame); diff --git a/src/core/renderer/content_renderer_client_qt.h b/src/core/renderer/content_renderer_client_qt.h index 24a841cb8..3eb6be43a 100644 --- a/src/core/renderer/content_renderer_client_qt.h +++ b/src/core/renderer/content_renderer_client_qt.h @@ -73,10 +73,11 @@ namespace content { struct WebPluginInfo; } +class UserResourceController; + namespace QtWebEngineCore { class RenderThreadObserverQt; - class ContentRendererClientQt : public content::ContentRendererClient , public service_manager::LocalInterfaceProvider @@ -148,6 +149,7 @@ private: const error_page::Error &error, std::string *errorHtml); QScopedPointer m_renderThreadObserver; + QScopedPointer m_userResourceController; QScopedPointer m_visitedLinkReader; QScopedPointer m_webCacheImpl; #if QT_CONFIG(webengine_spellchecker) diff --git a/src/core/renderer/render_thread_observer_qt.cpp b/src/core/renderer/render_thread_observer_qt.cpp index b2ed38f21..edb66e8a5 100644 --- a/src/core/renderer/render_thread_observer_qt.cpp +++ b/src/core/renderer/render_thread_observer_qt.cpp @@ -54,15 +54,11 @@ void RenderThreadObserverQt::RegisterMojoInterfaces(blink::AssociatedInterfaceRe { associated_interfaces->AddInterface( base::Bind(&RenderThreadObserverQt::OnRendererConfigurationAssociatedRequest, base::Unretained(this))); - associated_interfaces->AddInterface( - base::Bind(&UserResourceController::BindReceiver, - base::Unretained(UserResourceController::instance()))); } void RenderThreadObserverQt::UnregisterMojoInterfaces(blink::AssociatedInterfaceRegistry *associated_interfaces) { associated_interfaces->RemoveInterface(qtwebengine::mojom::RendererConfiguration::Name_); - associated_interfaces->RemoveInterface(qtwebengine::mojom::UserResourceController::Name_); } void RenderThreadObserverQt::SetInitialConfiguration(bool is_incognito_process) diff --git a/src/core/renderer/user_resource_controller.cpp b/src/core/renderer/user_resource_controller.cpp index 5f446e3b2..0c2f9b137 100644 --- a/src/core/renderer/user_resource_controller.cpp +++ b/src/core/renderer/user_resource_controller.cpp @@ -65,8 +65,6 @@ #include -Q_GLOBAL_STATIC(UserResourceController, qt_webengine_userResourceController) - static content::RenderView *const globalScriptsIndex = nullptr; // 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 public qtwebengine::mojom::UserResourceControllerRenderFrame { public: - RenderFrameObserverHelper(content::RenderFrame *render_frame); + RenderFrameObserverHelper(content::RenderFrame *render_frame, + UserResourceController *controller); void BindReceiver( mojo::PendingAssociatedReceiver receiver); @@ -161,6 +160,7 @@ private: class Runner; QScopedPointer m_runner; mojo::AssociatedReceiver m_binding; + UserResourceController *m_userResourceController; }; // Helper class to create WeakPtrs so the AfterLoad tasks can be canceled and to @@ -168,13 +168,16 @@ private: class UserResourceController::RenderFrameObserverHelper::Runner : public base::SupportsWeakPtr { public: - explicit Runner(blink::WebLocalFrame *frame) : m_frame(frame) {} + explicit Runner(blink::WebLocalFrame *frame, UserResourceController *controller) + : m_frame(frame), m_userResourceController(controller) + { + } void run(QtWebEngineCore::UserScriptData::InjectionPoint p) { DCHECK_LT(p, m_ran.size()); if (!m_ran[p]) { - UserResourceController::instance()->runScripts(p, m_frame); + m_userResourceController->runScripts(p, m_frame); m_ran[p] = true; } } @@ -182,17 +185,19 @@ public: private: blink::WebLocalFrame *m_frame; std::bitset<3> m_ran; + UserResourceController *m_userResourceController; }; // Used only for script cleanup on RenderView destruction. class UserResourceController::RenderViewObserverHelper : public content::RenderViewObserver { public: - RenderViewObserverHelper(content::RenderView *render_view); + RenderViewObserverHelper(content::RenderView *render_view, UserResourceController *controller); private: // RenderViewObserver implementation. void OnDestruct() override; + UserResourceController *m_userResourceController; }; void UserResourceController::runScripts(QtWebEngineCore::UserScriptData::InjectionPoint p, @@ -230,16 +235,20 @@ void UserResourceController::RunScriptsAtDocumentEnd(content::RenderFrame *rende } UserResourceController::RenderFrameObserverHelper::RenderFrameObserverHelper( - content::RenderFrame *render_frame) - : content::RenderFrameObserver(render_frame), m_binding(this) + content::RenderFrame *render_frame, UserResourceController *controller) + : content::RenderFrameObserver(render_frame) + , m_binding(this) + , m_userResourceController(controller) { render_frame->GetAssociatedInterfaceRegistry()->AddInterface( base::BindRepeating(&UserResourceController::RenderFrameObserverHelper::BindReceiver, base::Unretained(this))); } -UserResourceController::RenderViewObserverHelper::RenderViewObserverHelper(content::RenderView *render_view) - : content::RenderViewObserver(render_view) +UserResourceController::RenderViewObserverHelper::RenderViewObserverHelper( + content::RenderView *render_view, UserResourceController *controller) + : content::RenderViewObserver(render_view), m_userResourceController(controller) + {} void UserResourceController::RenderFrameObserverHelper::BindReceiver( @@ -259,7 +268,7 @@ void UserResourceController::RenderFrameObserverHelper::DidCommitProvisionalLoad // process has been notified of the DidCommitProvisionalLoad event to ensure // that the WebChannelTransportHost is ready to receive messages. - m_runner.reset(new Runner(render_frame()->GetWebFrame())); + m_runner.reset(new Runner(render_frame()->GetWebFrame(), m_userResourceController)); base::ThreadTaskRunnerHandle::Get()->PostTask( FROM_HERE, @@ -302,7 +311,7 @@ void UserResourceController::RenderViewObserverHelper::OnDestruct() { // Remove all scripts associated with the render view. if (content::RenderView *view = render_view()) - UserResourceController::instance()->renderViewDestroyed(view); + m_userResourceController->renderViewDestroyed(view); delete this; } @@ -311,7 +320,7 @@ void UserResourceController::RenderFrameObserverHelper::AddScript( { if (content::RenderFrame *frame = render_frame()) if (content::RenderView *view = frame->GetRenderView()) - UserResourceController::instance()->addScriptForView(script, view); + m_userResourceController->addScriptForView(script, view); } void UserResourceController::RenderFrameObserverHelper::RemoveScript( @@ -319,19 +328,14 @@ void UserResourceController::RenderFrameObserverHelper::RemoveScript( { if (content::RenderFrame *frame = render_frame()) if (content::RenderView *view = frame->GetRenderView()) - UserResourceController::instance()->removeScriptForView(script, view); + m_userResourceController->removeScriptForView(script, view); } void UserResourceController::RenderFrameObserverHelper::ClearScripts() { if (content::RenderFrame *frame = render_frame()) if (content::RenderView *view = frame->GetRenderView()) - UserResourceController::instance()->clearScriptsForView(view); -} - -UserResourceController *UserResourceController::instance() -{ - return qt_webengine_userResourceController(); + m_userResourceController->clearScriptsForView(view); } void UserResourceController::BindReceiver( @@ -352,13 +356,13 @@ UserResourceController::UserResourceController() : m_binding(this) void UserResourceController::renderFrameCreated(content::RenderFrame *renderFrame) { // Will destroy itself when the RenderFrame is destroyed. - new RenderFrameObserverHelper(renderFrame); + new RenderFrameObserverHelper(renderFrame, this); } void UserResourceController::renderViewCreated(content::RenderView *renderView) { // Will destroy itself when the RenderView is destroyed. - new RenderViewObserverHelper(renderView); + new RenderViewObserverHelper(renderView, this); } void UserResourceController::renderViewDestroyed(content::RenderView *renderView) @@ -419,3 +423,16 @@ void UserResourceController::ClearScripts() { clearScriptsForView(globalScriptsIndex); } + +void UserResourceController::RegisterMojoInterfaces( + blink::AssociatedInterfaceRegistry *associated_interfaces) +{ + associated_interfaces->AddInterface( + base::Bind(&UserResourceController::BindReceiver, base::Unretained(this))); +} + +void UserResourceController::UnregisterMojoInterfaces( + blink::AssociatedInterfaceRegistry *associated_interfaces) +{ + associated_interfaces->RemoveInterface(qtwebengine::mojom::UserResourceController::Name_); +} diff --git a/src/core/renderer/user_resource_controller.h b/src/core/renderer/user_resource_controller.h index 9350c9591..cb1a4d38b 100644 --- a/src/core/renderer/user_resource_controller.h +++ b/src/core/renderer/user_resource_controller.h @@ -62,7 +62,6 @@ class UserResourceController : public content::RenderThreadObserver, { public: - static UserResourceController *instance(); UserResourceController(); void renderFrameCreated(content::RenderFrame *); void renderViewCreated(content::RenderView *); @@ -78,6 +77,11 @@ public: private: Q_DISABLE_COPY(UserResourceController) + // content::RenderThreadObserver: + void RegisterMojoInterfaces(blink::AssociatedInterfaceRegistry *associated_interfaces) override; + void + UnregisterMojoInterfaces(blink::AssociatedInterfaceRegistry *associated_interfaces) override; + class RenderFrameObserverHelper; class RenderViewObserverHelper; -- cgit v1.2.3