Commit 5b685a09 authored by Nate Fischer's avatar Nate Fischer Committed by Commit Bot

AW: only call onPageStarted once for loadDataWithBaseUrl

Previously, WebView would explicitly post onPageStarted for
loadDataWithBaseUrl, since this navigation was handled by blink and it
did not generate the event.

With browser-side navigation (PlzNavigate), this is no longer the case
and the navigation code will post this event under the normal pipeline.
For this reason, we need to disable the extra postOnPageStarted when
browser-side is turned on.

As part of the implementation, this adds a Java wrapper around
IsBrowserSideNavigationEnabled.

This also modifies an integration test to check that we only get
onPageStarted called once.

Bug: 702785
Test: run_webview_instrumentation_test_apk -f LoadDataWithBaseUrlTest#testloadDataWithBaseUrlCallsOnPageStarted
Change-Id: I3abc7b3503feee26e5b2438f9b039be30750e4ef
Reviewed-on: https://chromium-review.googlesource.com/683634Reviewed-by: default avatarBo <boliu@chromium.org>
Reviewed-by: default avatarJohn Abd-El-Malek <jam@chromium.org>
Commit-Queue: Nate Fischer <ntfschr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504441}
parent 81e842fd
...@@ -78,6 +78,7 @@ import org.chromium.content_public.browser.NavigationHistory; ...@@ -78,6 +78,7 @@ import org.chromium.content_public.browser.NavigationHistory;
import org.chromium.content_public.browser.WebContents; import org.chromium.content_public.browser.WebContents;
import org.chromium.content_public.browser.navigation_controller.LoadURLType; import org.chromium.content_public.browser.navigation_controller.LoadURLType;
import org.chromium.content_public.browser.navigation_controller.UserAgentOverrideOption; import org.chromium.content_public.browser.navigation_controller.UserAgentOverrideOption;
import org.chromium.content_public.common.BrowserSideNavigationPolicy;
import org.chromium.content_public.common.ContentUrlConstants; import org.chromium.content_public.common.ContentUrlConstants;
import org.chromium.content_public.common.Referrer; import org.chromium.content_public.common.Referrer;
import org.chromium.device.gamepad.GamepadList; import org.chromium.device.gamepad.GamepadList;
...@@ -1663,7 +1664,8 @@ public class AwContents implements SmartClipProvider { ...@@ -1663,7 +1664,8 @@ public class AwContents implements SmartClipProvider {
requestVisitedHistoryFromClient(); requestVisitedHistoryFromClient();
} }
if (params.getLoadUrlType() == LoadURLType.DATA && params.getBaseUrl() != null) { if (params.getLoadUrlType() == LoadURLType.DATA && params.getBaseUrl() != null
&& !BrowserSideNavigationPolicy.isBrowserSideNavigationEnabled()) {
// Data loads with a base url will be resolved in Blink, and not cause an onPageStarted // Data loads with a base url will be resolved in Blink, and not cause an onPageStarted
// event to be sent. Sending the callback directly from here. // event to be sent. Sending the callback directly from here.
mContentsClient.getCallbackHelper().postOnPageStarted(params.getBaseUrl()); mContentsClient.getCallbackHelper().postOnPageStarted(params.getBaseUrl());
......
...@@ -187,11 +187,18 @@ public class LoadDataWithBaseUrlTest extends AwTestBase { ...@@ -187,11 +187,18 @@ public class LoadDataWithBaseUrlTest extends AwTestBase {
final String baseUrl = "http://base.com/"; final String baseUrl = "http://base.com/";
TestCallbackHelperContainer.OnPageStartedHelper onPageStartedHelper = TestCallbackHelperContainer.OnPageStartedHelper onPageStartedHelper =
mContentsClient.getOnPageStartedHelper(); mContentsClient.getOnPageStartedHelper();
final int callCount = onPageStartedHelper.getCallCount(); TestCallbackHelperContainer.OnPageFinishedHelper onPageFinishedHelper =
mContentsClient.getOnPageFinishedHelper();
final int pageStartedCount = onPageStartedHelper.getCallCount();
final int pageFinishedCount = onPageFinishedHelper.getCallCount();
loadDataWithBaseUrlAsync(mAwContents, CommonResources.ABOUT_HTML, "text/html", false, loadDataWithBaseUrlAsync(mAwContents, CommonResources.ABOUT_HTML, "text/html", false,
baseUrl, ContentUrlConstants.ABOUT_BLANK_DISPLAY_URL); baseUrl, ContentUrlConstants.ABOUT_BLANK_DISPLAY_URL);
onPageStartedHelper.waitForCallback(callCount); onPageStartedHelper.waitForCallback(pageStartedCount);
assertEquals(baseUrl, onPageStartedHelper.getUrl()); assertEquals(baseUrl, onPageStartedHelper.getUrl());
onPageFinishedHelper.waitForCallback(pageFinishedCount);
assertEquals("onPageStarted should only be called once", pageStartedCount + 1,
onPageStartedHelper.getCallCount());
} }
@SmallTest @SmallTest
......
...@@ -36,6 +36,7 @@ source_set("common") { ...@@ -36,6 +36,7 @@ source_set("common") {
sources = [ sources = [
"accessibility_messages.h", "accessibility_messages.h",
"all_messages.h", "all_messages.h",
"android/browser_side_navigation_policy_android.cc",
"android/gin_java_bridge_errors.cc", "android/gin_java_bridge_errors.cc",
"android/gin_java_bridge_errors.h", "android/gin_java_bridge_errors.h",
"android/gin_java_bridge_value.cc", "android/gin_java_bridge_value.cc",
......
// Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include <jni.h>
#include "base/android/scoped_java_ref.h"
#include "content/public/common/browser_side_navigation_policy.h"
#include "jni/BrowserSideNavigationPolicy_jni.h"
using base::android::JavaParamRef;
namespace content {
jboolean IsBrowserSideNavigationEnabled(JNIEnv* env,
const JavaParamRef<jclass>& clazz) {
return IsBrowserSideNavigationEnabled();
}
} // namespace content
...@@ -245,6 +245,7 @@ android_library("content_java") { ...@@ -245,6 +245,7 @@ android_library("content_java") {
"java/src/org/chromium/content_public/browser/WebContents.java", "java/src/org/chromium/content_public/browser/WebContents.java",
"java/src/org/chromium/content_public/browser/WebContentsObserver.java", "java/src/org/chromium/content_public/browser/WebContentsObserver.java",
"java/src/org/chromium/content_public/browser/WebContentsStatics.java", "java/src/org/chromium/content_public/browser/WebContentsStatics.java",
"java/src/org/chromium/content_public/common/BrowserSideNavigationPolicy.java",
"java/src/org/chromium/content_public/common/ContentProcessInfo.java", "java/src/org/chromium/content_public/common/ContentProcessInfo.java",
"java/src/org/chromium/content_public/common/ContentUrlConstants.java", "java/src/org/chromium/content_public/common/ContentUrlConstants.java",
"java/src/org/chromium/content_public/common/MediaMetadata.java", "java/src/org/chromium/content_public/common/MediaMetadata.java",
...@@ -374,6 +375,7 @@ generate_jni("content_jni_headers") { ...@@ -374,6 +375,7 @@ generate_jni("content_jni_headers") {
"java/src/org/chromium/content/browser/webcontents/WebContentsImpl.java", "java/src/org/chromium/content/browser/webcontents/WebContentsImpl.java",
"java/src/org/chromium/content/browser/webcontents/WebContentsObserverProxy.java", "java/src/org/chromium/content/browser/webcontents/WebContentsObserverProxy.java",
"java/src/org/chromium/content_public/browser/LoadUrlParams.java", "java/src/org/chromium/content_public/browser/LoadUrlParams.java",
"java/src/org/chromium/content_public/common/BrowserSideNavigationPolicy.java",
"java/src/org/chromium/content_public/common/MediaMetadata.java", "java/src/org/chromium/content_public/common/MediaMetadata.java",
"java/src/org/chromium/content_public/common/ResourceRequestBody.java", "java/src/org/chromium/content_public/common/ResourceRequestBody.java",
] ]
......
// Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.content_public.common;
import org.chromium.base.annotations.JNINamespace;
/**
* This is a utility class to wrap browser_side_navigation_policy.cc.
*/
@JNINamespace("content")
public final class BrowserSideNavigationPolicy {
public static boolean isBrowserSideNavigationEnabled() {
return nativeIsBrowserSideNavigationEnabled();
}
private static native boolean nativeIsBrowserSideNavigationEnabled();
// Do not instantiate this class.
private BrowserSideNavigationPolicy() {}
}
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