Remove CompositorThread dependency on WebThread

Remove CompositorThread dependency on WebThread

content:: creates and manages the compositor thread itself, it doesn't need to
use a WebThread type or use WebCompositorSupport to manage threaded-ness. This
cuts the dependency of content::CompositorThread to just depend on base::Thread.
The message loop is bound to in content::RenderWidgetCompositor.


Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=185945

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=186636

[boliu, 2013-03-04 05:59:26]

https://codereview.chromium.org/12386078/diff/4001/content/renderer/render_th...
File content/renderer/render_thread_impl.cc (left):

https://codereview.chromium.org/12386078/diff/4001/content/renderer/render_th...
content/renderer/render_thread_impl.cc:610:
compositor_support->initialize(compositor_thread_->GetWebThread());
WebCompositorSupport::initialize is no longer called. Is it not needed anymore?

[jamesr, 2013-03-04 06:02:46]

On 2013/03/04 05:59:26, boliu wrote:
>
https://codereview.chromium.org/12386078/diff/4001/content/renderer/render_th...
> File content/renderer/render_thread_impl.cc (left):
> 
>
https://codereview.chromium.org/12386078/diff/4001/content/renderer/render_th...
> content/renderer/render_thread_impl.cc:610:
> compositor_support->initialize(compositor_thread_->GetWebThread());
> WebCompositorSupport::initialize is no longer called. Is it not needed
anymore?

It's not called or depend on by anything in content::.  It's still relied upon
by a few things in webkit_support that are used to run webkit_unit_tests and
DumpRenderTree.  I'm removing that (but it'll take a WebKit roll) after which
WebCompositorSupport::initialize/shutdown can go away.

[jamesr, 2013-03-04 06:16:38]

On 2013/03/04 06:02:46, jamesr wrote:
> On 2013/03/04 05:59:26, boliu wrote:
> >
>
https://codereview.chromium.org/12386078/diff/4001/content/renderer/render_th...
> > File content/renderer/render_thread_impl.cc (left):
> > 
> >
>
https://codereview.chromium.org/12386078/diff/4001/content/renderer/render_th...
> > content/renderer/render_thread_impl.cc:610:
> > compositor_support->initialize(compositor_thread_->GetWebThread());
> > WebCompositorSupport::initialize is no longer called. Is it not needed
> anymore?
> 
> It's not called or depend on by anything in content::.  It's still relied upon
> by a few things in webkit_support that are used to run webkit_unit_tests and
> DumpRenderTree.  I'm removing that (but it'll take a WebKit roll) after which
> WebCompositorSupport::initialize/shutdown can go away.

First part of that up here: https://codereview.chromium.org/12385074/

[piman, 2013-03-04 17:11:48]

lgtm

