From 141411bccb8ff5f5d56bf79c8d7f911318a7c086 Mon Sep 17 00:00:00 2001 From: hush Date: Fri, 12 Dec 2014 19:42:33 -0800 Subject: Cherry-pick: Don't schedule more invokeFunctors than necessary. Cherry-pick with conflicts of crrev.com/81c62e2d9601d38c44f71857dfabc31e2726cd70 BUG: 18706908 Original description: The problematic sequence of events is as follows: 1. ShouldRequestOnNonUiThread, which posts a closure (request_draw_gl_closure_) to UI thread 2. That closure gets run on UI thread, and it schedules the invokeFunctor with Android framework 3. Before the corresponding invokeFunctor actually happens on RT (which is DrawGL process mode), ShouldRequestOnUiTdread is called on the UI thread. At this point, pending_non_ui_ is not null, we cancel the callback, which does nothing, because WebView has already scheduled an invokeFunctor with the Android framework in Step 2. Then we schedule another invokeFunctor immediately on the UI thread. So there are 2 invokeFunctors queued in Android framework in this case. This CL tries keep track of whether or not we've queued an invokeFunctor in Android framework already. Change-Id: I0a084d92ea74412c3938645d63170604e6352318 --- android_webview/browser/shared_renderer_state.cc | 14 +++++++++++++- android_webview/native/aw_contents.cc | 20 ++++++++++---------- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/android_webview/browser/shared_renderer_state.cc b/android_webview/browser/shared_renderer_state.cc index 0de8371b1c..b880ce00ad 100644 --- a/android_webview/browser/shared_renderer_state.cc +++ b/android_webview/browser/shared_renderer_state.cc @@ -18,8 +18,8 @@ class RequestDrawGLTracker { RequestDrawGLTracker(); bool ShouldRequestOnNoneUiThread(SharedRendererState* state); bool ShouldRequestOnUiThread(SharedRendererState* state); - void DidRequestOnUiThread(); void ResetPending(); + void SetQueuedFunctorOnUi(SharedRendererState* state); private: base::Lock lock_; @@ -46,6 +46,9 @@ bool RequestDrawGLTracker::ShouldRequestOnUiThread(SharedRendererState* state) { pending_non_ui_->ResetRequestDrawGLCallback(); pending_non_ui_ = NULL; } + // At this time, we could have already called RequestDrawGL on the UI thread, + // but the corresponding GL mode process hasn't happened yet. In this case, + // don't schedule another requestDrawGL on the UI thread. if (pending_ui_) return false; pending_ui_ = state; @@ -58,6 +61,14 @@ void RequestDrawGLTracker::ResetPending() { pending_ui_ = NULL; } +void RequestDrawGLTracker::SetQueuedFunctorOnUi(SharedRendererState* state) { + base::AutoLock lock(lock_); + DCHECK(state); + DCHECK(pending_ui_ == state || pending_non_ui_ == state); + pending_ui_ = state; + pending_non_ui_ = NULL; +} + } // namespace internal namespace { @@ -119,6 +130,7 @@ void SharedRendererState::ResetRequestDrawGLCallback() { void SharedRendererState::ClientRequestDrawGLOnUIThread() { DCHECK(ui_loop_->BelongsToCurrentThread()); ResetRequestDrawGLCallback(); + g_request_draw_gl_tracker.Get().SetQueuedFunctorOnUi(this); if (!client_on_ui_->RequestDrawGL(NULL, false)) { g_request_draw_gl_tracker.Get().ResetPending(); LOG(ERROR) << "Failed to request GL process. Deadlock likely"; diff --git a/android_webview/native/aw_contents.cc b/android_webview/native/aw_contents.cc index 4addcab196..f05bd31106 100644 --- a/android_webview/native/aw_contents.cc +++ b/android_webview/native/aw_contents.cc @@ -353,6 +353,16 @@ void AwContents::DrawGL(AwDrawGLInfo* draw_info) { return; } + // kModeProcessNoContext should never happen because we tear down hardware + // in onTrimMemory. However that guarantee is maintained outside of chromium + // code. Not notifying shared state in kModeProcessNoContext can lead to + // immediate deadlock, which is slightly more catastrophic than leaks or + // corruption. + if (draw_info->mode == AwDrawGLInfo::kModeProcess || + draw_info->mode == AwDrawGLInfo::kModeProcessNoContext) { + shared_renderer_state_.DidDrawGLProcess(); + } + { GLViewRendererManager* manager = GLViewRendererManager::GetInstance(); base::AutoLock lock(render_thread_lock_); @@ -371,16 +381,6 @@ void AwContents::DrawGL(AwDrawGLInfo* draw_info) { LOG(ERROR) << "Received unexpected kModeProcessNoContext"; } - // kModeProcessNoContext should never happen because we tear down hardware - // in onTrimMemory. However that guarantee is maintained outside of chromium - // code. Not notifying shared state in kModeProcessNoContext can lead to - // immediate deadlock, which is slightly more catastrophic than leaks or - // corruption. - if (draw_info->mode == AwDrawGLInfo::kModeProcess || - draw_info->mode == AwDrawGLInfo::kModeProcessNoContext) { - shared_renderer_state_.DidDrawGLProcess(); - } - if (shared_renderer_state_.IsInsideHardwareRelease()) { hardware_renderer_.reset(); // Flush the idle queue in tear down. -- cgit v1.2.3