Move ownership of MojoFrameSinkManager out of FrameSinkManagerHost.
FrameSinkManagerHost is meant to be held in the Browser process to
give friendly C++ access over mojo to the FrameSinkManager
implementation (which is MojoFrameSinkManager).
Currently the FrameSinkManager is in the Browser too, but we want to
be able to move it, at which point FrameSinkManagerHost can no longer
own it. So we move it out to GpuProcessTransportFactory which will
be able to optionally own it when the display compositor is in the
browser process.
Since the ImageTransportFactory now needs a MessageLoop, we must
create one in RenderViewHostTestEnabler, but that collides with the
TestBrowserThreadBundle in many tests. So, we make it only create a
MessageLoop if one doesn't already exist, and we adjust construction
ordering in tests so that TestBrowserThreadBundle is always created
before RenderViewHostTestEnabler.
Since TestBrowserThreadBundle is created earlier, it also gets
destroyed later. This exposed a flaw in MockRenderProcessHost which
can double-free itself if its posted DeleteSoon is allowed to run,
since RenderViewHostTestEnabler deletes the object directly as well.
So we ensure the posted delete is cancelled if another delete happens
in the meantime.
ExtensionsTest sets up a BrowserClient and must keep it alive until
the MessageLoop/TestBrowserThreadBundle is destroyed in case any tasks
posted would try to use the BrowserClient.
extensions::TestExtensionEnvironment also provides a MessageLoop,
similar to TestBrowserThreadBundle, so must be constructed before
RenderViewHostTestEnabler. We pass it to the constructor for tests
that want to use both together, as we did for the
TestBrowserThreadBundle.
R=fsamuel@chromium.org
Bug: 730213
Change-Id: I6cb0670efd05224fe3a3d98939bcaa32a56cc9ae
Reviewed-on: https://chromium-review.googlesource.com/527675
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#479096}diff --git a/chrome/browser/extensions/extension_service_test_base.cc b/chrome/browser/extensions/extension_service_test_base.cc
index b50b7ee..2e3b693 100644
--- a/chrome/browser/extensions/extension_service_test_base.cc
+++ b/chrome/browser/extensions/extension_service_test_base.cc
@@ -80,10 +80,10 @@
default;
ExtensionServiceTestBase::ExtensionServiceTestBase()
- : thread_bundle_(new content::TestBrowserThreadBundle(kThreadOptions)),
- service_(NULL),
+ : thread_bundle_(kThreadOptions),
+ service_(nullptr),
testing_local_state_(TestingBrowserProcess::GetGlobal()),
- registry_(NULL) {
+ registry_(nullptr) {
base::FilePath test_data_dir;
if (!PathService::Get(chrome::DIR_TEST_DATA, &test_data_dir)) {
ADD_FAILURE();
diff --git a/chrome/browser/extensions/extension_service_test_base.h b/chrome/browser/extensions/extension_service_test_base.h
index 8ec7c2b..0e4179c 100644
--- a/chrome/browser/extensions/extension_service_test_base.h
+++ b/chrome/browser/extensions/extension_service_test_base.h
@@ -140,11 +140,13 @@
// after thread_bundle_ in the destruction order.
base::ShadowingAtExitManager at_exit_manager_;
+ // The MessageLoop is used by RenderViewHostTestEnabler, so this must be
+ // created before it.
+ content::TestBrowserThreadBundle thread_bundle_;
+
// Enable creation of WebContents without initializing a renderer.
content::RenderViewHostTestEnabler rvh_test_enabler_;
- std::unique_ptr<content::TestBrowserThreadBundle> thread_bundle_;
-
protected:
// It's unfortunate that these are exposed to subclasses (rather than used
// through the accessor methods above), but too many tests already use them
diff --git a/chrome/browser/renderer_context_menu/render_view_context_menu_unittest.cc b/chrome/browser/renderer_context_menu/render_view_context_menu_unittest.cc
index 57cf157e..5f9fdf29 100644
--- a/chrome/browser/renderer_context_menu/render_view_context_menu_unittest.cc
+++ b/chrome/browser/renderer_context_menu/render_view_context_menu_unittest.cc
@@ -101,6 +101,12 @@
class RenderViewContextMenuTest : public testing::Test {
protected:
RenderViewContextMenuTest() = default;
+ // If the test uses a TestExtensionEnvironment, which provides a MessageLoop,
+ // it needs to be passed to the constructor so that it exists before the
+ // RenderViewHostTestEnabler which needs to use the MessageLoop.
+ explicit RenderViewContextMenuTest(
+ std::unique_ptr<extensions::TestExtensionEnvironment> env)
+ : environment_(std::move(env)) {}
// Proxy defined here to minimize friend classes in RenderViewContextMenu
static bool ExtensionContextAndPatternMatch(
@@ -124,6 +130,9 @@
type, contexts);
}
+ protected:
+ std::unique_ptr<extensions::TestExtensionEnvironment> environment_;
+
private:
content::RenderViewHostTestEnabler rvh_test_enabler_;
@@ -301,7 +310,9 @@
class RenderViewContextMenuExtensionsTest : public RenderViewContextMenuTest {
protected:
- RenderViewContextMenuExtensionsTest() = default;
+ RenderViewContextMenuExtensionsTest()
+ : RenderViewContextMenuTest(
+ base::MakeUnique<extensions::TestExtensionEnvironment>()) {}
void SetUp() override {
RenderViewContextMenuTest::SetUp();
@@ -314,14 +325,11 @@
RenderViewContextMenuTest::TearDown();
}
- TestingProfile* profile() const { return environment_.profile(); }
+ TestingProfile* profile() const { return environment_->profile(); }
- extensions::TestExtensionEnvironment& environment() {
- return environment_;
- }
+ extensions::TestExtensionEnvironment& environment() { return *environment_; }
protected:
- extensions::TestExtensionEnvironment environment_;
std::unique_ptr<ProtocolHandlerRegistry> registry_;
DISALLOW_COPY_AND_ASSIGN(RenderViewContextMenuExtensionsTest);
diff --git a/components/viz/frame_sinks/mojo_frame_sink_manager.cc b/components/viz/frame_sinks/mojo_frame_sink_manager.cc
index 10392c8..f3fcac09 100644
--- a/components/viz/frame_sinks/mojo_frame_sink_manager.cc
+++ b/components/viz/frame_sinks/mojo_frame_sink_manager.cc
@@ -38,7 +38,7 @@
manager_.RemoveObserver(this);
}
-void MojoFrameSinkManager::Connect(
+void MojoFrameSinkManager::BindPtrAndSetClient(
cc::mojom::FrameSinkManagerRequest request,
cc::mojom::FrameSinkManagerClientPtr client) {
DCHECK(!binding_.is_bound());
diff --git a/components/viz/frame_sinks/mojo_frame_sink_manager.h b/components/viz/frame_sinks/mojo_frame_sink_manager.h
index c905855..f0f4f893 100644
--- a/components/viz/frame_sinks/mojo_frame_sink_manager.h
+++ b/components/viz/frame_sinks/mojo_frame_sink_manager.h
@@ -44,9 +44,10 @@
cc::SurfaceManager* surface_manager() { return &manager_; }
- // Binds to |request| and store connection back to |client|.
- void Connect(cc::mojom::FrameSinkManagerRequest request,
- cc::mojom::FrameSinkManagerClientPtr client);
+ // Binds |this| as a FrameSinkManager for a given |request|. This may
+ // only be called once.
+ void BindPtrAndSetClient(cc::mojom::FrameSinkManagerRequest request,
+ cc::mojom::FrameSinkManagerClientPtr client);
// cc::mojom::FrameSinkManager implementation:
void CreateRootCompositorFrameSink(
diff --git a/components/viz/host/frame_sink_manager_host.cc b/components/viz/host/frame_sink_manager_host.cc
index 6ab4212..1a9cb0e 100644
--- a/components/viz/host/frame_sink_manager_host.cc
+++ b/components/viz/host/frame_sink_manager_host.cc
@@ -8,26 +8,20 @@
#include "cc/surfaces/surface_info.h"
#include "cc/surfaces/surface_manager.h"
+#include "components/viz/frame_sinks/mojo_frame_sink_manager.h"
namespace viz {
-FrameSinkManagerHost::FrameSinkManagerHost()
- : binding_(this),
- frame_sink_manager_(false, // Use surface sequences.
- nullptr) {}
+FrameSinkManagerHost::FrameSinkManagerHost() : binding_(this) {}
-FrameSinkManagerHost::~FrameSinkManagerHost() {}
+FrameSinkManagerHost::~FrameSinkManagerHost() = default;
-cc::SurfaceManager* FrameSinkManagerHost::surface_manager() {
- return frame_sink_manager_.surface_manager();
-}
-
-void FrameSinkManagerHost::ConnectToFrameSinkManager() {
- DCHECK(!frame_sink_manager_ptr_.is_bound());
- cc::mojom::FrameSinkManagerClientPtr client;
- binding_.Bind(mojo::MakeRequest(&client));
- frame_sink_manager_.Connect(mojo::MakeRequest(&frame_sink_manager_ptr_),
- std::move(client));
+void FrameSinkManagerHost::BindManagerClientAndSetManagerPtr(
+ cc::mojom::FrameSinkManagerClientRequest request,
+ cc::mojom::FrameSinkManagerPtr ptr) {
+ DCHECK(!binding_.is_bound());
+ binding_.Bind(std::move(request));
+ frame_sink_manager_ptr_ = std::move(ptr);
}
void FrameSinkManagerHost::AddObserver(FrameSinkObserver* observer) {
@@ -71,4 +65,27 @@
observer.OnSurfaceCreated(surface_info);
}
+// static
+void FrameSinkManagerHost::ConnectWithInProcessFrameSinkManager(
+ FrameSinkManagerHost* host,
+ MojoFrameSinkManager* manager) {
+ // A mojo pointer to |host| which is the FrameSinkManager's client.
+ cc::mojom::FrameSinkManagerClientPtr host_mojo;
+ // A mojo pointer to |manager|.
+ cc::mojom::FrameSinkManagerPtr manager_mojo;
+
+ // A request to bind to each of the above interfaces.
+ cc::mojom::FrameSinkManagerClientRequest host_mojo_request =
+ mojo::MakeRequest(&host_mojo);
+ cc::mojom::FrameSinkManagerRequest manager_mojo_request =
+ mojo::MakeRequest(&manager_mojo);
+
+ // Sets |manager_mojo| which is given to the |host|.
+ manager->BindPtrAndSetClient(std::move(manager_mojo_request),
+ std::move(host_mojo));
+ // Sets |host_mojo| which was given to the |manager|.
+ host->BindManagerClientAndSetManagerPtr(std::move(host_mojo_request),
+ std::move(manager_mojo));
+}
+
} // namespace viz
diff --git a/components/viz/host/frame_sink_manager_host.h b/components/viz/host/frame_sink_manager_host.h
index 6642580a..dd3f80d 100644
--- a/components/viz/host/frame_sink_manager_host.h
+++ b/components/viz/host/frame_sink_manager_host.h
@@ -10,7 +10,6 @@
#include "base/observer_list.h"
#include "cc/ipc/frame_sink_manager.mojom.h"
#include "cc/surfaces/frame_sink_id.h"
-#include "components/viz/frame_sinks/mojo_frame_sink_manager.h"
#include "components/viz/host/frame_sink_observer.h"
#include "components/viz/host/viz_host_export.h"
#include "mojo/public/cpp/bindings/binding.h"
@@ -21,19 +20,21 @@
}
namespace viz {
+class MojoFrameSinkManager;
-// Browser side implementation of mojom::FrameSinkManager. Manages frame sinks
-// and is intended to replace SurfaceManager.
+// Browser side implementation of mojom::FrameSinkManager, to be used from the
+// UI thread. Manages frame sinks and is intended to replace SurfaceManager.
class VIZ_HOST_EXPORT FrameSinkManagerHost
: NON_EXPORTED_BASE(cc::mojom::FrameSinkManagerClient) {
public:
FrameSinkManagerHost();
~FrameSinkManagerHost() override;
- cc::SurfaceManager* surface_manager();
-
- // Start Mojo connection to FrameSinkManager. Most tests won't need this.
- void ConnectToFrameSinkManager();
+ // Binds |this| as a FrameSinkManagerClient to the |request|. May only be
+ // called once.
+ void BindManagerClientAndSetManagerPtr(
+ cc::mojom::FrameSinkManagerClientRequest request,
+ cc::mojom::FrameSinkManagerPtr ptr);
void AddObserver(FrameSinkObserver* observer);
void RemoveObserver(FrameSinkObserver* observer);
@@ -49,21 +50,20 @@
void UnregisterFrameSinkHierarchy(const cc::FrameSinkId& parent_frame_sink_id,
const cc::FrameSinkId& child_frame_sink_id);
+ static void ConnectWithInProcessFrameSinkManager(
+ FrameSinkManagerHost* host,
+ MojoFrameSinkManager* manager);
+
private:
// cc::mojom::FrameSinkManagerClient:
void OnSurfaceCreated(const cc::SurfaceInfo& surface_info) override;
- // Mojo connection to |frame_sink_manager_|.
+ // Mojo connection to the FrameSinkManager.
cc::mojom::FrameSinkManagerPtr frame_sink_manager_ptr_;
- // Mojo connection back from |frame_sink_manager_|.
+ // Mojo connection back from the FrameSinkManager.
mojo::Binding<cc::mojom::FrameSinkManagerClient> binding_;
- // This is owned here so that SurfaceManager will be accessible in process.
- // Other than using SurfaceManager, access to |frame_sink_manager_| should
- // happen using Mojo. See http://crbug.com/657959.
- MojoFrameSinkManager frame_sink_manager_;
-
// Local observers to that receive OnSurfaceCreated() messages from IPC.
base::ObserverList<FrameSinkObserver> observers_;
diff --git a/content/browser/browser_main_loop.cc b/content/browser/browser_main_loop.cc
index 766dfdbb..ff7508f 100644
--- a/content/browser/browser_main_loop.cc
+++ b/content/browser/browser_main_loop.cc
@@ -53,6 +53,7 @@
#include "components/tracing/common/trace_to_console.h"
#include "components/tracing/common/tracing_switches.h"
#include "components/viz/display_compositor/host_shared_bitmap_manager.h"
+#include "components/viz/frame_sinks/mojo_frame_sink_manager.h"
#include "components/viz/host/frame_sink_manager_host.h"
#include "content/browser/browser_thread_impl.h"
#include "content/browser/child_process_security_policy_impl.h"
@@ -1220,6 +1221,7 @@
#endif
#if !defined(OS_ANDROID)
+ frame_sink_manager_.reset();
frame_sink_manager_host_.reset();
#endif
@@ -1360,6 +1362,12 @@
}
}
+#if !defined(OS_ANDROID)
+cc::SurfaceManager* BrowserMainLoop::GetSurfaceManager() const {
+ return frame_sink_manager_->surface_manager();
+}
+#endif
+
void BrowserMainLoop::StopStartupTracingTimer() {
startup_trace_timer_.Stop();
}
@@ -1451,7 +1459,13 @@
#if !defined(OS_ANDROID)
if (!service_manager::ServiceManagerIsRemote()) {
frame_sink_manager_host_ = base::MakeUnique<viz::FrameSinkManagerHost>();
- frame_sink_manager_host_->ConnectToFrameSinkManager();
+
+ // TODO(danakj): Don't make a MojoFrameSinkManager when display is in the
+ // Gpu process, instead get the mojo pointer from the Gpu process.
+ frame_sink_manager_ =
+ base::MakeUnique<viz::MojoFrameSinkManager>(false, nullptr);
+ viz::FrameSinkManagerHost::ConnectWithInProcessFrameSinkManager(
+ frame_sink_manager_host_.get(), frame_sink_manager_.get());
}
#endif
diff --git a/content/browser/browser_main_loop.h b/content/browser/browser_main_loop.h
index 596d78f..67501c4 100644
--- a/content/browser/browser_main_loop.h
+++ b/content/browser/browser_main_loop.h
@@ -34,6 +34,10 @@
} // namespace trace_event
} // namespace base
+namespace cc {
+class SurfaceManager;
+}
+
namespace discardable_memory {
class DiscardableSharedMemoryManager;
}
@@ -78,6 +82,7 @@
namespace viz {
class FrameSinkManagerHost;
+class MojoFrameSinkManager;
}
namespace content {
@@ -170,6 +175,10 @@
viz::FrameSinkManagerHost* frame_sink_manager_host() const {
return frame_sink_manager_host_.get();
}
+
+ // TODO(crbug.com/657959): This will be removed once there are no users, as
+ // SurfaceManager is being moved out of process.
+ cc::SurfaceManager* GetSurfaceManager() const;
#endif
void StopStartupTracingTimer();
@@ -329,6 +338,12 @@
memory_instrumentation_coordinator_;
#if !defined(OS_ANDROID)
std::unique_ptr<viz::FrameSinkManagerHost> frame_sink_manager_host_;
+ // This is owned here so that SurfaceManager will be accessible in process
+ // when display is in the same process. Other than using SurfaceManager,
+ // access to |in_process_frame_sink_manager_| should happen via
+ // |frame_sink_manager_host_| instead which uses Mojo. See
+ // http://crbug.com/657959.
+ std::unique_ptr<viz::MojoFrameSinkManager> frame_sink_manager_;
#endif
// DO NOT add members here. Add them to the right categories above.
diff --git a/content/browser/compositor/gpu_process_transport_factory.cc b/content/browser/compositor/gpu_process_transport_factory.cc
index 73b55bd9..f3fedcc 100644
--- a/content/browser/compositor/gpu_process_transport_factory.cc
+++ b/content/browser/compositor/gpu_process_transport_factory.cc
@@ -727,9 +727,7 @@
}
cc::SurfaceManager* GpuProcessTransportFactory::GetSurfaceManager() {
- return BrowserMainLoop::GetInstance()
- ->frame_sink_manager_host()
- ->surface_manager();
+ return BrowserMainLoop::GetInstance()->GetSurfaceManager();
}
viz::FrameSinkManagerHost*
diff --git a/content/browser/compositor/gpu_process_transport_factory.h b/content/browser/compositor/gpu_process_transport_factory.h
index c9013aa..2669561d 100644
--- a/content/browser/compositor/gpu_process_transport_factory.h
+++ b/content/browser/compositor/gpu_process_transport_factory.h
@@ -18,6 +18,7 @@
#include "build/build_config.h"
#include "cc/output/renderer_settings.h"
#include "cc/surfaces/frame_sink_id_allocator.h"
+#include "components/viz/host/frame_sink_manager_host.h"
#include "content/browser/compositor/image_transport_factory.h"
#include "gpu/ipc/client/gpu_channel_host.h"
#include "ui/compositor/compositor.h"
@@ -34,10 +35,6 @@
class ContextProviderCommandBuffer;
}
-namespace viz {
-class FrameSinkManagerHost;
-}
-
namespace content {
class OutputDeviceBacking;
diff --git a/content/browser/compositor/reflector_impl_unittest.cc b/content/browser/compositor/reflector_impl_unittest.cc
index 95280c6..98ff7a7 100644
--- a/content/browser/compositor/reflector_impl_unittest.cc
+++ b/content/browser/compositor/reflector_impl_unittest.cc
@@ -132,10 +132,9 @@
ui::InitializeContextFactoryForTests(enable_pixel_output, &context_factory,
&context_factory_private);
- ImageTransportFactory::InitializeForUnitTests(
- std::unique_ptr<ImageTransportFactory>(
- new NoTransportImageTransportFactory));
message_loop_.reset(new base::MessageLoop());
+ ImageTransportFactory::InitializeForUnitTests(
+ base::MakeUnique<NoTransportImageTransportFactory>());
task_runner_ = message_loop_->task_runner();
compositor_task_runner_ = new FakeTaskRunner();
begin_frame_source_.reset(new cc::DelayBasedBeginFrameSource(
diff --git a/content/browser/compositor/test/no_transport_image_transport_factory.cc b/content/browser/compositor/test/no_transport_image_transport_factory.cc
index c2f01f1..fba94b0 100644
--- a/content/browser/compositor/test/no_transport_image_transport_factory.cc
+++ b/content/browser/compositor/test/no_transport_image_transport_factory.cc
@@ -13,36 +13,38 @@
#include "components/viz/display_compositor/gl_helper.h"
#include "gpu/command_buffer/client/gles2_interface.h"
#include "ui/compositor/compositor.h"
-#include "ui/compositor/test/in_process_context_factory.h"
namespace content {
NoTransportImageTransportFactory::NoTransportImageTransportFactory()
- : frame_sink_manager_host_(base::MakeUnique<viz::FrameSinkManagerHost>()),
- context_factory_(base::MakeUnique<ui::InProcessContextFactory>(
- frame_sink_manager_host_.get())) {
+ : frame_sink_manager_(false /* use surface references */, nullptr),
+ context_factory_(&frame_sink_manager_host_,
+ frame_sink_manager_.surface_manager()) {
+ viz::FrameSinkManagerHost::ConnectWithInProcessFrameSinkManager(
+ &frame_sink_manager_host_, &frame_sink_manager_);
+
// The context factory created here is for unit tests, thus using a higher
// refresh rate to spend less time waiting for BeginFrames.
- context_factory_->SetUseFastRefreshRateForTests();
+ context_factory_.SetUseFastRefreshRateForTests();
}
NoTransportImageTransportFactory::~NoTransportImageTransportFactory() {
std::unique_ptr<viz::GLHelper> lost_gl_helper = std::move(gl_helper_);
- context_factory_->SendOnLostResources();
+ context_factory_.SendOnLostResources();
}
ui::ContextFactory* NoTransportImageTransportFactory::GetContextFactory() {
- return context_factory_.get();
+ return &context_factory_;
}
ui::ContextFactoryPrivate*
NoTransportImageTransportFactory::GetContextFactoryPrivate() {
- return context_factory_.get();
+ return &context_factory_;
}
viz::GLHelper* NoTransportImageTransportFactory::GetGLHelper() {
if (!gl_helper_) {
- context_provider_ = context_factory_->SharedMainThreadContextProvider();
+ context_provider_ = context_factory_.SharedMainThreadContextProvider();
gl_helper_.reset(new viz::GLHelper(context_provider_->ContextGL(),
context_provider_->ContextSupport()));
}
diff --git a/content/browser/compositor/test/no_transport_image_transport_factory.h b/content/browser/compositor/test/no_transport_image_transport_factory.h
index 6e548f20..a238287 100644
--- a/content/browser/compositor/test/no_transport_image_transport_factory.h
+++ b/content/browser/compositor/test/no_transport_image_transport_factory.h
@@ -10,8 +10,10 @@
#include "base/macros.h"
#include "build/build_config.h"
#include "cc/surfaces/surface_manager.h"
+#include "components/viz/frame_sinks/mojo_frame_sink_manager.h"
#include "components/viz/host/frame_sink_manager_host.h"
#include "content/browser/compositor/image_transport_factory.h"
+#include "ui/compositor/test/in_process_context_factory.h"
namespace cc {
class ContextProvider;
@@ -41,8 +43,10 @@
#endif
private:
- std::unique_ptr<viz::FrameSinkManagerHost> frame_sink_manager_host_;
- std::unique_ptr<ui::InProcessContextFactory> context_factory_;
+ // The FrameSinkManager implementation lives in-process here for tests.
+ viz::MojoFrameSinkManager frame_sink_manager_;
+ viz::FrameSinkManagerHost frame_sink_manager_host_;
+ ui::InProcessContextFactory context_factory_;
scoped_refptr<cc::ContextProvider> context_provider_;
std::unique_ptr<viz::GLHelper> gl_helper_;
diff --git a/content/browser/renderer_host/compositor_impl_android.cc b/content/browser/renderer_host/compositor_impl_android.cc
index c514d5a..006a51d 100644
--- a/content/browser/renderer_host/compositor_impl_android.cc
+++ b/content/browser/renderer_host/compositor_impl_android.cc
@@ -49,6 +49,7 @@
#include "components/viz/display_compositor/compositor_overlay_candidate_validator_android.h"
#include "components/viz/display_compositor/gl_helper.h"
#include "components/viz/display_compositor/host_shared_bitmap_manager.h"
+#include "components/viz/frame_sinks/mojo_frame_sink_manager.h"
#include "components/viz/host/frame_sink_manager_host.h"
#include "content/browser/gpu/browser_gpu_channel_host_factory.h"
#include "content/browser/gpu/browser_gpu_memory_buffer_manager.h"
@@ -97,12 +98,23 @@
struct CompositorDependencies {
CompositorDependencies() : frame_sink_id_allocator(kDefaultClientId) {
- frame_sink_manager_host.ConnectToFrameSinkManager();
+ // TODO(danakj): Don't make a MojoFrameSinkManager when display is in the
+ // Gpu process, instead get the mojo pointer from the Gpu process.
+ frame_sink_manager =
+ base::MakeUnique<viz::MojoFrameSinkManager>(false, nullptr);
+ viz::FrameSinkManagerHost::ConnectWithInProcessFrameSinkManager(
+ &frame_sink_manager_host, frame_sink_manager.get());
}
SingleThreadTaskGraphRunner task_graph_runner;
viz::FrameSinkManagerHost frame_sink_manager_host;
cc::FrameSinkIdAllocator frame_sink_id_allocator;
+ // This is owned here so that SurfaceManager will be accessible in process
+ // when display is in the same process. Other than using SurfaceManager,
+ // access to |in_process_frame_sink_manager_| should happen via
+ // |frame_sink_manager_host_| instead which uses Mojo. See
+ // http://crbug.com/657959.
+ std::unique_ptr<viz::MojoFrameSinkManager> frame_sink_manager;
#if BUILDFLAG(ENABLE_VULKAN)
scoped_refptr<cc::VulkanContextProvider> vulkan_context_provider;
@@ -402,8 +414,7 @@
// static
cc::SurfaceManager* CompositorImpl::GetSurfaceManager() {
- return g_compositor_dependencies.Get()
- .frame_sink_manager_host.surface_manager();
+ return g_compositor_dependencies.Get().frame_sink_manager->surface_manager();
}
// static
diff --git a/content/browser/renderer_host/offscreen_canvas_provider_impl_unittest.cc b/content/browser/renderer_host/offscreen_canvas_provider_impl_unittest.cc
index 7633f98..fc515d4 100644
--- a/content/browser/renderer_host/offscreen_canvas_provider_impl_unittest.cc
+++ b/content/browser/renderer_host/offscreen_canvas_provider_impl_unittest.cc
@@ -145,16 +145,22 @@
void SetUp() override {
#if !defined(OS_ANDROID)
ImageTransportFactory::InitializeForUnitTests(
- std::unique_ptr<ImageTransportFactory>(
- new NoTransportImageTransportFactory));
+ base::MakeUnique<NoTransportImageTransportFactory>());
#endif
frame_sink_manager_host_ = base::MakeUnique<viz::FrameSinkManagerHost>();
- frame_sink_manager_host_->ConnectToFrameSinkManager();
+
+ // The FrameSinkManager implementation is in-process here for tests.
+ frame_sink_manager_ =
+ base::MakeUnique<viz::MojoFrameSinkManager>(false, nullptr);
+ viz::FrameSinkManagerHost::ConnectWithInProcessFrameSinkManager(
+ frame_sink_manager_host_.get(), frame_sink_manager_.get());
+
provider_ = base::MakeUnique<OffscreenCanvasProviderImpl>(
frame_sink_manager_host_.get(), kRendererClientId);
}
void TearDown() override {
provider_.reset();
+ frame_sink_manager_.reset();
frame_sink_manager_host_.reset();
#if !defined(OS_ANDROID)
ImageTransportFactory::Terminate();
@@ -162,8 +168,11 @@
}
private:
+ // A MessageLoop is required for mojo bindings which are used to
+ // connect to graphics services.
base::MessageLoop message_loop_;
std::unique_ptr<viz::FrameSinkManagerHost> frame_sink_manager_host_;
+ std::unique_ptr<viz::MojoFrameSinkManager> frame_sink_manager_;
std::unique_ptr<OffscreenCanvasProviderImpl> provider_;
};
diff --git a/content/browser/renderer_host/render_widget_host_view_mac_editcommand_helper.h b/content/browser/renderer_host/render_widget_host_view_mac_editcommand_helper.h
index 9cdb0aa..bf9a5c8bb 100644
--- a/content/browser/renderer_host/render_widget_host_view_mac_editcommand_helper.h
+++ b/content/browser/renderer_host/render_widget_host_view_mac_editcommand_helper.h
@@ -29,10 +29,11 @@
// fact a distinct object) When these selectors are called, the relevant
// edit command is executed in WebCore.
class CONTENT_EXPORT RenderWidgetHostViewMacEditCommandHelper {
- FRIEND_TEST_ALL_PREFIXES(RenderWidgetHostViewMacEditCommandHelperTest,
- TestAddEditingSelectorsToClass);
- FRIEND_TEST_ALL_PREFIXES(RenderWidgetHostViewMacEditCommandHelperTest,
- TestEditingCommandDelivery);
+ FRIEND_TEST_ALL_PREFIXES(RenderWidgetHostViewMacEditCommandHelperTest,
+ TestAddEditingSelectorsToClass);
+ FRIEND_TEST_ALL_PREFIXES(
+ RenderWidgetHostViewMacEditCommandHelperWithTaskEnvTest,
+ TestEditingCommandDelivery);
public:
RenderWidgetHostViewMacEditCommandHelper();
diff --git a/content/browser/renderer_host/render_widget_host_view_mac_editcommand_helper_unittest.mm b/content/browser/renderer_host/render_widget_host_view_mac_editcommand_helper_unittest.mm
index 7846fc8d..d77f146 100644
--- a/content/browser/renderer_host/render_widget_host_view_mac_editcommand_helper_unittest.mm
+++ b/content/browser/renderer_host/render_widget_host_view_mac_editcommand_helper_unittest.mm
@@ -9,6 +9,7 @@
#include <stdint.h>
#include "base/mac/scoped_nsautorelease_pool.h"
+#include "base/message_loop/message_loop.h"
#include "base/test/scoped_task_environment.h"
#include "base/threading/thread_task_runner_handle.h"
#include "content/browser/compositor/test/no_transport_image_transport_factory.h"
@@ -107,17 +108,33 @@
protected:
void SetUp() override {
ImageTransportFactory::InitializeForUnitTests(
- std::unique_ptr<ImageTransportFactory>(
- new NoTransportImageTransportFactory));
+ base::MakeUnique<NoTransportImageTransportFactory>());
}
void TearDown() override { ImageTransportFactory::Terminate(); }
+
+ private:
+ base::MessageLoop message_loop_;
+};
+
+class RenderWidgetHostViewMacEditCommandHelperWithTaskEnvTest
+ : public PlatformTest {
+ protected:
+ void SetUp() override {
+ ImageTransportFactory::InitializeForUnitTests(
+ base::MakeUnique<NoTransportImageTransportFactory>());
+ }
+ void TearDown() override { ImageTransportFactory::Terminate(); }
+
+ private:
+ // This has a MessageLoop for ImageTransportFactory.
+ base::test::ScopedTaskEnvironment scoped_task_environment_;
};
} // namespace
// Tests that editing commands make it through the pipeline all the way to
// RenderWidgetHost.
-TEST_F(RenderWidgetHostViewMacEditCommandHelperTest,
+TEST_F(RenderWidgetHostViewMacEditCommandHelperWithTaskEnvTest,
TestEditingCommandDelivery) {
MockRenderWidgetHostDelegate delegate;
TestBrowserContext browser_context;
@@ -131,7 +148,6 @@
ui::test::ScopedSetSupportedScaleFactors scoped_supported(supported_factors);
base::mac::ScopedNSAutoreleasePool pool;
- base::test::ScopedTaskEnvironment scoped_task_environment_;
int32_t routing_id = process_host->GetNextRoutingID();
RenderWidgetHostEditCommandCounter* render_widget =
diff --git a/content/public/test/mock_render_process_host.cc b/content/public/test/mock_render_process_host.cc
index 471aa1f..4198037 100644
--- a/content/public/test/mock_render_process_host.cc
+++ b/content/public/test/mock_render_process_host.cc
@@ -46,7 +46,8 @@
deletion_callback_called_(false),
is_for_guests_only_(false),
is_process_backgrounded_(false),
- worker_ref_count_(0) {
+ worker_ref_count_(0),
+ weak_ptr_factory_(this) {
// Child process security operations can't be unit tested unless we add
// ourselves as an existing child process.
ChildProcessSecurityPolicyImpl::GetInstance()->Add(GetID());
@@ -212,11 +213,19 @@
return false;
}
+static void DeleteIt(base::WeakPtr<MockRenderProcessHost> h) {
+ if (h)
+ delete h.get();
+}
+
void MockRenderProcessHost::Cleanup() {
if (listeners_.IsEmpty()) {
for (auto& observer : observers_)
observer.RenderProcessHostDestroyed(this);
- base::ThreadTaskRunnerHandle::Get()->DeleteSoon(FROM_HERE, this);
+ // Post the delete of |this| as a WeakPtr so that if |this| is deleted by a
+ // test directly, we don't double free.
+ base::ThreadTaskRunnerHandle::Get()->PostTask(
+ FROM_HERE, base::Bind(&DeleteIt, weak_ptr_factory_.GetWeakPtr()));
RenderProcessHostImpl::UnregisterHost(GetID());
deletion_callback_called_ = true;
}
diff --git a/content/public/test/mock_render_process_host.h b/content/public/test/mock_render_process_host.h
index 86fa2dba..5c1cb5c 100644
--- a/content/public/test/mock_render_process_host.h
+++ b/content/public/test/mock_render_process_host.h
@@ -12,6 +12,7 @@
#include <vector>
#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
#include "base/metrics/persistent_memory_allocator.h"
#include "base/observer_list.h"
#include "content/public/browser/render_process_host.h"
@@ -166,7 +167,6 @@
BrowserContext* browser_context_;
base::ObserverList<RenderProcessHostObserver> observers_;
- IDMap<RenderWidgetHost*> render_widget_hosts_;
int prev_routing_id_;
IDMap<IPC::Listener*> listeners_;
bool fast_shutdown_started_;
@@ -179,6 +179,7 @@
renderer_interface_;
std::map<std::string, InterfaceBinder> binder_overrides_;
service_manager::Identity child_identity_;
+ base::WeakPtrFactory<MockRenderProcessHost> weak_ptr_factory_;
DISALLOW_COPY_AND_ASSIGN(MockRenderProcessHost);
};
diff --git a/content/public/test/test_renderer_host.cc b/content/public/test/test_renderer_host.cc
index 6adcd65..a5b67e1 100644
--- a/content/public/test/test_renderer_host.cc
+++ b/content/public/test/test_renderer_host.cc
@@ -129,9 +129,15 @@
: rph_factory_(new MockRenderProcessHostFactory()),
rvh_factory_(new TestRenderViewHostFactory(rph_factory_.get())),
rfh_factory_(new TestRenderFrameHostFactory()) {
+ // A MessageLoop is needed for Mojo bindings to graphics services. Some
+ // tests have their own, so this only creates one when none exists. This
+ // means tests must ensure any MessageLoop they make is created before
+ // the RenderViewHostTestEnabler.
+ if (!base::MessageLoop::current())
+ message_loop_ = base::MakeUnique<base::MessageLoop>();
#if !defined(OS_ANDROID)
ImageTransportFactory::InitializeForUnitTests(
- base::WrapUnique(new NoTransportImageTransportFactory));
+ base::MakeUnique<NoTransportImageTransportFactory>());
#else
if (!screen_)
screen_.reset(ui::CreateDummyScreenAndroid());
diff --git a/content/public/test/test_renderer_host.h b/content/public/test/test_renderer_host.h
index 09c3e26..8292ece 100644
--- a/content/public/test/test_renderer_host.h
+++ b/content/public/test/test_renderer_host.h
@@ -198,6 +198,7 @@
#if defined(OS_ANDROID)
std::unique_ptr<display::Screen> screen_;
#endif
+ std::unique_ptr<base::MessageLoop> message_loop_;
std::unique_ptr<MockRenderProcessHostFactory> rph_factory_;
std::unique_ptr<TestRenderViewHostFactory> rvh_factory_;
std::unique_ptr<TestRenderFrameHostFactory> rfh_factory_;
diff --git a/content/test/BUILD.gn b/content/test/BUILD.gn
index 5ac19340..a3f3210 100644
--- a/content/test/BUILD.gn
+++ b/content/test/BUILD.gn
@@ -250,6 +250,7 @@
"//components/leveldb/public/interfaces",
"//components/payments/mojom:mojom_payment_app",
"//components/viz/display_compositor",
+ "//components/viz/frame_sinks",
"//components/viz/host",
"//content/app:both_for_content_tests",
"//content/browser:for_content_tests",
diff --git a/extensions/browser/api/bluetooth/bluetooth_event_router_unittest.cc b/extensions/browser/api/bluetooth/bluetooth_event_router_unittest.cc
index a0c8831..1104bb5 100644
--- a/extensions/browser/api/bluetooth/bluetooth_event_router_unittest.cc
+++ b/extensions/browser/api/bluetooth/bluetooth_event_router_unittest.cc
@@ -42,7 +42,8 @@
class BluetoothEventRouterTest : public ExtensionsTest {
public:
BluetoothEventRouterTest()
- : mock_adapter_(new testing::StrictMock<device::MockBluetoothAdapter>()) {
+ : ExtensionsTest(base::MakeUnique<content::TestBrowserThreadBundle>()),
+ mock_adapter_(new testing::StrictMock<device::MockBluetoothAdapter>()) {
}
void SetUp() override {
@@ -59,7 +60,6 @@
}
protected:
- content::TestBrowserThreadBundle test_browser_thread_bundle_;
testing::StrictMock<device::MockBluetoothAdapter>* mock_adapter_;
std::unique_ptr<BluetoothEventRouter> router_;
};
diff --git a/extensions/browser/api/file_handlers/mime_util_unittest.cc b/extensions/browser/api/file_handlers/mime_util_unittest.cc
index becb4fe..269d5b9 100644
--- a/extensions/browser/api/file_handlers/mime_util_unittest.cc
+++ b/extensions/browser/api/file_handlers/mime_util_unittest.cc
@@ -51,7 +51,8 @@
class FileHandlersMimeUtilTest : public ExtensionsTest {
protected:
- FileHandlersMimeUtilTest() {}
+ FileHandlersMimeUtilTest()
+ : ExtensionsTest(base::MakeUnique<content::TestBrowserThreadBundle>()) {}
~FileHandlersMimeUtilTest() override {}
void SetUp() override {
@@ -66,7 +67,6 @@
kSampleContent.size()));
}
- content::TestBrowserThreadBundle thread_bundle_;
ExtensionsAPIClient extensions_api_client_;
scoped_refptr<storage::FileSystemContext> file_system_context_;
base::FilePath html_mime_file_path_;
diff --git a/extensions/browser/api/storage/storage_frontend_unittest.cc b/extensions/browser/api/storage/storage_frontend_unittest.cc
index 4fe87c7..ec47409 100644
--- a/extensions/browser/api/storage/storage_frontend_unittest.cc
+++ b/extensions/browser/api/storage/storage_frontend_unittest.cc
@@ -39,7 +39,8 @@
// history, the test names are unchanged.
class ExtensionSettingsFrontendTest : public ExtensionsTest {
public:
- ExtensionSettingsFrontendTest() = default;
+ ExtensionSettingsFrontendTest()
+ : ExtensionsTest(base::MakeUnique<content::TestBrowserThreadBundle>()) {}
void SetUp() override {
ExtensionsTest::SetUp();
@@ -66,7 +67,6 @@
scoped_refptr<ValueStoreFactoryImpl> storage_factory_;
private:
- content::TestBrowserThreadBundle test_browser_thread_bundle_;
ExtensionsAPIClient extensions_api_client_;
};
diff --git a/extensions/browser/api_unittest.cc b/extensions/browser/api_unittest.cc
index 9dda18c..301017f 100644
--- a/extensions/browser/api_unittest.cc
+++ b/extensions/browser/api_unittest.cc
@@ -29,15 +29,14 @@
namespace extensions {
-ApiUnitTest::ApiUnitTest() {}
+ApiUnitTest::ApiUnitTest()
+ : ExtensionsTest(base::MakeUnique<content::TestBrowserThreadBundle>()) {}
ApiUnitTest::~ApiUnitTest() {}
void ApiUnitTest::SetUp() {
ExtensionsTest::SetUp();
- thread_bundle_.reset(new content::TestBrowserThreadBundle(
- content::TestBrowserThreadBundle::DEFAULT));
user_prefs::UserPrefs::Set(browser_context(), &testing_pref_service_);
extension_ = ExtensionBuilder()
diff --git a/extensions/browser/api_unittest.h b/extensions/browser/api_unittest.h
index e1404fd..1f07889 100644
--- a/extensions/browser/api_unittest.h
+++ b/extensions/browser/api_unittest.h
@@ -19,7 +19,6 @@
}
namespace content {
-class TestBrowserThreadBundle;
class WebContents;
}
@@ -86,7 +85,6 @@
const std::string& args);
private:
- std::unique_ptr<content::TestBrowserThreadBundle> thread_bundle_;
sync_preferences::TestingPrefServiceSyncable testing_pref_service_;
// The WebContents used to associate a RenderViewHost with API function calls,
diff --git a/extensions/browser/app_window/app_window_geometry_cache_unittest.cc b/extensions/browser/app_window/app_window_geometry_cache_unittest.cc
index 17bece6..0cb12a1 100644
--- a/extensions/browser/app_window/app_window_geometry_cache_unittest.cc
+++ b/extensions/browser/app_window/app_window_geometry_cache_unittest.cc
@@ -41,7 +41,8 @@
// Base class for tests.
class AppWindowGeometryCacheTest : public ExtensionsTest {
public:
- AppWindowGeometryCacheTest() = default;
+ AppWindowGeometryCacheTest()
+ : ExtensionsTest(base::MakeUnique<content::TestBrowserThreadBundle>()) {}
// testing::Test overrides:
void SetUp() override;
@@ -65,7 +66,6 @@
std::string AddExtensionWithPrefs(const std::string& name);
protected:
- content::TestBrowserThreadBundle test_browser_thread_bundle_;
ExtensionPrefs* extension_prefs_; // Weak.
std::unique_ptr<AppWindowGeometryCache> cache_;
};
diff --git a/extensions/browser/content_hash_fetcher_unittest.cc b/extensions/browser/content_hash_fetcher_unittest.cc
index 68a1111d..46be54e5 100644
--- a/extensions/browser/content_hash_fetcher_unittest.cc
+++ b/extensions/browser/content_hash_fetcher_unittest.cc
@@ -125,19 +125,16 @@
class ContentHashFetcherTest : public ExtensionsTest {
public:
- ContentHashFetcherTest() {}
- ~ContentHashFetcherTest() override {}
-
- void SetUp() override {
- ExtensionsTest::SetUp();
- // We need a real IO thread to be able to intercept the network request
- // for the missing verified_contents.json file.
- browser_threads_.reset(new content::TestBrowserThreadBundle(
- content::TestBrowserThreadBundle::REAL_IO_THREAD));
+ ContentHashFetcherTest()
+ // We need a real IO thread to be able to intercept the network request
+ // for the missing verified_contents.json file.
+ : ExtensionsTest(base::MakeUnique<content::TestBrowserThreadBundle>(
+ content::TestBrowserThreadBundle::REAL_IO_THREAD)) {
request_context_ = new net::TestURLRequestContextGetter(
content::BrowserThread::GetTaskRunnerForThread(
content::BrowserThread::IO));
}
+ ~ContentHashFetcherTest() override {}
net::URLRequestContextGetter* request_context() {
return request_context_.get();
@@ -180,7 +177,6 @@
}
protected:
- std::unique_ptr<content::TestBrowserThreadBundle> browser_threads_;
std::unique_ptr<net::TestURLRequestInterceptor> interceptor_;
scoped_refptr<net::TestURLRequestContextGetter> request_context_;
base::ScopedTempDir temp_dir_;
diff --git a/extensions/browser/content_verify_job_unittest.cc b/extensions/browser/content_verify_job_unittest.cc
index 4ecfeb8..4bfb3eb 100644
--- a/extensions/browser/content_verify_job_unittest.cc
+++ b/extensions/browser/content_verify_job_unittest.cc
@@ -76,17 +76,12 @@
class ContentVerifyJobUnittest : public ExtensionsTest {
public:
- ContentVerifyJobUnittest() {}
+ ContentVerifyJobUnittest()
+ // The TestBrowserThreadBundle is needed for ContentVerifyJob::Start().
+ : ExtensionsTest(base::MakeUnique<content::TestBrowserThreadBundle>(
+ content::TestBrowserThreadBundle::REAL_IO_THREAD)) {}
~ContentVerifyJobUnittest() override {}
- void SetUp() override {
- ExtensionsTest::SetUp();
-
- // Needed for ContentVerifyJob::Start().
- browser_threads_ = base::MakeUnique<content::TestBrowserThreadBundle>(
- content::TestBrowserThreadBundle::REAL_IO_THREAD);
- }
-
// Helper to get files from our subdirectory in the general extensions test
// data dir.
base::FilePath GetTestPath(const base::FilePath& relative_path) {
@@ -135,7 +130,6 @@
private:
base::ScopedTempDir temp_dir_;
- std::unique_ptr<content::TestBrowserThreadBundle> browser_threads_;
DISALLOW_COPY_AND_ASSIGN(ContentVerifyJobUnittest);
};
diff --git a/extensions/browser/event_router_unittest.cc b/extensions/browser/event_router_unittest.cc
index 9890820..7b1ee9064 100644
--- a/extensions/browser/event_router_unittest.cc
+++ b/extensions/browser/event_router_unittest.cc
@@ -127,7 +127,8 @@
class EventRouterTest : public ExtensionsTest {
public:
- EventRouterTest() {}
+ EventRouterTest()
+ : ExtensionsTest(base::MakeUnique<content::TestBrowserThreadBundle>()) {}
protected:
// Tests adding and removing observers from EventRouter.
@@ -174,7 +175,6 @@
}
private:
- content::TestBrowserThreadBundle thread_bundle_;
base::HistogramTester histogram_tester_;
DISALLOW_COPY_AND_ASSIGN(EventRouterTest);
diff --git a/extensions/browser/extension_icon_image_unittest.cc b/extensions/browser/extension_icon_image_unittest.cc
index 5b624c0a..4e56c8f 100644
--- a/extensions/browser/extension_icon_image_unittest.cc
+++ b/extensions/browser/extension_icon_image_unittest.cc
@@ -69,7 +69,9 @@
public IconImage::Observer {
public:
ExtensionIconImageTest()
- : image_loaded_count_(0), quit_in_image_loaded_(false) {}
+ : ExtensionsTest(base::MakeUnique<content::TestBrowserThreadBundle>()),
+ image_loaded_count_(0),
+ quit_in_image_loaded_(false) {}
~ExtensionIconImageTest() override {}
@@ -125,7 +127,6 @@
}
private:
- content::TestBrowserThreadBundle test_browser_thread_bundle_;
int image_loaded_count_;
bool quit_in_image_loaded_;
diff --git a/extensions/browser/extensions_test.cc b/extensions/browser/extensions_test.cc
index 7cec642..dc2c904 100644
--- a/extensions/browser/extensions_test.cc
+++ b/extensions/browser/extensions_test.cc
@@ -32,9 +32,21 @@
namespace extensions {
-ExtensionsTest::ExtensionsTest() {}
+ExtensionsTest::ExtensionsTest()
+ : rvh_test_enabler_(
+ base::MakeUnique<content::RenderViewHostTestEnabler>()) {}
+
+ExtensionsTest::ExtensionsTest(
+ std::unique_ptr<content::TestBrowserThreadBundle> thread_bundle)
+ : thread_bundle_(std::move(thread_bundle)),
+ rvh_test_enabler_(
+ base::MakeUnique<content::RenderViewHostTestEnabler>()) {}
ExtensionsTest::~ExtensionsTest() {
+ // Destroy the task runners before nulling the browser/utility clients, as
+ // posted tasks may use them.
+ rvh_test_enabler_.reset();
+ thread_bundle_.reset();
content::SetBrowserClientForTesting(nullptr);
content::SetUtilityClientForTesting(nullptr);
}
diff --git a/extensions/browser/extensions_test.h b/extensions/browser/extensions_test.h
index 411cfb7..ee7a24b 100644
--- a/extensions/browser/extensions_test.h
+++ b/extensions/browser/extensions_test.h
@@ -37,6 +37,10 @@
class ExtensionsTest : public testing::Test {
public:
ExtensionsTest();
+ // If the test uses a TestBrowserThreadBundle, then it must be given to the
+ // constructor here so that this class can use its MessageLoop.
+ explicit ExtensionsTest(
+ std::unique_ptr<content::TestBrowserThreadBundle> thread_bundle);
~ExtensionsTest() override;
// Returned as a BrowserContext since most users don't need methods from
@@ -70,11 +74,15 @@
std::unique_ptr<ExtensionPrefValueMap> extension_pref_value_map_;
std::unique_ptr<PrefService> pref_service_;
+ MockExtensionSystemFactory<MockExtensionSystem> extension_system_factory_;
+
+ // Optional thread bundle for some subclasses. Needs to exist before
+ // the RenderViewHostTestEnabler if it is going to exist.
+ std::unique_ptr<content::TestBrowserThreadBundle> thread_bundle_;
+
// The existence of this object enables tests via
// RenderViewHostTester.
- content::RenderViewHostTestEnabler rvh_test_enabler_;
-
- MockExtensionSystemFactory<MockExtensionSystem> extension_system_factory_;
+ std::unique_ptr<content::RenderViewHostTestEnabler> rvh_test_enabler_;
DISALLOW_COPY_AND_ASSIGN(ExtensionsTest);
};
diff --git a/extensions/browser/image_loader_unittest.cc b/extensions/browser/image_loader_unittest.cc
index 9e178a3..9ec4490 100644
--- a/extensions/browser/image_loader_unittest.cc
+++ b/extensions/browser/image_loader_unittest.cc
@@ -36,7 +36,10 @@
class ImageLoaderTest : public ExtensionsTest {
public:
- ImageLoaderTest() : image_loaded_count_(0), quit_in_image_loaded_(false) {}
+ ImageLoaderTest()
+ : ExtensionsTest(base::MakeUnique<content::TestBrowserThreadBundle>()),
+ image_loaded_count_(0),
+ quit_in_image_loaded_(false) {}
void OnImageLoaded(const gfx::Image& image) {
image_loaded_count_++;
@@ -96,7 +99,6 @@
gfx::ImageFamily image_family_;
private:
- content::TestBrowserThreadBundle test_browser_thread_bundle_;
int image_loaded_count_;
bool quit_in_image_loaded_;
};
diff --git a/extensions/browser/load_monitoring_extension_host_queue_unittest.cc b/extensions/browser/load_monitoring_extension_host_queue_unittest.cc
index 6b1ac99..b263035 100644
--- a/extensions/browser/load_monitoring_extension_host_queue_unittest.cc
+++ b/extensions/browser/load_monitoring_extension_host_queue_unittest.cc
@@ -50,7 +50,8 @@
class LoadMonitoringExtensionHostQueueTest : public ExtensionsTest {
public:
LoadMonitoringExtensionHostQueueTest()
- : finished_(false),
+ : ExtensionsTest(base::MakeUnique<content::TestBrowserThreadBundle>()),
+ finished_(false),
// Arbitrary choice of an invalid size_t.
num_queued_(g_invalid_size_t),
num_loaded_(g_invalid_size_t),
@@ -101,7 +102,6 @@
max_active_loading_ = max_active_loading;
}
- content::TestBrowserThreadBundle thread_bundle_;
std::unique_ptr<LoadMonitoringExtensionHostQueue> queue_;
std::vector<std::unique_ptr<StubDeferredStartRenderHost>> stubs_;
diff --git a/extensions/browser/mojo/keep_alive_impl_unittest.cc b/extensions/browser/mojo/keep_alive_impl_unittest.cc
index 5658d72..fadf523e 100644
--- a/extensions/browser/mojo/keep_alive_impl_unittest.cc
+++ b/extensions/browser/mojo/keep_alive_impl_unittest.cc
@@ -7,8 +7,8 @@
#include <utility>
#include "base/macros.h"
-#include "base/message_loop/message_loop.h"
#include "base/run_loop.h"
+#include "content/public/test/test_browser_thread_bundle.h"
#include "extensions/browser/extension_registry.h"
#include "extensions/browser/extensions_test.h"
#include "extensions/browser/process_manager.h"
@@ -19,12 +19,12 @@
class KeepAliveTest : public ExtensionsTest {
public:
- KeepAliveTest() {}
+ KeepAliveTest()
+ : ExtensionsTest(base::MakeUnique<content::TestBrowserThreadBundle>()) {}
~KeepAliveTest() override {}
void SetUp() override {
ExtensionsTest::SetUp();
- message_loop_.reset(new base::MessageLoop);
extension_ =
ExtensionBuilder()
.SetManifest(
@@ -46,11 +46,6 @@
.Build();
}
- void TearDown() override {
- message_loop_.reset();
- ExtensionsTest::TearDown();
- }
-
void WaitUntilLazyKeepAliveChanges() {
int initial_keep_alive_count = GetKeepAliveCount();
while (GetKeepAliveCount() == initial_keep_alive_count) {
@@ -72,7 +67,6 @@
}
private:
- std::unique_ptr<base::MessageLoop> message_loop_;
scoped_refptr<const Extension> extension_;
DISALLOW_COPY_AND_ASSIGN(KeepAliveTest);
diff --git a/extensions/browser/requirements_checker_unittest.cc b/extensions/browser/requirements_checker_unittest.cc
index 6890c91..d88f8ea1 100644
--- a/extensions/browser/requirements_checker_unittest.cc
+++ b/extensions/browser/requirements_checker_unittest.cc
@@ -57,7 +57,8 @@
class RequirementsCheckerTest : public ExtensionsTest {
public:
- RequirementsCheckerTest() {
+ RequirementsCheckerTest()
+ : ExtensionsTest(base::MakeUnique<content::TestBrowserThreadBundle>()) {
manifest_dict_ = base::MakeUnique<base::DictionaryValue>();
}
@@ -102,7 +103,6 @@
PreloadCheckRunner runner_;
private:
- content::TestBrowserThreadBundle bundle_;
scoped_refptr<Extension> extension_;
std::unique_ptr<base::DictionaryValue> manifest_dict_;
};
diff --git a/extensions/browser/sandboxed_unpacker_unittest.cc b/extensions/browser/sandboxed_unpacker_unittest.cc
index 1b6e71e6..8975af2 100644
--- a/extensions/browser/sandboxed_unpacker_unittest.cc
+++ b/extensions/browser/sandboxed_unpacker_unittest.cc
@@ -64,11 +64,13 @@
class SandboxedUnpackerTest : public ExtensionsTest {
public:
+ SandboxedUnpackerTest()
+ : ExtensionsTest(base::MakeUnique<content::TestBrowserThreadBundle>(
+ content::TestBrowserThreadBundle::IO_MAINLOOP)) {}
+
void SetUp() override {
ExtensionsTest::SetUp();
ASSERT_TRUE(extensions_dir_.CreateUniqueTempDir());
- browser_threads_.reset(new content::TestBrowserThreadBundle(
- content::TestBrowserThreadBundle::IO_MAINLOOP));
in_process_utility_thread_helper_.reset(
new content::InProcessUtilityThreadHelper);
// It will delete itself.
@@ -82,9 +84,10 @@
void TearDown() override {
// Need to destruct SandboxedUnpacker before the message loop since
// it posts a task to it.
- sandboxed_unpacker_ = NULL;
+ sandboxed_unpacker_ = nullptr;
base::RunLoop().RunUntilIdle();
ExtensionsTest::TearDown();
+ in_process_utility_thread_helper_.reset();
}
base::FilePath GetCrxFullPath(const std::string& crx_name) {
@@ -132,7 +135,6 @@
base::ScopedTempDir extensions_dir_;
MockSandboxedUnpackerClient* client_;
scoped_refptr<SandboxedUnpacker> sandboxed_unpacker_;
- std::unique_ptr<content::TestBrowserThreadBundle> browser_threads_;
std::unique_ptr<content::InProcessUtilityThreadHelper>
in_process_utility_thread_helper_;
};
diff --git a/extensions/browser/updater/update_service_unittest.cc b/extensions/browser/updater/update_service_unittest.cc
index 4db5545..49e381e 100644
--- a/extensions/browser/updater/update_service_unittest.cc
+++ b/extensions/browser/updater/update_service_unittest.cc
@@ -151,14 +151,12 @@
class UpdateServiceTest : public ExtensionsTest {
public:
- UpdateServiceTest() {}
+ UpdateServiceTest()
+ : ExtensionsTest(base::MakeUnique<content::TestBrowserThreadBundle>()) {}
~UpdateServiceTest() override {}
void SetUp() override {
ExtensionsTest::SetUp();
- browser_threads_.reset(new content::TestBrowserThreadBundle(
- content::TestBrowserThreadBundle::DEFAULT));
-
extensions_browser_client()->set_extension_system_factory(
&fake_extension_system_factory_);
extensions_browser_client()->SetUpdateClientFactory(base::Bind(
@@ -199,7 +197,6 @@
private:
UpdateService* update_service_;
scoped_refptr<FakeUpdateClient> update_client_;
- std::unique_ptr<content::TestBrowserThreadBundle> browser_threads_;
MockExtensionSystemFactory<FakeExtensionSystem>
fake_extension_system_factory_;
};
diff --git a/extensions/browser/warning_service_unittest.cc b/extensions/browser/warning_service_unittest.cc
index 96b6c5d3..1cd3fd9 100644
--- a/extensions/browser/warning_service_unittest.cc
+++ b/extensions/browser/warning_service_unittest.cc
@@ -34,7 +34,11 @@
MOCK_METHOD1(ExtensionWarningsChanged, void(const ExtensionIdSet&));
};
-typedef ExtensionsTest WarningServiceTest;
+class WarningServiceTest : public ExtensionsTest {
+ public:
+ WarningServiceTest()
+ : ExtensionsTest(base::MakeUnique<content::TestBrowserThreadBundle>()) {}
+};
const char ext1_id[] = "extension1";
const char ext2_id[] = "extension2";
@@ -46,7 +50,6 @@
// Check that inserting a warning triggers notifications, whereas inserting
// the same warning again is silent.
TEST_F(WarningServiceTest, SetWarning) {
- content::TestBrowserThreadBundle thread_bundle_;
content::TestBrowserContext browser_context;
TestWarningService warning_service(&browser_context);
MockObserver observer;
@@ -70,7 +73,6 @@
// Check that ClearWarnings deletes exactly the specified warnings and
// triggers notifications where appropriate.
TEST_F(WarningServiceTest, ClearWarnings) {
- content::TestBrowserThreadBundle thread_bundle_;
content::TestBrowserContext browser_context;
TestWarningService warning_service(&browser_context);
MockObserver observer;
diff --git a/extensions/shell/browser/api/identity/identity_api_unittest.cc b/extensions/shell/browser/api/identity/identity_api_unittest.cc
index 122b979..342e831 100644
--- a/extensions/shell/browser/api/identity/identity_api_unittest.cc
+++ b/extensions/shell/browser/api/identity/identity_api_unittest.cc
@@ -9,7 +9,6 @@
#include <utility>
#include "base/values.h"
-#include "content/public/test/test_browser_thread_bundle.h"
#include "extensions/browser/api_unittest.h"
#include "extensions/common/extension_builder.h"
#include "extensions/shell/browser/shell_oauth2_token_service.h"
diff --git a/extensions/shell/browser/shell_native_app_window_aura_unittest.cc b/extensions/shell/browser/shell_native_app_window_aura_unittest.cc
index 1a4a1dd..288e1b4 100644
--- a/extensions/shell/browser/shell_native_app_window_aura_unittest.cc
+++ b/extensions/shell/browser/shell_native_app_window_aura_unittest.cc
@@ -25,14 +25,14 @@
class ShellNativeAppWindowAuraTest : public ExtensionsTest {
public:
- ShellNativeAppWindowAuraTest() {
+ ShellNativeAppWindowAuraTest()
+ : ExtensionsTest(base::MakeUnique<content::TestBrowserThreadBundle>()) {
AppWindowClient::Set(&app_window_client_);
}
~ShellNativeAppWindowAuraTest() override {}
protected:
- content::TestBrowserThreadBundle thread_bundle_;
ShellAppWindowClient app_window_client_;
};
diff --git a/extensions/shell/browser/shell_oauth2_token_service_unittest.cc b/extensions/shell/browser/shell_oauth2_token_service_unittest.cc
index 649fd3ab..75a90edb 100644
--- a/extensions/shell/browser/shell_oauth2_token_service_unittest.cc
+++ b/extensions/shell/browser/shell_oauth2_token_service_unittest.cc
@@ -4,6 +4,7 @@
#include "extensions/shell/browser/shell_oauth2_token_service.h"
+#include "base/memory/ptr_util.h"
#include "content/public/test/test_browser_thread_bundle.h"
#include "extensions/browser/extensions_test.h"
@@ -11,11 +12,9 @@
class ShellOAuth2TokenServiceTest : public ExtensionsTest {
public:
- ShellOAuth2TokenServiceTest() {}
+ ShellOAuth2TokenServiceTest()
+ : ExtensionsTest(base::MakeUnique<content::TestBrowserThreadBundle>()) {}
~ShellOAuth2TokenServiceTest() override {}
-
- private:
- content::TestBrowserThreadBundle thread_bundle_;
};
// Verifies setting the refresh token makes it available.
diff --git a/services/ui/gpu/gpu_main.cc b/services/ui/gpu/gpu_main.cc
index cb42d4df..1acc72a 100644
--- a/services/ui/gpu/gpu_main.cc
+++ b/services/ui/gpu/gpu_main.cc
@@ -208,7 +208,8 @@
frame_sink_manager_ = base::MakeUnique<viz::MojoFrameSinkManager>(
true, display_provider_.get());
- frame_sink_manager_->Connect(std::move(request), std::move(client));
+ frame_sink_manager_->BindPtrAndSetClient(std::move(request),
+ std::move(client));
}
void GpuMain::TearDownOnCompositorThread() {
diff --git a/ui/aura/demo/demo_main.cc b/ui/aura/demo/demo_main.cc
index 74b131b..b3bdf81 100644
--- a/ui/aura/demo/demo_main.cc
+++ b/ui/aura/demo/demo_main.cc
@@ -139,8 +139,9 @@
// The ContextFactory must exist before any Compositors are created.
viz::FrameSinkManagerHost frame_sink_manager;
- auto context_factory =
- base::MakeUnique<ui::InProcessContextFactory>(&frame_sink_manager);
+ cc::SurfaceManager surface_manager;
+ auto context_factory = base::MakeUnique<ui::InProcessContextFactory>(
+ &frame_sink_manager, &surface_manager);
context_factory->set_use_test_surface(false);
// Create the message-loop here before creating the root window.
diff --git a/ui/compositor/test/context_factories_for_test.cc b/ui/compositor/test/context_factories_for_test.cc
index d1b18c8..e6e320d 100644
--- a/ui/compositor/test/context_factories_for_test.cc
+++ b/ui/compositor/test/context_factories_for_test.cc
@@ -15,8 +15,9 @@
namespace {
static viz::FrameSinkManagerHost* g_frame_sink_manager = nullptr;
-static ui::InProcessContextFactory* g_implicit_factory = NULL;
-static gl::DisableNullDrawGLBindings* g_disable_null_draw = NULL;
+static cc::SurfaceManager* g_surface_manager = nullptr;
+static ui::InProcessContextFactory* g_implicit_factory = nullptr;
+static gl::DisableNullDrawGLBindings* g_disable_null_draw = nullptr;
} // namespace
@@ -35,7 +36,9 @@
if (enable_pixel_output)
g_disable_null_draw = new gl::DisableNullDrawGLBindings;
g_frame_sink_manager = new viz::FrameSinkManagerHost;
- g_implicit_factory = new InProcessContextFactory(g_frame_sink_manager);
+ g_surface_manager = new cc::SurfaceManager;
+ g_implicit_factory =
+ new InProcessContextFactory(g_frame_sink_manager, g_surface_manager);
g_implicit_factory->SetUseFastRefreshRateForTests();
*context_factory = g_implicit_factory;
*context_factory_private = g_implicit_factory;
@@ -45,12 +48,14 @@
if (g_implicit_factory) {
g_implicit_factory->SendOnLostResources();
delete g_implicit_factory;
- g_implicit_factory = NULL;
+ g_implicit_factory = nullptr;
}
+ delete g_surface_manager;
+ g_surface_manager = nullptr;
delete g_frame_sink_manager;
g_frame_sink_manager = nullptr;
delete g_disable_null_draw;
- g_disable_null_draw = NULL;
+ g_disable_null_draw = nullptr;
}
} // namespace ui
diff --git a/ui/compositor/test/in_process_context_factory.cc b/ui/compositor/test/in_process_context_factory.cc
index 325e13f..ec253793 100644
--- a/ui/compositor/test/in_process_context_factory.cc
+++ b/ui/compositor/test/in_process_context_factory.cc
@@ -135,10 +135,12 @@
};
InProcessContextFactory::InProcessContextFactory(
- viz::FrameSinkManagerHost* frame_sink_manager)
+ viz::FrameSinkManagerHost* frame_sink_manager,
+ cc::SurfaceManager* surface_manager)
: frame_sink_id_allocator_(kDefaultClientId),
use_test_surface_(true),
- frame_sink_manager_(frame_sink_manager) {
+ frame_sink_manager_(frame_sink_manager),
+ surface_manager_(surface_manager) {
DCHECK(frame_sink_manager);
DCHECK_NE(gl::GetGLImplementation(), gl::kGLImplementationNone)
<< "If running tests, ensure that main() is calling "
@@ -316,7 +318,7 @@
}
cc::SurfaceManager* InProcessContextFactory::GetSurfaceManager() {
- return frame_sink_manager_->surface_manager();
+ return surface_manager_;
}
viz::FrameSinkManagerHost* InProcessContextFactory::GetFrameSinkManagerHost() {
diff --git a/ui/compositor/test/in_process_context_factory.h b/ui/compositor/test/in_process_context_factory.h
index c8595878..3fbdbf5a 100644
--- a/ui/compositor/test/in_process_context_factory.h
+++ b/ui/compositor/test/in_process_context_factory.h
@@ -33,9 +33,12 @@
class InProcessContextFactory : public ContextFactory,
public ContextFactoryPrivate {
public:
- // surface_manager is owned by the creator of this and must outlive the
- // context factory.
- explicit InProcessContextFactory(viz::FrameSinkManagerHost* manager);
+ // Both |frame_sink_manager_host| and |surface_manager| must outlive the
+ // ContextFactory.
+ // TODO(crbug.com/657959): |surface_manager| should go away and we should use
+ // the CompositorFrameSink from the FrameSinkManagerHost.
+ InProcessContextFactory(viz::FrameSinkManagerHost* frame_sink_manager_host,
+ cc::SurfaceManager* surface_manager);
~InProcessContextFactory() override;
// If true (the default) an OutputSurface is created that does not display
@@ -99,6 +102,7 @@
bool use_test_surface_;
double refresh_rate_ = 60.0;
viz::FrameSinkManagerHost* frame_sink_manager_;
+ cc::SurfaceManager* surface_manager_;
base::ObserverList<ContextFactoryObserver> observer_list_;
cc::RendererSettings renderer_settings_;
diff --git a/ui/views/examples/examples_main.cc b/ui/views/examples/examples_main.cc
index 7c44a31c..5138585 100644
--- a/ui/views/examples/examples_main.cc
+++ b/ui/views/examples/examples_main.cc
@@ -68,8 +68,9 @@
// The ContextFactory must exist before any Compositors are created.
viz::FrameSinkManagerHost frame_sink_manager_;
- auto context_factory =
- base::MakeUnique<ui::InProcessContextFactory>(&frame_sink_manager_);
+ cc::SurfaceManager surface_manager_;
+ auto context_factory = base::MakeUnique<ui::InProcessContextFactory>(
+ &frame_sink_manager_, &surface_manager_);
context_factory->set_use_test_surface(false);
base::MessageLoopForUI message_loop;