[commit-bot: I haz the power, 2013-03-04 18:25:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/12386078/4001

[commit-bot: I haz the power, 2013-03-04 19:39:03]

Change committed as 185945

[jamesr, 2013-03-04 23:43:22]

Reverted in r185987 due to performance issues on some of the windows page
cyclers.  See
http://chromium-perf.appspot.com/?tab=chromium-rel-win7-single&graph=times&re...

This shouldn't have changed behavior at all, so I busterated something.  Will
investigate.

[jamesr, 2013-03-05 00:25:10]

On 2013/03/04 23:43:22, jamesr wrote:
> Reverted in r185987 due to performance issues on some of the windows page
> cyclers.  See
>
http://chromium-perf.appspot.com/?tab=chromium-rel-win7-single&graph=times&re...
> 
> This shouldn't have changed behavior at all, so I busterated something.  Will
> investigate.

Aha, by not calling WebCompositorSupport::initialize I failed to set the value
of WebCompositorSupport::isThreadingEnabled() which is exposed to WebKit and
tested in several places.

[jamesr, 2013-03-05 01:54:32]

Reopening since this'll have to be relanded after
https://codereview.chromium.org/12378094/ +
https://bugs.webkit.org/show_bug.cgi?id=111386 land and roll into the tree.

[jamesr, 2013-03-05 23:48:25]

On 2013/03/05 01:54:32, jamesr wrote:
> Reopening since this'll have to be relanded after
> https://codereview.chromium.org/12378094/ +
> https://bugs.webkit.org/show_bug.cgi?id=111386 land and roll into the tree.

WebKit side landed in r144827, now just waiting for a WebKit roll past that to
land in Chromium.

[boliu, 2013-03-06 18:22:46]

On 2013/03/05 23:48:25, jamesr wrote:
> WebKit side landed in r144827, now just waiting for a WebKit roll past that to
> land in Chromium.

Webkit rolled past 144827. Land this again pretty please?

[commit-bot: I haz the power, 2013-03-06 19:28:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/12386078/4001

[commit-bot: I haz the power, 2013-03-06 19:29:01]

Failed to apply patch for content/renderer/render_thread_impl.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file content/renderer/render_thread_impl.cc
  Hunk #1 FAILED at 322.
  Hunk #2 succeeded at 411 (offset 2 lines).
  Hunk #3 succeeded at 597 (offset 2 lines).
  Hunk #4 succeeded at 879 (offset 2 lines).
  1 out of 4 hunks FAILED -- saving rejects to file
content/renderer/render_thread_impl.cc.rej

Patch:       content/renderer/render_thread_impl.cc
Index: content/renderer/render_thread_impl.cc
diff --git a/content/renderer/render_thread_impl.cc
b/content/renderer/render_thread_impl.cc
index
d9eaf1821ffd299526494982b863ca3751ceea3a..bee6fc1d541aba05d48e312afb90653f87da6567
100644
--- a/content/renderer/render_thread_impl.cc
+++ b/content/renderer/render_thread_impl.cc
@@ -322,7 +322,6 @@ void RenderThreadImpl::Init() {
   idle_notification_delay_in_ms_ = kInitialIdleHandlerDelayMs;
   idle_notifications_to_skip_ = 0;
   require_user_gesture_for_focus_ = true;
-  compositor_initialized_ = false;
 
   appcache_dispatcher_.reset(new AppCacheDispatcher(Get()));
   dom_storage_dispatcher_.reset(new DomStorageDispatcher());
@@ -409,10 +408,6 @@ RenderThreadImpl::~RenderThreadImpl() {
     compositor_output_surface_filter_ = NULL;
   }
 
-  if (compositor_initialized_) {
-    WebKit::Platform::current()->compositorSupport()->shutdown();
-    compositor_initialized_ = false;
-  }
   if (compositor_thread_.get()) {
     RemoveFilter(compositor_thread_->GetMessageFilter());
     compositor_thread_.reset();
@@ -599,26 +594,22 @@ void RenderThreadImpl::EnsureWebKitInitialized() {
   WebKit::setIDBFactory(
       webkit_platform_support_.get()->idbFactory());
 
-  WebKit::WebCompositorSupport* compositor_support =
-      WebKit::Platform::current()->compositorSupport();
   const CommandLine& command_line = *CommandLine::ForCurrentProcess();
 
   bool enable = command_line.HasSwitch(switches::kEnableThreadedCompositing);
   if (enable) {
     compositor_thread_.reset(new CompositorThread(this));
     AddFilter(compositor_thread_->GetMessageFilter());
-    compositor_support->initialize(compositor_thread_->GetWebThread());
-  } else {
-    compositor_support->initialize(NULL);
   }
-  compositor_initialized_ = true;
 
-  MessageLoop* output_surface_loop = enable ?
-      compositor_thread_->message_loop() :
-      MessageLoop::current();
+  base::MessageLoopProxy* output_surface_loop;
+  if (enable)
+    output_surface_loop = compositor_thread_->message_loop_proxy();
+  else
+    output_surface_loop = base::MessageLoopProxy::current();
 
-  compositor_output_surface_filter_ = CompositorOutputSurface::CreateFilter(
-      output_surface_loop->message_loop_proxy());
+  compositor_output_surface_filter_ =
+      CompositorOutputSurface::CreateFilter(output_surface_loop);
   AddFilter(compositor_output_surface_filter_.get());
 
   WebScriptController::enableV8SingleThreadMode();
@@ -885,7 +876,7 @@ void RenderThreadImpl::OnGpuVDAContextLoss() {
   if (!self->gpu_vda_context3d_.get())
     return;
   if (self->compositor_thread()) {
-    self->compositor_thread()->GetWebThread()->message_loop()->DeleteSoon(
+    self->compositor_thread()->message_loop_proxy()->DeleteSoon(
         FROM_HERE, self->gpu_vda_context3d_.release());
   } else {
     self->gpu_vda_context3d_.reset();

[commit-bot: I haz the power, 2013-03-06 19:55:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/12386078/18001

[commit-bot: I haz the power, 2013-03-06 22:14:39]

Retried try job too often on win_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-03-07 04:06:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/12386078/18001

[commit-bot: I haz the power, 2013-03-07 06:07:30]

Change committed as 186636