Commit 4aff0098 authored by John Abd-El-Malek's avatar John Abd-El-Malek Committed by Commit Bot

Fix Android WebView crash when an app loads a JavaScript URL during navigation.

When there's an ongoing navigation, if the app loada a JavaScript URL we create
a temporary NavigationRequest in NavigatorImpl::RequestNavigation. However the
call to RenderFrameHostManager::GetFrameHostForNavigation was resetting the
speculative RFH of the original NavigationRequest since the site instance of a
JavaScript load is always the same as the existing frame.

This is a reland of r523264 with an extra fix to ensure that the RFH is
initialized.

Bug: 793432
Change-Id: I7a029d907f5c133c00ce096d9d733cb8194fcd43
Reviewed-on: https://chromium-review.googlesource.com/822970Reviewed-by: default avatarNasko Oskov <nasko@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523489}
parent 71d3dd99
...@@ -1157,9 +1157,12 @@ void NavigatorImpl::RequestNavigation( ...@@ -1157,9 +1157,12 @@ void NavigatorImpl::RequestNavigation(
// a Javascript URL should not interrupt a previous navigation. // a Javascript URL should not interrupt a previous navigation.
// Note: The scoped_request will be destroyed at the end of this function. // Note: The scoped_request will be destroyed at the end of this function.
if (dest_url.SchemeIs(url::kJavaScriptScheme)) { if (dest_url.SchemeIs(url::kJavaScriptScheme)) {
// Don't call frame_tree_node->render_manager()->GetFrameHostForNavigation
// as that might clear the speculative RFH of an ongoing navigation.
RenderFrameHostImpl* render_frame_host = RenderFrameHostImpl* render_frame_host =
frame_tree_node->render_manager()->GetFrameHostForNavigation( frame_tree_node->current_frame_host();
*scoped_request.get()); frame_tree_node->render_manager()->InitializeRenderFrameIfNecessary(
render_frame_host);
render_frame_host->CommitNavigation( render_frame_host->CommitNavigation(
nullptr, // response nullptr, // response
mojom::URLLoaderClientEndpointsPtr(), mojom::URLLoaderClientEndpointsPtr(),
......
...@@ -1267,6 +1267,36 @@ RenderFrameHostManager::GetSiteInstanceForNavigation( ...@@ -1267,6 +1267,36 @@ RenderFrameHostManager::GetSiteInstanceForNavigation(
return new_instance; return new_instance;
} }
void RenderFrameHostManager::InitializeRenderFrameIfNecessary(
RenderFrameHostImpl* render_frame_host) {
// TODO: this copies some logic inside GetFrameHostForNavigation, which also
// duplicates logic in Navigate. They should all use this method, but that
// involves slight reordering.
if (render_frame_host->IsRenderFrameLive())
return;
if (!ReinitializeRenderFrame(render_frame_host))
return;
if (render_frame_host != render_frame_host_.get())
return;
EnsureRenderFrameHostVisibilityConsistent();
// TODO: uncomment this when the method is shared. Not adding the call now
// to make merge to 63 easier.
// EnsureRenderFrameHostPageFocusConsistent();
// TODO(nasko): This is a very ugly hack. The Chrome extensions process
// manager still uses NotificationService and expects to see a
// RenderViewHost changed notification after WebContents and
// RenderFrameHostManager are completely initialized. This should be
// removed once the process manager moves away from NotificationService.
// See https://crbug.com/462682.
delegate_->NotifyMainFrameSwappedFromRenderManager(
nullptr, render_frame_host_->render_view_host());
}
RenderFrameHostManager::SiteInstanceDescriptor RenderFrameHostManager::SiteInstanceDescriptor
RenderFrameHostManager::DetermineSiteInstanceForURL( RenderFrameHostManager::DetermineSiteInstanceForURL(
const GURL& dest_url, const GURL& dest_url,
......
...@@ -521,6 +521,9 @@ class CONTENT_EXPORT RenderFrameHostManager ...@@ -521,6 +521,9 @@ class CONTENT_EXPORT RenderFrameHostManager
scoped_refptr<SiteInstance> GetSiteInstanceForNavigationRequest( scoped_refptr<SiteInstance> GetSiteInstanceForNavigationRequest(
const NavigationRequest& navigation_request); const NavigationRequest& navigation_request);
// Helper to initialize the RenderFrame if it's not initialized.
void InitializeRenderFrameIfNecessary(RenderFrameHostImpl* render_frame_host);
private: private:
friend class NavigatorTestWithBrowserSideNavigation; friend class NavigatorTestWithBrowserSideNavigation;
friend class RenderFrameHostManagerTest; friend class RenderFrameHostManagerTest;
......
...@@ -211,7 +211,7 @@ void WebContentsViewAndroid::Focus() { ...@@ -211,7 +211,7 @@ void WebContentsViewAndroid::Focus() {
RenderWidgetHostViewAndroid* rwhv = GetRenderWidgetHostViewAndroid(); RenderWidgetHostViewAndroid* rwhv = GetRenderWidgetHostViewAndroid();
if (web_contents_->ShowingInterstitialPage()) { if (web_contents_->ShowingInterstitialPage()) {
web_contents_->GetInterstitialPage()->Focus(); web_contents_->GetInterstitialPage()->Focus();
} else { } else if (rwhv) {
rwhv->Focus(); rwhv->Focus();
} }
} }
......
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