Commit 7f1241d2 authored by Lucas Furukawa Gadani's avatar Lucas Furukawa Gadani Committed by Commit Bot

Fix focus after an interstitial is destroyed.

When an interstitial is created, it steals the page's focus. When it is
destroyed, it is supposed to return focus to the page, however, since
the WebContents still thinks an interstitial is being displayed, the
RenderWidgetHostImpl tries to maintain the focus on the interstitial.
This CL fixes that by moving the Focus() call into
WebContentsImpl::DetachInterstitialPage, instead of being called in
InterstitialPageImpl::Hide.

Bug: 739676
Change-Id: I9eff1af3cb1500cc3f38400ae9e13e553bb0cf54
Reviewed-on: https://chromium-review.googlesource.com/565172Reviewed-by: default avatarCharlie Reis <creis@chromium.org>
Reviewed-by: default avatarAlex Moshchuk <alexmos@chromium.org>
Commit-Queue: Lucas Gadani <lfg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485782}
parent fe2a747f
......@@ -284,27 +284,17 @@ void InterstitialPageImpl::Hide() {
old_view->Show();
}
// If the focus was on the interstitial, let's keep it to the page.
// (Note that in unit-tests the RVH may not have a view).
if (render_view_host_->GetWidget()->GetView() &&
render_view_host_->GetWidget()->GetView()->HasFocus() &&
controller_->delegate()->GetRenderViewHost()->GetWidget()->GetView()) {
controller_->delegate()
->GetRenderViewHost()
->GetWidget()
->GetView()
->Focus();
}
// Delete this and call Shutdown on the RVH asynchronously, as we may have
// been called from a RVH delegate method, and we can't delete the RVH out
// from under itself.
base::ThreadTaskRunnerHandle::Get()->PostNonNestableTask(
FROM_HERE, base::Bind(&InterstitialPageImpl::Shutdown,
weak_ptr_factory_.GetWeakPtr()));
bool has_focus = render_view_host_->GetWidget()->GetView() &&
render_view_host_->GetWidget()->GetView()->HasFocus();
render_view_host_ = NULL;
frame_tree_->root()->ResetForNewProcess();
controller_->delegate()->DetachInterstitialPage();
controller_->delegate()->DetachInterstitialPage(has_focus);
// Let's revert to the original title if necessary.
NavigationEntry* entry = controller_->GetVisibleEntry();
if (entry && !new_navigation_ && should_revert_web_contents_title_)
......
......@@ -169,6 +169,8 @@ class InterstitialPageImplTest : public ContentBrowserTest {
interstitial_.reset();
}
InterstitialPageImpl* interstitial() { return interstitial_.get(); }
bool FocusInputAndSelectText() {
return ExecuteScript(interstitial_->GetMainFrame(), "focus_select_input()");
}
......@@ -315,6 +317,34 @@ IN_PROC_BROWSER_TEST_F(InterstitialPageImplTest, SelectAll) {
TearDownInterstitialPage();
}
IN_PROC_BROWSER_TEST_F(InterstitialPageImplTest, FocusAfterDetaching) {
WebContentsImpl* web_contents =
static_cast<WebContentsImpl*>(shell()->web_contents());
// Load something into the WebContents.
EXPECT_TRUE(NavigateToURL(shell(), GURL("about:blank")));
// Blur the main frame.
web_contents->GetMainFrame()->GetRenderWidgetHost()->Blur();
EXPECT_FALSE(
web_contents->GetMainFrame()->GetRenderWidgetHost()->is_focused());
// Setup the interstitial and focus it.
SetUpInterstitialPage();
interstitial()->GetView()->GetRenderWidgetHost()->Focus();
EXPECT_TRUE(web_contents->ShowingInterstitialPage());
EXPECT_TRUE(static_cast<RenderWidgetHostImpl*>(
interstitial()->GetView()->GetRenderWidgetHost())
->is_focused());
// Tear down interstitial.
TearDownInterstitialPage();
// Since the interstitial was focused, the main frame should be now focused
// after the interstitial teardown.
EXPECT_TRUE(web_contents->GetRenderViewHost()->GetWidget()->is_focused());
}
// Ensure that we don't show the underlying RenderWidgetHostView if a subframe
// commits in the original page while an interstitial is showing.
// See https://crbug.com/729105.
......
......@@ -61,7 +61,7 @@ class NavigationControllerDelegate {
virtual void AttachInterstitialPage(
InterstitialPageImpl* interstitial_page) = 0;
virtual void DidProceedOnInterstitial() = 0;
virtual void DetachInterstitialPage() = 0;
virtual void DetachInterstitialPage(bool has_focus) = 0;
virtual void UpdateOverridingUserAgent() = 0;
};
......
......@@ -2919,11 +2919,17 @@ void WebContentsImpl::DidProceedOnInterstitial() {
LoadingStateChanged(true, true, nullptr);
}
void WebContentsImpl::DetachInterstitialPage() {
void WebContentsImpl::DetachInterstitialPage(bool has_focus) {
bool interstitial_pausing_throbber =
ShowingInterstitialPage() && interstitial_page_->pause_throbber();
if (ShowingInterstitialPage())
interstitial_page_ = nullptr;
// If the focus was on the interstitial, let's keep it to the page.
// (Note that in unit-tests the RVH may not have a view).
if (has_focus && GetRenderViewHost()->GetWidget()->GetView())
GetRenderViewHost()->GetWidget()->GetView()->Focus();
for (auto& observer : observers_)
observer.DidDetachInterstitialPage();
......
......@@ -814,7 +814,7 @@ class CONTENT_EXPORT WebContentsImpl
bool muted);
// Unsets the currently showing interstitial.
void DetachInterstitialPage() override;
void DetachInterstitialPage(bool has_focus) override;
void UpdateOverridingUserAgent() override;
......
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