content: Always allocate GpuMemoryBuffers on UI thread.

content: Always allocate GpuMemoryBuffers on UI thread.

Prior to this change, the browser compositor allocated
buffers on the UI thread while the buffer allocation for
renderers was done on the IO thread.

This moves all allocation to the UI thread, which makes life
easier for GpuMemoryBuffer implementations and avoids
potentially expensive allocation costs on the IO thread.

BUG=

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

[alexst (slow to review), 2014-05-27 22:14:20]

https://codereview.chromium.org/302443008/diff/1/content/browser/renderer_hos...
File content/browser/renderer_host/render_message_filter.cc (right):

https://codereview.chromium.org/302443008/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/render_message_filter.cc:1350: void
RenderMessageFilter::GpuMemoryBufferAllocatedOnUI(
I don't think you need to add this method, Send() already takes care of the
thread hop from UI to IO.

https://code.google.com/p/chromium/codesearch#chromium/src/content/public/bro...

[reveman, 2014-05-28 16:00:46]

PTAL

https://codereview.chromium.org/302443008/diff/1/content/browser/renderer_hos...
File content/browser/renderer_host/render_message_filter.cc (right):

https://codereview.chromium.org/302443008/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/render_message_filter.cc:1350: void
RenderMessageFilter::GpuMemoryBufferAllocatedOnUI(
On 2014/05/27 22:14:20, alexst wrote:
> I don't think you need to add this method, Send() already takes care of the
> thread hop from UI to IO.
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/public/bro...

Good call. I renamed the function below to GpuMemoryBufferAllocatedOnUI to make
it clear that we expect that function to only be called on the UI thread for
now.

[alexst (slow to review), 2014-05-28 16:05:34]

Cool, lgtm!

[reveman, 2014-05-28 16:30:04]

+sky for content/browser/

[sky, 2014-05-28 16:59:55]

On 2014/05/28 16:30:04, reveman wrote:
> +sky for content/browser/

Can you clarify your description. It says:

"This moves all allocation to the UI thread, which makes life
easier for GpuMemoryBuffer implementations and avoids
potentially expensive allocation cost on the UI thread."

How does moving allocations to the UI thread mean the UI thread isn't doing
expensive allocations?

[reveman, 2014-05-28 17:12:53]

On 2014/05/28 16:59:55, sky wrote:
> On 2014/05/28 16:30:04, reveman wrote:
> > +sky for content/browser/
> 
> Can you clarify your description. It says:
> 
> "This moves all allocation to the UI thread, which makes life
> easier for GpuMemoryBuffer implementations and avoids
> potentially expensive allocation cost on the UI thread."
> 
> How does moving allocations to the UI thread mean the UI thread isn't doing
> expensive allocations?

That was a typo. Should have been IO thread. I've updated the description.

[sky, 2014-05-28 18:08:45]

LGTM - thanks!

[sky, 2014-05-28 18:09:02]

Could you also update the description to match?

[reveman, 2014-05-28 18:39:46]

On 2014/05/28 18:09:02, sky wrote:
> Could you also update the description to match?

The description should be correct now.

[reveman, 2014-05-28 18:39:50]

The CQ bit was checked by reveman@chromium.org

[commit-bot: I haz the power, 2014-05-28 18:40:28]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/reveman@chromium.org/302443008/40001

[commit-bot: I haz the power, 2014-05-28 19:16:57]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[commit-bot: I haz the power, 2014-05-28 19:18:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-28 19:18:36]

Try jobs failed on following builders:
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[piman, 2014-05-28 23:15:20]

drive-by: instead of handling this message on the filter and PostTask, you can
simply handle it in RenderProcessHost

[reveman, 2014-05-30 23:41:14]

Moved handling of ChildProcessHostMsg_SyncAllocateGpuMemoryBuffer to
RenderProcessHostImpl.

[alexst (slow to review), 2014-05-31 02:42:54]

lgtm

[reveman, 2014-05-31 17:48:01]

The CQ bit was checked by reveman@chromium.org

[commit-bot: I haz the power, 2014-05-31 18:55:58]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/reveman@chromium.org/302443008/90001

[commit-bot: I haz the power, 2014-05-31 23:39:30]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/7...)
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/19...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-31 23:41:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-31 23:41:03]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)

[reveman, 2014-06-01 19:38:49]

The CQ bit was checked by reveman@chromium.org

[commit-bot: I haz the power, 2014-06-01 19:39:00]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/reveman@chromium.org/302443008/110001

[commit-bot: I haz the power, 2014-06-02 05:40:00]

Change committed as 274168

[piman, 2014-06-02 06:42:51]

lgtm