diff options
| author | Mikhail Naganov <mnaganov@chromium.org> | 2016-03-16 13:14:27 -0700 |
|---|---|---|
| committer | Mikhail Naganov <mnaganov@chromium.org> | 2016-03-16 20:16:34 +0000 |
| commit | 78fca66434b0e5986b4501ea291175ae4b6b56df (patch) | |
| tree | 435aa4211de3d679b2b22ca9df241de25c2dd16e | |
| parent | 1802181275c4b3bcd8c5458c0dbdb65b2b63dfe8 (diff) | |
| download | chromium_src-78fca66434b0e5986b4501ea291175ae4b6b56df.zip chromium_src-78fca66434b0e5986b4501ea291175ae4b6b56df.tar.gz chromium_src-78fca66434b0e5986b4501ea291175ae4b6b56df.tar.bz2 | |
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 ce1f4d009316d8f5f4c49b18e1bd94687d80c59d)
Review URL: https://codereview.chromium.org/1806853005 .
Cr-Commit-Position: refs/branch-heads/2623@{#627}
Cr-Branched-From: 92d77538a86529ca35f9220bd3cd512cbea1f086-refs/heads/master@{#369907}
3 files changed, 48 insertions, 1 deletions
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 index 177f8c7..6fb0b3d 100644 --- 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 class LoadDataWithBaseUrlTest extends AwTestBase { 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 index 22ab07c..d176f61 100644 --- a/content/browser/android/web_contents_observer_proxy.cc +++ b/content/browser/android/web_contents_observer_proxy.cc @@ -81,6 +81,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()); } @@ -90,6 +93,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(), @@ -304,8 +309,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 index 2346405..e22edbb 100644 --- a/content/browser/android/web_contents_observer_proxy.h +++ b/content/browser/android/web_contents_observer_proxy.h @@ -83,6 +83,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); }; |