Commit 510c597b authored by Ken Rockot's avatar Ken Rockot Committed by Commit Bot

Remove use of blink::InterfaceRegistry from RenderThreadImpl

This is unnecessary now that Blink code can freely use
service_manager client library APIs.

It's also incorrect since content::BlinkInterfaceRegistryImpl
uses an internal WeakPtr to the underlying BinderRegistry on
the main thread, while the BinderRegistry actually lives on the
IO thread.

This does not create any problematic races in practice since
usage on the main thread is guaranteed to preceed movement to
the IO thread, but it does violate the usage constraints of
WeakPtrFactory and triggers DCHECKs.

BUG=789064

Change-Id: Id491e7f126c861220d4b2c3b85ad25fd6384a1f5
Reviewed-on: https://chromium-review.googlesource.com/807265Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatarJohn Abd-El-Malek <jam@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521714}
parent 1d480b2a
......@@ -37,8 +37,8 @@
#include "content/test/mock_render_process.h"
#include "content/test/test_content_client.h"
#include "content/test/test_render_frame.h"
#include "services/service_manager/public/cpp/binder_registry.h"
#include "services/service_manager/public/cpp/connector.h"
#include "third_party/WebKit/public/platform/InterfaceRegistry.h"
#include "third_party/WebKit/public/platform/WebGestureEvent.h"
#include "third_party/WebKit/public/platform/WebInputEvent.h"
#include "third_party/WebKit/public/platform/WebMouseEvent.h"
......@@ -247,8 +247,8 @@ void RenderViewTest::SetUp() {
// Blink needs to be initialized before calling CreateContentRendererClient()
// because it uses blink internally.
blink_platform_impl_.Initialize();
blink::Initialize(blink_platform_impl_.Get(),
blink::InterfaceRegistry::GetEmptyInterfaceRegistry());
service_manager::BinderRegistry empty_registry;
blink::Initialize(blink_platform_impl_.Get(), &empty_registry);
content_client_.reset(CreateContentClient());
content_browser_client_.reset(CreateContentBrowserClient());
......
......@@ -115,7 +115,6 @@
#include "content/renderer/media/midi_message_filter.h"
#include "content/renderer/media/render_media_client.h"
#include "content/renderer/media/video_capture_impl_manager.h"
#include "content/renderer/mojo/blink_interface_registry_impl.h"
#include "content/renderer/mus/render_widget_window_tree_client_factory.h"
#include "content/renderer/mus/renderer_window_tree_client.h"
#include "content/renderer/net_info_helper.h"
......@@ -157,7 +156,6 @@
#include "net/base/url_util.h"
#include "ppapi/features/features.h"
#include "services/metrics/public/cpp/mojo_ukm_recorder.h"
#include "services/service_manager/public/cpp/binder_registry.h"
#include "services/service_manager/public/cpp/connector.h"
#include "services/service_manager/public/cpp/interface_provider.h"
#include "services/ui/public/cpp/gpu/context_provider_command_buffer.h"
......@@ -720,10 +718,7 @@ void RenderThreadImpl::Init(
quota_dispatcher_.reset(new QuotaDispatcher(message_loop()->task_runner()));
auto registry = std::make_unique<service_manager::BinderRegistry>();
BlinkInterfaceRegistryImpl interface_registry(
registry->GetWeakPtr(), associated_interfaces_.GetWeakPtr());
InitializeWebKit(resource_task_queue, &interface_registry);
InitializeWebKit(resource_task_queue, registry.get());
blink_initialized_time_ = base::TimeTicks::Now();
// In single process the single process is all there is.
......@@ -1227,7 +1222,7 @@ void RenderThreadImpl::InitializeCompositorThread() {
void RenderThreadImpl::InitializeWebKit(
const scoped_refptr<base::SingleThreadTaskRunner>& resource_task_queue,
blink::InterfaceRegistry* interface_registry) {
service_manager::BinderRegistry* registry) {
DCHECK(!blink_platform_impl_);
const base::CommandLine& command_line =
......@@ -1244,7 +1239,7 @@ void RenderThreadImpl::InitializeWebKit(
GetContentClient()
->renderer()
->SetRuntimeFeaturesDefaultsBeforeBlinkInitialization();
blink::Initialize(blink_platform_impl_.get(), interface_registry);
blink::Initialize(blink_platform_impl_.get(), registry);
v8::Isolate* isolate = blink::MainThreadIsolate();
isolate->SetCreateHistogramFunction(CreateHistogram);
......
......@@ -54,6 +54,7 @@
#include "net/base/network_change_notifier.h"
#include "net/nqe/effective_connection_type.h"
#include "services/service_manager/public/cpp/bind_source_info.h"
#include "services/service_manager/public/cpp/binder_registry.h"
#include "services/viz/public/interfaces/compositing/compositing_mode_watcher.mojom.h"
#include "third_party/WebKit/public/platform/WebConnectionType.h"
#include "third_party/WebKit/public/platform/scheduler/renderer/renderer_scheduler.h"
......@@ -70,7 +71,6 @@ namespace blink {
namespace scheduler {
class WebThreadBase;
}
class InterfaceRegistry;
class WebMediaStreamCenter;
class WebMediaStreamCenterClient;
}
......@@ -582,7 +582,7 @@ class CONTENT_EXPORT RenderThreadImpl
void InitializeWebKit(
const scoped_refptr<base::SingleThreadTaskRunner>& resource_task_queue,
blink::InterfaceRegistry* registry);
service_manager::BinderRegistry* registry);
void OnTransferBitmap(const SkBitmap& bitmap, int resource_id);
void OnGetAccessibilityTree();
......
......@@ -22,6 +22,7 @@ include_rules = [
"+services/catalog",
"+services/device/public/cpp/generic_sensor",
"+services/proxy_resolver",
"+services/service_manager/public",
"+ui/base/resource/data_pack.h",
"+ui/base/resource/resource_bundle.h",
"+ui/base/resource/resource_bundle_android.h",
......
......@@ -27,7 +27,7 @@
#include "media/base/media.h"
#include "media/media_features.h"
#include "net/cookies/cookie_monster.h"
#include "third_party/WebKit/public/platform/InterfaceRegistry.h"
#include "services/service_manager/public/cpp/binder_registry.h"
#include "third_party/WebKit/public/platform/WebConnectionType.h"
#include "third_party/WebKit/public/platform/WebData.h"
#include "third_party/WebKit/public/platform/WebNetworkStateNotifier.h"
......@@ -163,8 +163,8 @@ TestBlinkWebUnitTestSupport::TestBlinkWebUnitTestSupport()
// Initialize mojo firstly to enable Blink initialization to use it.
InitializeMojo();
blink::Initialize(this,
blink::InterfaceRegistry::GetEmptyInterfaceRegistry());
service_manager::BinderRegistry empty_registry;
blink::Initialize(this, &empty_registry);
blink::SetLayoutTestMode(true);
blink::WebRuntimeFeatures::EnableDatabase(true);
blink::WebRuntimeFeatures::EnableNotifications(true);
......
......@@ -10,8 +10,8 @@
#include "base/test/test_suite.h"
#include "build/build_config.h"
#include "media/base/media.h"
#include "services/service_manager/public/cpp/binder_registry.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/WebKit/public/platform/InterfaceRegistry.h"
#include "third_party/WebKit/public/platform/WebThread.h"
#include "third_party/WebKit/public/platform/scheduler/renderer/renderer_scheduler.h"
#include "third_party/WebKit/public/platform/scheduler/test/renderer_scheduler_test_support.h"
......@@ -98,8 +98,8 @@ void BlinkMediaTestSuite::Initialize() {
std::unique_ptr<base::MessageLoop> message_loop;
if (!base::MessageLoop::current())
message_loop.reset(new base::MessageLoop());
blink::Initialize(blink_platform_support_.get(),
blink::InterfaceRegistry::GetEmptyInterfaceRegistry());
service_manager::BinderRegistry empty_registry;
blink::Initialize(blink_platform_support_.get(), &empty_registry);
}
int main(int argc, char** argv) {
......
......@@ -11,8 +11,8 @@
#include "gin/array_buffer.h"
#include "gin/public/isolate_holder.h"
#include "services/data_decoder/image_decoder_impl.h"
#include "services/service_manager/public/cpp/binder_registry.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/WebKit/public/platform/InterfaceRegistry.h"
#include "third_party/WebKit/public/platform/scheduler/child/webthread_base.h"
#include "third_party/WebKit/public/web/WebKit.h"
#include "third_party/skia/include/core/SkBitmap.h"
......@@ -76,8 +76,8 @@ class BlinkInitializer : public blink::Platform {
gin::V8Initializer::LoadV8Natives();
#endif
blink::Initialize(this,
blink::InterfaceRegistry::GetEmptyInterfaceRegistry());
service_manager::BinderRegistry empty_registry;
blink::Initialize(this, &empty_registry);
}
~BlinkInitializer() override {}
......
......@@ -42,6 +42,7 @@
#include "platform/bindings/V8PerIsolateData.h"
#include "platform/heap/Heap.h"
#include "platform/wtf/Assertions.h"
#include "platform/wtf/Functional.h"
#include "platform/wtf/PtrUtil.h"
#include "platform/wtf/WTF.h"
#include "public/platform/InterfaceRegistry.h"
......@@ -73,7 +74,7 @@ static BlinkInitializer& GetBlinkInitializer() {
return *initializer;
}
void Initialize(Platform* platform, InterfaceRegistry* registry) {
void Initialize(Platform* platform, service_manager::BinderRegistry* registry) {
DCHECK(registry);
Platform::Initialize(platform);
......@@ -115,7 +116,8 @@ void Initialize(Platform* platform, InterfaceRegistry* registry) {
}
}
void BlinkInitializer::RegisterInterfaces(InterfaceRegistry& registry) {
void BlinkInitializer::RegisterInterfaces(
service_manager::BinderRegistry& registry) {
ModulesInitializer::RegisterInterfaces(registry);
WebThread* main_thread = Platform::Current()->MainThread();
// GetSingleThreadTaskRunner() uses GetWebTaskRunner() internally.
......@@ -123,8 +125,9 @@ void BlinkInitializer::RegisterInterfaces(InterfaceRegistry& registry) {
if (!main_thread || !main_thread->GetWebTaskRunner())
return;
registry.AddInterface(CrossThreadBind(&OomInterventionImpl::Create),
main_thread->GetSingleThreadTaskRunner());
registry.AddInterface(
ConvertToBaseCallback(CrossThreadBind(&OomInterventionImpl::Create)),
main_thread->GetSingleThreadTaskRunner());
}
void BlinkInitializer::InitLocalFrame(LocalFrame& frame) const {
......
......@@ -11,7 +11,7 @@ namespace blink {
class BlinkInitializer : public ModulesInitializer {
public:
void RegisterInterfaces(InterfaceRegistry&) override;
void RegisterInterfaces(service_manager::BinderRegistry&) override;
void OnClearWindowObjectInMainWorld(Document&,
const Settings&) const override;
void InitLocalFrame(LocalFrame&) const override;
......
......@@ -34,6 +34,7 @@
#include "base/macros.h"
#include "core/CoreExport.h"
#include "platform/wtf/Allocator.h"
#include "services/service_manager/public/cpp/binder_registry.h"
namespace blink {
......@@ -42,7 +43,6 @@ class HTMLMediaElement;
class InspectedFrames;
class InspectorDOMAgent;
class InspectorSession;
class InterfaceRegistry;
class LocalFrame;
class MediaControls;
class Page;
......@@ -76,7 +76,7 @@ class CORE_EXPORT CoreInitializer {
// Called on startup to register Mojo interfaces that for control messages,
// e.g. messages that are not routed to a specific frame.
virtual void RegisterInterfaces(InterfaceRegistry&) = 0;
virtual void RegisterInterfaces(service_manager::BinderRegistry&) = 0;
// Methods defined in CoreInitializer and implemented by ModulesInitializer to
// bypass the inverted dependency from core/ to modules/.
// Mojo Interfaces registered with LocalFrame
......
......@@ -20,7 +20,7 @@ include_rules = [
"+services/metrics/public",
"+services/network/public/interfaces",
"+services/resource_coordinator/public/cpp/resource_coordinator_features.h",
"+services/service_manager/public/interfaces/interface_provider.mojom-blink.h",
"+services/service_manager/public",
"+third_party/skia/include",
"+ui/gfx/geometry",
"-web",
......
......@@ -83,6 +83,7 @@
#include "modules/webgl/WebGLRenderingContext.h"
#include "platform/CrossThreadFunctional.h"
#include "platform/mojo/MojoHelper.h"
#include "platform/wtf/Functional.h"
#include "platform/wtf/PtrUtil.h"
#include "public/platform/InterfaceRegistry.h"
#include "public/platform/WebSecurityOrigin.h"
......@@ -276,10 +277,12 @@ void ModulesInitializer::CollectAllGarbageForAnimationWorklet() const {
AnimationWorkletThread::CollectAllGarbage();
}
void ModulesInitializer::RegisterInterfaces(InterfaceRegistry& registry) {
void ModulesInitializer::RegisterInterfaces(
service_manager::BinderRegistry& registry) {
DCHECK(Platform::Current());
registry.AddInterface(blink::CrossThreadBind(&WebDatabaseImpl::Create),
Platform::Current()->GetIOTaskRunner());
registry.AddInterface(
ConvertToBaseCallback(blink::CrossThreadBind(&WebDatabaseImpl::Create)),
Platform::Current()->GetIOTaskRunner());
}
} // namespace blink
......@@ -13,7 +13,7 @@ namespace blink {
class MODULES_EXPORT ModulesInitializer : public CoreInitializer {
public:
void Initialize() override;
void RegisterInterfaces(InterfaceRegistry&) override;
void RegisterInterfaces(service_manager::BinderRegistry&) override;
protected:
void InitLocalFrame(LocalFrame&) const override;
......
......@@ -79,6 +79,7 @@ _CONFIG = [
# cetera, as well as generated Mojo bindings.
'mojo::.+',
'(?:.+::)?mojom::.+',
"service_manager::BinderRegistry",
# TODO(dcheng): Remove this once Connector isn't needed in Blink
# anymore.
'service_manager::Connector',
......
......@@ -640,6 +640,7 @@ source_set("blink_headers") {
public_deps = [
"//net",
"//services/service_manager/public/cpp",
"//skia",
"//third_party/WebKit/common:blink_common",
"//url",
......
......@@ -9,7 +9,7 @@ include_rules = [
# Enforce to use mojom-shared.h in WebKit/public so that it can compile
# inside and outside Blink.
"+services/network/public/interfaces/cors.mojom-shared.h",
"+services/service_manager/public/cpp/interface_provider.h",
"+services/service_manager/public",
# Allowed only inside INSIDE_BLINK
"+core",
......
......@@ -32,19 +32,18 @@
#define WebKit_h
#include "public/platform/Platform.h"
#include "services/service_manager/public/cpp/binder_registry.h"
#include "v8/include/v8.h"
namespace blink {
class InterfaceRegistry;
// Initialize the entire Blink (wtf, platform, core, modules and web).
// If you just need wtf and platform, use Platform::initialize instead.
//
// Must be called on the thread that will be the main thread before
// using any other public APIs. The provided Platform; must be
// non-null and must remain valid until the current thread calls shutdown.
BLINK_EXPORT void Initialize(Platform*, InterfaceRegistry*);
BLINK_EXPORT void Initialize(Platform*, service_manager::BinderRegistry*);
// Get the V8 Isolate for the main thread.
// initialize must have been called first.
......
......@@ -89,6 +89,7 @@ if (use_v8_context_snapshot) {
deps = [
"//gin:gin",
"//mojo/edk/system:system",
"//services/service_manager/public/cpp",
"//third_party/WebKit/public:blink",
"//v8",
]
......
......@@ -3,5 +3,6 @@ include_rules = [
"+v8",
"+third_party/WebKit/public",
"+gin/v8_initializer.h",
"+mojo/edk/embedder/embedder.h",
"+mojo/edk/embedder",
"+services/service_manager/public",
]
......@@ -9,7 +9,7 @@
#include "base/task_scheduler/task_scheduler.h"
#include "gin/v8_initializer.h"
#include "mojo/edk/embedder/embedder.h"
#include "third_party/WebKit/public/platform/InterfaceRegistry.h"
#include "services/service_manager/public/cpp/binder_registry.h"
#include "third_party/WebKit/public/platform/WebThread.h"
#include "third_party/WebKit/public/web/WebKit.h"
#include "third_party/WebKit/public/web/WebV8ContextSnapshot.h"
......@@ -55,8 +55,8 @@ int main(int argc, char** argv) {
// Take a snapshot.
SnapshotPlatform platform;
blink::Initialize(&platform,
blink::InterfaceRegistry::GetEmptyInterfaceRegistry());
service_manager::BinderRegistry empty_registry;
blink::Initialize(&platform, &empty_registry);
v8::StartupData blob = blink::WebV8ContextSnapshot::TakeSnapshot();
// Save the snapshot as a file. Filename is given in a command line option.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment