Store and use last base URL between DidStart / DidStopLoading

As it turns out that the committed entry may omit the base URL if there has been a javascript: URL navigation after LoadDataWithBaseURL. Thus we have to store the base url on DidStartLoading and use it in DidStopLoading / DidFinishLoad. We have to preserve the old hack with SetToBaseURLForDataURLIfNeeded because if the base URL is not valid, DidFinishLoad doesn't receive it from the renderer. This change is a short-term workaround. The correct solution is to pass a flag that the load was through LoadDataWithBaseURL via Blink, so base URL can be restored correctly within NavigationController. BUG=594001 Review URL: https://codereview.chromium.org/1779363004 Cr-Commit-Position: refs/heads/master@{#381108} (cherry picked from commit ce1f4d0) Review URL: https://codereview.chromium.org/1799973002 . Cr-Commit-Position: refs/branch-heads/2661@{crosswalk-project#228} Cr-Branched-From: ef6f6ae-refs/heads/master@{#378081}
huningxin · Mar 14, 2016 · c278e82 · c278e82
1 parent dc4b45b
commit c278e82
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 1 deletion.
diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/LoadDataWithBaseUrlTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/LoadDataWithBaseUrlTest.java
@@ -373,4 +373,37 @@ public void testLoadLargeData() throws Throwable {
         assertEquals("true", executeJavaScriptAndWaitForResult(mAwContents, mContentsClient,
                         "window.gotToEndOfBody"));
     }
+
+    @SmallTest
+    @Feature({"AndroidWebView"})
+    public void testOnPageFinishedWhenInterrupted() throws Throwable {
+        // See crbug.com/594001 -- when a javascript: URL is loaded, the pending entry
+        // gets discarded and the previous load goes through a different path
+        // inside NavigationController.
+        final String pageHtml = "<html><body>Hello, world!</body></html>";
+        final String baseUrl = "http://example.com/";
+        final TestCallbackHelperContainer.OnPageFinishedHelper onPageFinishedHelper =
+                mContentsClient.getOnPageFinishedHelper();
+        final int callCount = onPageFinishedHelper.getCallCount();
+        loadDataWithBaseUrlAsync(mAwContents, pageHtml, "text/html", false, baseUrl, null);
+        loadUrlAsync(mAwContents, "javascript:42");
+        onPageFinishedHelper.waitForCallback(callCount);
+        assertEquals(baseUrl, onPageFinishedHelper.getUrl());
+    }
+
+    @SmallTest
+    @Feature({"AndroidWebView"})
+    public void testOnPageFinishedWithInvalidBaseUrlWhenInterrupted() throws Throwable {
+        final String pageHtml = CommonResources.ABOUT_HTML;
+        final String invalidBaseUrl = "http://";
+        final TestCallbackHelperContainer.OnPageFinishedHelper onPageFinishedHelper =
+                mContentsClient.getOnPageFinishedHelper();
+        final int callCount = onPageFinishedHelper.getCallCount();
+        getAwSettingsOnUiThread(mAwContents).setJavaScriptEnabled(true);
+        loadDataWithBaseUrlAsync(mAwContents, pageHtml, "text/html", false, invalidBaseUrl, null);
+        loadUrlAsync(mAwContents, "javascript:42");
+        onPageFinishedHelper.waitForCallback(callCount);
+        // Verify that the load succeeds. The actual base url is undefined.
+        assertEquals(CommonResources.ABOUT_TITLE, getTitleOnUiThread(mAwContents));
+    }
 }
diff --git a/content/browser/android/web_contents_observer_proxy.cc b/content/browser/android/web_contents_observer_proxy.cc
@@ -93,6 +93,9 @@ void WebContentsObserverProxy::DidStartLoading() {
   ScopedJavaLocalRef<jobject> obj(java_observer_);
   ScopedJavaLocalRef<jstring> jstring_url(
       ConvertUTF8ToJavaString(env, web_contents()->GetVisibleURL().spec()));
+  if (auto entry = web_contents()->GetController().GetPendingEntry()) {
+    base_url_of_last_started_data_url_ = entry->GetBaseURLForDataURL();
+  }
   Java_WebContentsObserverProxy_didStartLoading(env, obj.obj(),
                                                 jstring_url.obj());
 }
@@ -102,6 +105,8 @@ void WebContentsObserverProxy::DidStopLoading() {
   ScopedJavaLocalRef<jobject> obj(java_observer_);
   std::string url_string = web_contents()->GetLastCommittedURL().spec();
   SetToBaseURLForDataURLIfNeeded(&url_string);
+  // DidStopLoading is the last event we should get.
+  base_url_of_last_started_data_url_ = GURL::EmptyGURL();
   ScopedJavaLocalRef<jstring> jstring_url(ConvertUTF8ToJavaString(
       env, url_string));
   Java_WebContentsObserverProxy_didStopLoading(env, obj.obj(),
@@ -317,8 +322,16 @@ void WebContentsObserverProxy::SetToBaseURLForDataURLIfNeeded(
   NavigationEntry* entry =
       web_contents()->GetController().GetLastCommittedEntry();
   // Note that GetBaseURLForDataURL is only used by the Android WebView.
-  if (entry && !entry->GetBaseURLForDataURL().is_empty())
+  // FIXME: Should we only return valid specs and "about:blank" for invalid
+  // ones? This may break apps.
+  if (entry && !entry->GetBaseURLForDataURL().is_empty()) {
     *url = entry->GetBaseURLForDataURL().possibly_invalid_spec();
+  } else if (!base_url_of_last_started_data_url_.is_empty()) {
+    // NavigationController can lose the pending entry and recreate it without
+    // a base URL if there has been a loadUrl("javascript:...") after
+    // loadDataWithBaseUrl.
+    *url = base_url_of_last_started_data_url_.possibly_invalid_spec();
+  }
 }
 
 bool RegisterWebContentsObserverProxy(JNIEnv* env) {

diff --git a/content/browser/android/web_contents_observer_proxy.h b/content/browser/android/web_contents_observer_proxy.h
@@ -84,6 +84,7 @@ class WebContentsObserverProxy : public WebContentsObserver {
                            bool was_ignored_by_handler);
 
   base::android::ScopedJavaGlobalRef<jobject> java_observer_;
+  GURL base_url_of_last_started_data_url_;
 
   DISALLOW_COPY_AND_ASSIGN(WebContentsObserverProxy);
 };