Add a suppression for GLHelper blocking the UI thread.

Add a suppression for GLHelper blocking the UI thread.

BUG=125415
TEST=Manual


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=134759

[mazda, 2012-04-27 22:52:54]

Could you review this CL?

It's very rare that this actually blocks the UI thread because
WaitableEvent::Wait runs only when the last instance of GLHelper is deleted and
the GLHelper task is running then.
I'd like to suppress it for now.

Thanks,

[piman, 2012-04-27 23:15:11]

On 2012/04/27 22:52:54, mazda wrote:
> Could you review this CL?
> 
> It's very rare that this actually blocks the UI thread because
> WaitableEvent::Wait runs only when the last instance of GLHelper is deleted
and
> the GLHelper task is running then.
> I'd like to suppress it for now.
> 
> Thanks,

Actually I just found out that it can be quite common.
A common case to generate the thumbnail is when we close the tab. If we do that,
then it will create the GLHelper, kick the thumbnail, and immediately destroy
the GLHelper. If there's no other GLHelper, it will block then.
On top of that it seems to expose a threading issue that I'm looking more deeply
into (ReadBackFramebufferComplete may run after GLHelper is destroyed, but
access its fields).

[mazda, 2012-04-30 17:15:03]

On 2012/04/27 23:15:11, piman wrote:
> On 2012/04/27 22:52:54, mazda wrote:
> > Could you review this CL?
> > 
> > It's very rare that this actually blocks the UI thread because
> > WaitableEvent::Wait runs only when the last instance of GLHelper is deleted
> and
> > the GLHelper task is running then.
> > I'd like to suppress it for now.
> > 
> > Thanks,
> 
> Actually I just found out that it can be quite common.
> A common case to generate the thumbnail is when we close the tab. If we do
that,
> then it will create the GLHelper, kick the thumbnail, and immediately destroy
> the GLHelper. If there's no other GLHelper, it will block then.
> On top of that it seems to expose a threading issue that I'm looking more
deeply
> into (ReadBackFramebufferComplete may run after GLHelper is destroyed, but
> access its fields).

Closing tab does not cause the thumbnailer to generate thumbnails (though it
might be better to do so as noted in http://crbug.com/125400). But I found that
instant was making WebContents to show or hide very aggressively, and that leads
to unnecessary GLHelper creations and destructions.
I didn't see this instant behavior before and am checking if this is an intended
behavior.

[mazda, 2012-04-30 22:09:16]

On 2012/04/30 17:15:03, mazda wrote:
> On 2012/04/27 23:15:11, piman wrote:
> > On 2012/04/27 22:52:54, mazda wrote:
> > > Could you review this CL?
> > > 
> > > It's very rare that this actually blocks the UI thread because
> > > WaitableEvent::Wait runs only when the last instance of GLHelper is
deleted
> > and
> > > the GLHelper task is running then.
> > > I'd like to suppress it for now.
> > > 
> > > Thanks,
> > 
> > Actually I just found out that it can be quite common.
> > A common case to generate the thumbnail is when we close the tab. If we do
> that,
> > then it will create the GLHelper, kick the thumbnail, and immediately
destroy
> > the GLHelper. If there's no other GLHelper, it will block then.
> > On top of that it seems to expose a threading issue that I'm looking more
> deeply
> > into (ReadBackFramebufferComplete may run after GLHelper is destroyed, but
> > access its fields).
> 
> Closing tab does not cause the thumbnailer to generate thumbnails (though it
> might be better to do so as noted in http://crbug.com/125400). But I found
that
> instant was making WebContents to show or hide very aggressively, and that
leads
> to unnecessary GLHelper creations and destructions.
> I didn't see this instant behavior before and am checking if this is an
intended
> behavior.

It turned out the frequent destructions of GLHelper was caused by an Instant
issue (http://crbug.com/125620).
I'm thinking of adding code to prevent thumbnailer from trying to generate
thumbnail if the WebContents is for the preview by Instant. Then, it should
become very rare that GLHelper::~GLHelper waits for the completion of reading
back framebuffer.

[piman, 2012-05-01 03:08:12]

LGTM. Let's land this. I'm working on the fixes for the thread safety issues.

[mazda, 2012-05-01 07:27:56]

Brett,
Could you do an OWNERS review for base?

Antoine,
What kind of thread safety issue is happening? Is there any way to reproduce it?

Thanks,

[brettw, 2012-05-01 17:55:54]

lgtm

[commit-bot: I haz the power, 2012-05-01 18:04:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mazda@chromium.org/10254024/1

[commit-bot: I haz the power, 2012-05-01 20:29:40]

Change committed as 134759