Defer CGLFlushDrawable until OSX-requested drawRect to avoid spinning when window is obscured.

Defer CGLFlushDrawable until OSX-requested drawRect to avoid spinning when window is obscured.

Sadly, this fix regresses some black flashing bugs:
 -flicker-test2.html occasionally flashes while resizing the window.

I think this regression could be fixed if we always draw with the OpenGL context once the context is created. But that would be a third drawing path and some significant code, so let's consider it in a follow up.

BUG=127709
TEST=open https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/sdk/demos/google/particles/index.html ; open Activity Monitor and observe the chrome process CPU usage ; resize chrome window so it's smaller than the Activity Monitor window (or increase size of Activity Monitor); move Activity Monitor on top of chrome window to fully obscure it ; verify that the CPU usage of chrome does not spike upwards while the chrome window is obscured.

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

[jbates, 2012-05-17 19:55:02]

ptal

[Ken Russell (switch to Gerrit), 2012-05-17 20:28:34]

LGTM as long as it's been tested. One question.

https://chromiumcodereview.appspot.com/10382213/diff/2001/content/browser/ren...
File content/browser/renderer_host/render_widget_host_view_mac.mm (right):

https://chromiumcodereview.appspot.com/10382213/diff/2001/content/browser/ren...
content/browser/renderer_host/render_widget_host_view_mac.mm:373: // them and
the GPU process is waiting.
Is there any issue with this being inside the short-circuit (after the
is_hidden_ test above)? Just making sure this was tested.

[jbates, 2012-05-18 00:31:47]

sky: content/browser/OWNERS
joi: content/port/OWNERS
jam: fyi, this adds a new API to content/port (but you're not an owner there)

https://chromiumcodereview.appspot.com/10382213/diff/2001/content/browser/ren...
File content/browser/renderer_host/render_widget_host_view_mac.mm (right):

https://chromiumcodereview.appspot.com/10382213/diff/2001/content/browser/ren...
content/browser/renderer_host/render_widget_host_view_mac.mm:373: // them and
the GPU process is waiting.
On 2012/05/17 20:28:34, kbr wrote:
> Is there any issue with this being inside the short-circuit (after the
> is_hidden_ test above)? Just making sure this was tested.

The code at line 953 ensures that we only need to Ack during the transition to
is_hidden_ == true. (Swaps will be acked immediately while is_hidden_ is true.)

[Jói, 2012-05-18 07:28:27]

FYI, jam@ is also an owner transitively, via the content/OWNERS file.

Cheers,
Jói

https://chromiumcodereview.appspot.com/10382213/diff/2001/content/port/browse...
File content/port/browser/render_widget_host_view_port.h (right):

https://chromiumcodereview.appspot.com/10382213/diff/2001/content/port/browse...
content/port/browser/render_widget_host_view_port.h:180: virtual void
AboutToWaitForBackingStoreMsg() {}
I'd prefer to leave this pure virtual like all the other methods.  Should this
be within the #if defined(OS_MACOSX) block since you only implement it for Mac?

[sky, 2012-05-18 15:48:06]

I'm not a good reviewer for the mac side. I added Nico.

[Nico, 2012-05-18 16:32:55]

kbr knows mac stuff and he lg'd. This is just waiting for an owners stamp.

[jam, 2012-05-18 16:39:35]

lgtm

[sky, 2012-05-18 16:44:07]

Thanks for the clarification.
LGTM (not that jbates needs it now that John LGTMd it)

On Fri, May 18, 2012 at 9:32 AM,  <thakis@chromium.org> wrote:
> kbr knows mac stuff and he lg'd. This is just waiting for an owners stamp.
>
> https://chromiumcodereview.appspot.com/10382213/

[jbates, 2012-05-21 22:41:59]

https://chromiumcodereview.appspot.com/10382213/diff/2001/content/port/browse...
File content/port/browser/render_widget_host_view_port.h (right):

https://chromiumcodereview.appspot.com/10382213/diff/2001/content/port/browse...
content/port/browser/render_widget_host_view_port.h:180: virtual void
AboutToWaitForBackingStoreMsg() {}
On 2012/05/18 07:28:27, Jói wrote:
> I'd prefer to leave this pure virtual like all the other methods.  Should this
> be within the #if defined(OS_MACOSX) block since you only implement it for
Mac?

Done.

[commit-bot: I haz the power, 2012-05-21 22:44:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jbates@chromium.org/10382213/10001

[commit-bot: I haz the power, 2012-05-21 23:06:05]

Try job failure for 10382213-10001 (retry) on mac_rel for step "compile"
(clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-05-22 00:52:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jbates@chromium.org/10382213/2004

[commit-bot: I haz the power, 2012-05-22 04:36:17]

Try job failure for 10382213-2004 (retry) on win_rel for steps "base_unittests,
sync_unit_tests".
It's a second try, previously, steps "base_unittests, browser_tests,
sync_unit_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-05-22 17:57:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jbates@chromium.org/10382213/2004