aw: Track UI request GL separately from other threads

aw: Track UI request GL separately from other threads

Track UI and other threads differently in requesting DrawGL. The request
itself is posted through UI thread, so not tracking them separately can
lead to deadlocks. See bug for more detail.

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

[boliu, 2014-08-05 22:29:24]

Tested that fix works

[hush (inactive), 2014-08-05 23:04:39]

https://codereview.chromium.org/431383003/diff/1/android_webview/browser/defe...
File android_webview/browser/deferred_gpu_command_service.cc (right):

https://codereview.chromium.org/431383003/diff/1/android_webview/browser/defe...
android_webview/browser/deferred_gpu_command_service.cc:127: bool on_ui_thread =
renderer_state->CurrentlyOnUIThread();
on_ui_thread only means render state was running on UI thread during that point
of time. But it does not mean renderer state won't switch threads, nor does it
mean UI is blocked on a request to RT.

DeferredGpuCommandService::RequestProcessGL can be running on any thread (blink
main thread, UI thread, and render thread)
Suppose DeferredGpuCommandService::RequestProcessGL is running on blink main
thread now, and right after this line, render state hops from UI thread to
Render Thread. 

Will this be a problem?

https://codereview.chromium.org/431383003/diff/1/android_webview/browser/defe...
android_webview/browser/deferred_gpu_command_service.cc:133: }
if g_request_pending_on_ui is true, g_request_pending is definitely true.
Right?
If so, we can just set g_request_pending regardless, and treat
g_request_pending_on_ui separately

[boliu, 2014-08-05 23:07:55]

https://codereview.chromium.org/431383003/diff/1/android_webview/browser/defe...
File android_webview/browser/deferred_gpu_command_service.cc (right):

https://codereview.chromium.org/431383003/diff/1/android_webview/browser/defe...
android_webview/browser/deferred_gpu_command_service.cc:127: bool on_ui_thread =
renderer_state->CurrentlyOnUIThread();
On 2014/08/05 23:04:39, hush wrote:
> on_ui_thread only means render state was running on UI thread during that
point
> of time. But it does not mean renderer state won't switch threads, nor does it
> mean UI is blocked on a request to RT.
> 
> DeferredGpuCommandService::RequestProcessGL can be running on any thread
(blink
> main thread, UI thread, and render thread)
> Suppose DeferredGpuCommandService::RequestProcessGL is running on blink main
> thread now, and right after this line, render state hops from UI thread to
> Render Thread. 
> 
> Will this be a problem?

That's how things work today, before or after this CL. What problem are you
talking about?

https://codereview.chromium.org/431383003/diff/1/android_webview/browser/defe...
android_webview/browser/deferred_gpu_command_service.cc:133: }
On 2014/08/05 23:04:39, hush wrote:
> if g_request_pending_on_ui is true, g_request_pending is definitely true.
> Right?

Yep, that's what I wanted to do here.

> If so, we can just set g_request_pending regardless, and treat
> g_request_pending_on_ui separately

You mean remove if (on_ui_thread)? Yeah that should work.

[hush (inactive), 2014-08-05 23:55:15]

lgtm

https://codereview.chromium.org/431383003/diff/1/android_webview/browser/defe...
File android_webview/browser/deferred_gpu_command_service.cc (right):

https://codereview.chromium.org/431383003/diff/1/android_webview/browser/defe...
android_webview/browser/deferred_gpu_command_service.cc:133: }
yes. if on_ui_thread is false, you will set g_request_pending in line 129. If it
is true, you will do that line 132.
On 2014/08/05 23:07:55, boliu wrote:
> On 2014/08/05 23:04:39, hush wrote:
> > if g_request_pending_on_ui is true, g_request_pending is definitely true.
> > Right?
> 
> Yep, that's what I wanted to do here.
> 
> > If so, we can just set g_request_pending regardless, and treat
> > g_request_pending_on_ui separately
> 
> You mean remove if (on_ui_thread)? Yeah that should work.

[boliu, 2014-08-06 00:05:46]

Also slipped in TRACE_EVENT for PerformIdleWork

https://codereview.chromium.org/431383003/diff/1/android_webview/browser/defe...
File android_webview/browser/deferred_gpu_command_service.cc (right):

https://codereview.chromium.org/431383003/diff/1/android_webview/browser/defe...
android_webview/browser/deferred_gpu_command_service.cc:133: }
On 2014/08/05 23:55:15, hush wrote:
> yes. if on_ui_thread is false, you will set g_request_pending in line 129. If
it
> is true, you will do that line 132.
> On 2014/08/05 23:07:55, boliu wrote:
> > On 2014/08/05 23:04:39, hush wrote:
> > > if g_request_pending_on_ui is true, g_request_pending is definitely true.
> > > Right?
> > 
> > Yep, that's what I wanted to do here.
> > 
> > > If so, we can just set g_request_pending regardless, and treat
> > > g_request_pending_on_ui separately
> > 
> > You mean remove if (on_ui_thread)? Yeah that should work.
> 

Done.

[boliu, 2014-08-06 00:08:02]

The CQ bit was checked by boliu@chromium.org

[commit-bot: I haz the power, 2014-08-06 00:09:24]

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

[commit-bot: I haz the power, 2014-08-06 06:45:21]

Change committed as 287713