Commit 81c62e2d authored by hush's avatar hush Committed by Commit bot

Don't schedule more invokeFunctors than necessary.

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.

BUG=442013

Review URL: https://codereview.chromium.org/801923003

Cr-Commit-Position: refs/heads/master@{#308241}
parent 031afab6
...@@ -23,8 +23,8 @@ class RequestDrawGLTracker { ...@@ -23,8 +23,8 @@ class RequestDrawGLTracker {
RequestDrawGLTracker(); RequestDrawGLTracker();
bool ShouldRequestOnNonUiThread(SharedRendererState* state); bool ShouldRequestOnNonUiThread(SharedRendererState* state);
bool ShouldRequestOnUiThread(SharedRendererState* state); bool ShouldRequestOnUiThread(SharedRendererState* state);
void DidRequestOnUiThread();
void ResetPending(); void ResetPending();
void SetQueuedFunctorOnUi(SharedRendererState* state);
private: private:
base::Lock lock_; base::Lock lock_;
...@@ -51,6 +51,9 @@ bool RequestDrawGLTracker::ShouldRequestOnUiThread(SharedRendererState* state) { ...@@ -51,6 +51,9 @@ bool RequestDrawGLTracker::ShouldRequestOnUiThread(SharedRendererState* state) {
pending_non_ui_->ResetRequestDrawGLCallback(); pending_non_ui_->ResetRequestDrawGLCallback();
pending_non_ui_ = NULL; 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_) if (pending_ui_)
return false; return false;
pending_ui_ = state; pending_ui_ = state;
...@@ -63,6 +66,14 @@ void RequestDrawGLTracker::ResetPending() { ...@@ -63,6 +66,14 @@ void RequestDrawGLTracker::ResetPending() {
pending_ui_ = NULL; 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 internal
namespace { namespace {
...@@ -125,6 +136,7 @@ void SharedRendererState::ResetRequestDrawGLCallback() { ...@@ -125,6 +136,7 @@ void SharedRendererState::ResetRequestDrawGLCallback() {
void SharedRendererState::ClientRequestDrawGLOnUI() { void SharedRendererState::ClientRequestDrawGLOnUI() {
DCHECK(ui_loop_->BelongsToCurrentThread()); DCHECK(ui_loop_->BelongsToCurrentThread());
ResetRequestDrawGLCallback(); ResetRequestDrawGLCallback();
g_request_draw_gl_tracker.Get().SetQueuedFunctorOnUi(this);
if (!browser_view_renderer_->RequestDrawGL(false)) { if (!browser_view_renderer_->RequestDrawGL(false)) {
g_request_draw_gl_tracker.Get().ResetPending(); g_request_draw_gl_tracker.Get().ResetPending();
LOG(ERROR) << "Failed to request GL process. Deadlock likely"; LOG(ERROR) << "Failed to request GL process. Deadlock likely";
...@@ -267,6 +279,16 @@ void SharedRendererState::DrawGL(AwDrawGLInfo* draw_info) { ...@@ -267,6 +279,16 @@ void SharedRendererState::DrawGL(AwDrawGLInfo* draw_info) {
return; 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) {
DidDrawGLProcess();
}
{ {
GLViewRendererManager* manager = GLViewRendererManager::GetInstance(); GLViewRendererManager* manager = GLViewRendererManager::GetInstance();
base::AutoLock lock(lock_); base::AutoLock lock(lock_);
...@@ -285,16 +307,6 @@ void SharedRendererState::DrawGL(AwDrawGLInfo* draw_info) { ...@@ -285,16 +307,6 @@ void SharedRendererState::DrawGL(AwDrawGLInfo* draw_info) {
LOG(ERROR) << "Received unexpected kModeProcessNoContext"; 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) {
DidDrawGLProcess();
}
if (IsInsideHardwareRelease()) { if (IsInsideHardwareRelease()) {
hardware_renderer_.reset(); hardware_renderer_.reset();
// Flush the idle queue in tear down. // Flush the idle queue in tear down.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment