Skip to content

Commit

Permalink
[Android WebView] Synthesize a fake page loading event on page source…
Browse files Browse the repository at this point in the history
… modification (Re-land)

When a script modifies page source of a non-committed page, we need to
notify clients, so they can update the URL bar to avoid confusion.

New logic since the last attempt (https://codereview.chromium.org/924833003/):
distinguish between a "vanilla" WebView state (basically, a newly created
WebView, where no loading attempts have been made) and an "attempted to
navigate" state. In the "vanilla" state, don't fire synthesized page loading
events to avoid confusing clients. This is safe, as WebView is guaranteed to be
on a blank page.

Implementation note: we detect navigation attempts using
didStartProvisionalLoadForFrame WebContentsObserver event on the Java side. As
for popups AwWebContentsObserver gets re-attached from the original popup
WebView to the one provided by the client, notifications issued inbetween can be
missed on the Java side. To work around this, we assume that WebViews opened as
popups can never be in "vanilla" state (as they are anyway opened as a result of
navigation).

BUG=458569,462213
TBR=davidben@chromium.org,tedchoc@chromium.org

Review URL: https://codereview.chromium.org/970883002

Cr-Commit-Position: refs/heads/master@{#319061}
  • Loading branch information
mnaganov authored and Commit bot committed Mar 4, 2015
1 parent a6d0c54 commit b2803fd
Show file tree
Hide file tree
Showing 21 changed files with 306 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@
import org.chromium.content.browser.ContentViewCore;
import org.chromium.content.browser.ContentViewStatics;
import org.chromium.content.browser.SmartClipProvider;
import org.chromium.content.browser.WebContentsObserver;
import org.chromium.content.common.CleanupReference;
import org.chromium.content_public.browser.GestureStateListener;
import org.chromium.content_public.browser.JavaScriptCallback;
Expand Down Expand Up @@ -204,7 +203,7 @@ public abstract static class VisualStateCallback {
private NavigationController mNavigationController;
private final AwContentsClient mContentsClient;
private final AwContentViewClient mContentViewClient;
private WebContentsObserver mWebContentsObserver;
private AwWebContentsObserver mWebContentsObserver;
private final AwContentsClientBridge mContentsClientBridge;
private final AwWebContentsDelegateAdapter mWebContentsDelegate;
private final AwContentsIoThreadClient mIoThreadClient;
Expand All @@ -227,6 +226,8 @@ public abstract static class VisualStateCallback {
private boolean mHasRequestedVisitedHistoryFromClient;
// TODO(boliu): This should be in a global context, not per webview.
private final double mDIPScale;
// Whether the WebView has attempted to do any load (including uncommitted loads).
private boolean mDidAttemptLoad = false;

// The base background color, i.e. not accounting for any CSS body from the current page.
private int mBaseBackgroundColor = Color.WHITE;
Expand Down Expand Up @@ -908,6 +909,9 @@ private void receivePopupContents(long popupNativeAwContents) {
if (wasWindowFocused) onWindowFocusChanged(wasWindowFocused);
if (wasFocused) onFocusChanged(true, 0, null);

// Popups are always assumed as having made a load attempt.
mDidAttemptLoad = true;

// Restore injected JavaScript interfaces.
for (Map.Entry<String, Pair<Object, Class>> entry : javascriptInterfaces.entrySet()) {
@SuppressWarnings("unchecked")
Expand Down Expand Up @@ -1239,6 +1243,19 @@ public String getUrl() {
return url;
}

/**
* Gets the last committed URL. It represents the current page that is
* displayed in WebContents. It represents the current security context.
*
* @return The URL of the current page or null if it's empty.
*/
public String getLastCommittedUrl() {
if (isDestroyed()) return null;
String url = mWebContents.getLastCommittedUrl();
if (url == null || url.trim().isEmpty()) return null;
return url;
}

public void requestFocus() {
mAwViewMethods.requestFocus();
}
Expand Down Expand Up @@ -1838,6 +1855,11 @@ public MessagePort[] createMessageChannel() {
return ports;
}

public boolean hasAccessedInitialDocument() {
if (isDestroyed()) return false;
return mWebContents.hasAccessedInitialDocument();
}

//--------------------------------------------------------------------------------------------
// View and ViewGroup method implementations
//--------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -2072,6 +2094,12 @@ public void insertVisualStateCallback(long requestId, VisualStateCallback callba
nativeInsertVisualStateCallback(mNativeAwContents, requestId, callback);
}

public boolean getDidAttemptLoad() {
if (mDidAttemptLoad) return mDidAttemptLoad;
mDidAttemptLoad = mWebContentsObserver.hasStartedAnyProvisionalLoad();
return mDidAttemptLoad;
}

//--------------------------------------------------------------------------------------------
// Methods called from native via JNI
//--------------------------------------------------------------------------------------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,8 @@ public abstract void runFileChooser(int processId, int renderId, int modeFlags,
// Call in response to a prior runFileChooser call.
protected static native void nativeFilesSelectedInChooser(int processId, int renderId,
int modeFlags, String[] filePath, String[] displayName);

@Override
@CalledByNative
public abstract void navigationStateChanged(int flags);
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import android.os.Handler;
import android.os.Message;
import android.provider.MediaStore;
import android.text.TextUtils;
import android.util.Log;
import android.view.KeyEvent;
import android.view.View;
Expand All @@ -19,6 +20,7 @@

import org.chromium.base.ContentUriUtils;
import org.chromium.base.ThreadUtils;
import org.chromium.content_public.browser.InvalidateTypes;

/**
* Adapts the AwWebContentsDelegate interface to the AwContentsClient interface.
Expand Down Expand Up @@ -212,6 +214,22 @@ public void activateContents() {
mContentsClient.onRequestFocus();
}

@Override
public void navigationStateChanged(int flags) {
if ((flags & InvalidateTypes.URL) != 0
&& mAwContents.hasAccessedInitialDocument()
&& mAwContents.getDidAttemptLoad()) {
// Hint the client to show the last committed url, as it may be unsafe to show
// the pending entry.
String url = mAwContents.getLastCommittedUrl();
url = TextUtils.isEmpty(url) ? "about:blank" : url;
mContentsClient.onPageStarted(url);
mContentsClient.onLoadResource(url);
mContentsClient.onProgressChanged(100);
mContentsClient.onPageFinished(url);
}
}

@Override
public void toggleFullscreenModeForTab(boolean enterFullscreen) {
if (enterFullscreen) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,17 @@
*/
public class AwWebContentsObserver extends WebContentsObserver {
private final AwContentsClient mAwContentsClient;
private boolean mHasStartedAnyProvisionalLoad = false;

public AwWebContentsObserver(WebContents webContents, AwContentsClient awContentsClient) {
super(webContents);
mAwContentsClient = awContentsClient;
}

boolean hasStartedAnyProvisionalLoad() {
return mHasStartedAnyProvisionalLoad;
}

@Override
public void didFinishLoad(long frameId, String validatedUrl, boolean isMainFrame) {
String unreachableWebDataUrl = AwContentsStatics.getUnreachableWebDataUrl();
Expand Down Expand Up @@ -67,4 +72,15 @@ public void didNavigateMainFrame(String url, String baseUrl,
public void didNavigateAnyFrame(String url, String baseUrl, boolean isReload) {
mAwContentsClient.doUpdateVisitedHistory(url, isReload);
}

@Override
public void didStartProvisionalLoadForFrame(
long frameId,
long parentFrameId,
boolean isMainFrame,
String validatedUrl,
boolean isErrorPage,
boolean isIframeSrcdoc) {
mHasStartedAnyProvisionalLoad = true;
}
}
1 change: 1 addition & 0 deletions android_webview/java_library_common.mk
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ $(call intermediates-dir-for,GYP,shared)/enums/console_message_level_java/org/ch
$(call intermediates-dir-for,GYP,shared)/enums/content_gamepad_mapping/org/chromium/content/browser/input/CanonicalAxisIndex.java \
$(call intermediates-dir-for,GYP,shared)/enums/content_gamepad_mapping/org/chromium/content/browser/input/CanonicalButtonIndex.java \
$(call intermediates-dir-for,GYP,shared)/enums/gesture_event_type_java/org/chromium/content/browser/GestureEventType.java \
$(call intermediates-dir-for,GYP,shared)/enums/invalidate_types_java/org/chromium/content_public/browser/InvalidateTypes.java \
$(call intermediates-dir-for,GYP,shared)/enums/navigation_controller_java/org/chromium/content_public/browser/navigation_controller/LoadURLType.java \
$(call intermediates-dir-for,GYP,shared)/enums/navigation_controller_java/org/chromium/content_public/browser/navigation_controller/UserAgentOverrideOption.java \
$(call intermediates-dir-for,GYP,shared)/enums/popup_item_type_java/org/chromium/content/browser/input/PopupItemType.java \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,11 @@ public void run() {
});

final String parentUrl = testWebServer.setResponse("/popupParent.html", mainHtml, null);
testWebServer.setResponse(popupPath, popupHtml, null);
if (popupHtml != null) {
testWebServer.setResponse(popupPath, popupHtml, null);
} else {
testWebServer.setResponseWithNoContentStatus(popupPath);
}

parentAwContentsClient.getOnCreateWindowHelper().setReturnValue(true);
loadUrlSync(parentAwContents, parentAwContentsClient.getOnPageFinishedHelper(), parentUrl);
Expand All @@ -541,10 +545,25 @@ public void run() {
currentCallCount, 1, WAIT_TIMEOUT_MS, TimeUnit.MILLISECONDS);
}

/**
* POD object for holding references to helper objects of a popup window.
*/
public static class PopupInfo {
public final TestAwContentsClient popupContentsClient;
public final AwTestContainerView popupContainerView;
public final AwContents popupContents;
public PopupInfo(TestAwContentsClient popupContentsClient,
AwTestContainerView popupContainerView, AwContents popupContents) {
this.popupContentsClient = popupContentsClient;
this.popupContainerView = popupContainerView;
this.popupContents = popupContents;
}
}

/**
* Supplies the popup window with AwContents then waits for the popup window to finish loading.
*/
public AwContents connectPendingPopup(final AwContents parentAwContents) throws Exception {
public PopupInfo connectPendingPopup(final AwContents parentAwContents) throws Exception {
TestAwContentsClient popupContentsClient;
AwTestContainerView popupContainerView;
final AwContents popupContents;
Expand All @@ -564,6 +583,6 @@ public void run() {
int callCount = onPageFinishedHelper.getCallCount();
onPageFinishedHelper.waitForCallback(callCount, 1, WAIT_TIMEOUT_MS, TimeUnit.MILLISECONDS);

return popupContents;
return new PopupInfo(popupContentsClient, popupContainerView, popupContents);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -279,4 +279,84 @@ private void doTestOnPageFinishedCalledForHrefNavigations(boolean useBaseUrl) th
webServer.shutdown();
}
}

@MediumTest
@Feature({"AndroidWebView"})
public void testOnPageFinishedNotCalledOnDomModificationForBlankWebView() throws Throwable {
TestWebServer webServer = TestWebServer.start();
try {
doTestOnPageFinishedNotCalledOnDomMutation(webServer);
} finally {
webServer.shutdown();
}
}

@MediumTest
@Feature({"AndroidWebView"})
public void testOnPageFinishedCalledOnDomModificationAfterNonCommittedLoad() throws Throwable {
enableJavaScriptOnUiThread(mAwContents);
TestWebServer webServer = TestWebServer.start();
try {
final String noContentUrl = webServer.setResponseWithNoContentStatus("/nocontent.html");
TestCallbackHelperContainer.OnPageFinishedHelper onPageFinishedHelper =
mContentsClient.getOnPageFinishedHelper();
final int onPageFinishedCallCount = onPageFinishedHelper.getCallCount();
loadUrlAsync(mAwContents, noContentUrl);
// Mutate DOM.
executeJavaScriptAndWaitForResult(mAwContents, mContentsClient,
"document.body.innerHTML='Hello, World!'");
onPageFinishedHelper.waitForCallback(onPageFinishedCallCount);
assertEquals("about:blank", onPageFinishedHelper.getUrl());
} finally {
webServer.shutdown();
}
}

@MediumTest
@Feature({"AndroidWebView"})
public void testOnPageFinishedNotCalledOnDomModificationAfterLoadUrl() throws Throwable {
TestWebServer webServer = TestWebServer.start();
try {
final String testUrl =
webServer.setResponse("/test.html", CommonResources.ABOUT_HTML, null);
loadUrlSync(mAwContents, mContentsClient.getOnPageFinishedHelper(), testUrl);
doTestOnPageFinishedNotCalledOnDomMutation(webServer);
} finally {
webServer.shutdown();
}
}

@MediumTest
@Feature({"AndroidWebView"})
public void testOnPageFinishedNotCalledOnDomModificationAfterLoadData()
throws Throwable {
TestWebServer webServer = TestWebServer.start();
try {
loadDataSync(mAwContents, mContentsClient.getOnPageFinishedHelper(),
CommonResources.ABOUT_HTML, "text/html", false);
doTestOnPageFinishedNotCalledOnDomMutation(webServer);
} finally {
webServer.shutdown();
}
}

private void doTestOnPageFinishedNotCalledOnDomMutation(TestWebServer webServer)
throws Throwable {
enableJavaScriptOnUiThread(mAwContents);
TestCallbackHelperContainer.OnPageFinishedHelper onPageFinishedHelper =
mContentsClient.getOnPageFinishedHelper();
final int onPageFinishedCallCount = onPageFinishedHelper.getCallCount();
// Mutate DOM.
executeJavaScriptAndWaitForResult(mAwContents, mContentsClient,
"document.body.innerHTML='Hello, World!'");
// Rather than wait a fixed time to see that an onPageFinished callback isn't issued
// we load another valid page. Since callbacks arrive sequentially if the next callback
// we get is for the synchronizationUrl we know that DOM mutation did not schedule
// a callback for the iframe.
final String syncUrl = webServer.setResponse("/sync.html", "", null);
loadUrlAsync(mAwContents, syncUrl);
onPageFinishedHelper.waitForCallback(onPageFinishedCallCount);
assertEquals(syncUrl, onPageFinishedHelper.getUrl());
assertEquals(onPageFinishedCallCount + 1, onPageFinishedHelper.getCallCount());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import org.chromium.android_webview.test.util.CommonResources;
import org.chromium.base.test.util.Feature;
import org.chromium.base.test.util.MinAndroidSdkLevel;
import org.chromium.content.browser.test.util.TestCallbackHelperContainer;
import org.chromium.net.test.util.TestWebServer;

/**
Expand Down Expand Up @@ -56,8 +57,31 @@ public void testPopupWindow() throws Throwable {
"This is a popup window");

triggerPopup(mParentContents, mParentContentsClient, mWebServer, parentPageHtml,
popupPageHtml, "/popup.html", "tryOpenWindow()");
AwContents popupContents = connectPendingPopup(mParentContents);
popupPageHtml, popupPath, "tryOpenWindow()");
AwContents popupContents = connectPendingPopup(mParentContents).popupContents;
assertEquals(POPUP_TITLE, getTitleOnUiThread(popupContents));
}

@SmallTest
@Feature({"AndroidWebView"})
public void testOnPageFinishedCalledOnDomModificationAfterNavigation() throws Throwable {
final String popupPath = "/popup.html";
final String parentPageHtml = CommonResources.makeHtmlPageFrom("", "<script>"
+ "function tryOpenWindow() {"
+ " window.popupWindow = window.open('" + popupPath + "');"
+ "}"
+ "function modifyDomOfPopup() {"
+ " window.popupWindow.document.body.innerHTML = 'Hello from the parent!';"
+ "}</script>");

triggerPopup(mParentContents, mParentContentsClient, mWebServer, parentPageHtml,
null, popupPath, "tryOpenWindow()");
TestCallbackHelperContainer.OnPageFinishedHelper onPageFinishedHelper =
connectPendingPopup(mParentContents).popupContentsClient.getOnPageFinishedHelper();
final int onPageFinishedCallCount = onPageFinishedHelper.getCallCount();
executeJavaScriptAndWaitForResult(mParentContents, mParentContentsClient,
"modifyDomOfPopup()");
onPageFinishedHelper.waitForCallback(onPageFinishedCallCount);
assertEquals("about:blank", onPageFinishedHelper.getUrl());
}
}
1 change: 1 addition & 0 deletions android_webview/libwebviewchromium.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
'../content/content.gyp:console_message_level_java',
'../content/content.gyp:content_gamepad_mapping',
'../content/content.gyp:gesture_event_type_java',
'../content/content.gyp:invalidate_types_java',
'../content/content.gyp:navigation_controller_java',
'../content/content.gyp:popup_item_type_java',
'../content/content.gyp:result_codes_java',
Expand Down
12 changes: 12 additions & 0 deletions android_webview/native/aw_web_contents_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,18 @@ void AwWebContentsDelegate::AddNewContents(WebContents* source,
}
}

void AwWebContentsDelegate::NavigationStateChanged(
content::WebContents* source,
content::InvalidateTypes changed_flags) {
JNIEnv* env = AttachCurrentThread();

ScopedJavaLocalRef<jobject> java_delegate = GetJavaDelegate(env);
if (java_delegate.obj()) {
Java_AwWebContentsDelegate_navigationStateChanged(env, java_delegate.obj(),
changed_flags);
}
}

// Notifies the delegate about the creation of a new WebContents. This
// typically happens when popups are created.
void AwWebContentsDelegate::WebContentsCreated(
Expand Down
2 changes: 2 additions & 0 deletions android_webview/native/aw_web_contents_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ class AwWebContentsDelegate
bool user_gesture,
bool* was_blocked) override;

void NavigationStateChanged(content::WebContents* source,
content::InvalidateTypes changed_flags) override;
void WebContentsCreated(content::WebContents* source_contents,
int opener_render_frame_id,
const base::string16& frame_name,
Expand Down
Loading

0 comments on commit b2803fd

Please sign in to comment.