Commit f1b469b0 authored by ppi@chromium.org's avatar ppi@chromium.org

Fix for renderer visibility on Android that doesn't break Aura and Mac.

http://crrev.com/287864 fixed an issue where a renderer that was killed in
background and later reloaded was being respawned with incorrect process
visibility, causing it to be killed on Android.

The fix skipped resetting the |is_hidden_| flag of RenderWidgetHost, so that
RenderWidgetHost crashing in background would retain its visibility status and
trigger the ::Show() paths correctly when recreating the View in foreground,
fixing the process visibility issues.

Unfortunetely, this wasn't that good of an idea.

It turns out that while it is the RenderWidgetHost that is the authoritative
source of visibility truth that views rely on, it is the view that is
responsible for providing signals (WasShown / WasHidden) to the widget host.
That means that after a renderer crashes and the view is destroyed,
RenderWidgetHost has no way to track its visibility anymore and all bets are
off.

It turns out that view implementations on Aura and Mac relied on the
RenderWidgetHost visibility being reset upon crash (http://crbug.com/401859).
Hence, this patch undoes the original "fix" and instead ensures that resetting
the |is_hidden_| flag issues a notification for the RenderProcessHost, so that
the process visibility signal that we attempted to fix with the original patch
is still working.

Regression test for http://crbug.com/399521 is in Chrome for Android repo
(BindingManagerIntegrationTest).

BUG=399521
BUG=401859

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

Cr-Commit-Position: refs/heads/master@{#289276}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@289276 0039d316-1c4b-4281-b951-d872f2087c98
parent c65d9ce0
...@@ -1198,6 +1198,18 @@ void RenderWidgetHostImpl::RendererExited(base::TerminationStatus status, ...@@ -1198,6 +1198,18 @@ void RenderWidgetHostImpl::RendererExited(base::TerminationStatus status,
// Reset some fields in preparation for recovering from a crash. // Reset some fields in preparation for recovering from a crash.
ResetSizeAndRepaintPendingFlags(); ResetSizeAndRepaintPendingFlags();
current_size_.SetSize(0, 0); current_size_.SetSize(0, 0);
// After the renderer crashes, the view is destroyed and so the
// RenderWidgetHost cannot track its visibility anymore. We assume such
// RenderWidgetHost to be visible for the sake of internal accounting - be
// careful about changing this - see http://crbug.com/401859.
//
// We need to at least make sure that the RenderProcessHost is notified about
// the |is_hidden_| change, so that the renderer will have correct visibility
// set when respawned.
if (!is_hidden_) {
process_->WidgetRestored();
is_hidden_ = false;
}
// Reset this to ensure the hung renderer mechanism is working properly. // Reset this to ensure the hung renderer mechanism is working properly.
in_flight_event_count_ = 0; in_flight_event_count_ = 0;
......
...@@ -696,7 +696,8 @@ class CONTENT_EXPORT RenderWidgetHostImpl ...@@ -696,7 +696,8 @@ class CONTENT_EXPORT RenderWidgetHostImpl
// Indicates whether a page is loading or not. // Indicates whether a page is loading or not.
bool is_loading_; bool is_loading_;
// Indicates whether a page is hidden or not. // Indicates whether a page is hidden or not. It has to stay in sync with the
// most recent call to process_->WidgetRestored() / WidgetHidden().
bool is_hidden_; bool is_hidden_;
// Indicates whether a page is fullscreen or not. // Indicates whether a page is fullscreen or not.
......
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