Upstream ShouldOverrideUrlLoading changes

android_webview/java/src/org/chromium/android_webview/AwContents.java

Index: android_webview/java/src/org/chromium/android_webview/AwContents.java
diff --git a/android_webview/java/src/org/chromium/android_webview/AwContents.java b/android_webview/java/src/org/chromium/android_webview/AwContents.java
index 26c026d485a22739b1fd045160985ea2ace9b5f5..04fe6a53781889664162b7f1f6b0fc095a9274fb 100644
--- a/android_webview/java/src/org/chromium/android_webview/AwContents.java
+++ b/android_webview/java/src/org/chromium/android_webview/AwContents.java
@@ -290,46 +290,13 @@ public class AwContents {
     }
 
     //--------------------------------------------------------------------------------------------
+    // Use this delegate only to post onPageStarted messages, since using WebContentsObserver
+    // causes bugs with existing applications. The three problems observed are stale URLs,
+    // out of order onPageStarted's and double onPageStarted's.
     private class InterceptNavigationDelegateImpl implements InterceptNavigationDelegate {
-        private String mLastLoadUrlAddress;
-
-        public void onUrlLoadRequested(String url) {
-            mLastLoadUrlAddress = url;
-        }
-
         @Override
         public boolean shouldIgnoreNavigation(NavigationParams navigationParams) {
             final String url = navigationParams.url;
-            final int transitionType = navigationParams.pageTransitionType;
-            final boolean isLoadUrl =
-                    (transitionType & PageTransitionTypes.PAGE_TRANSITION_FROM_API) != 0;
-            final boolean isBackForward =
-                    (transitionType & PageTransitionTypes.PAGE_TRANSITION_FORWARD_BACK) != 0;
-            final boolean isReload =
-                    (transitionType & PageTransitionTypes.PAGE_TRANSITION_CORE_MASK) ==
-                    PageTransitionTypes.PAGE_TRANSITION_RELOAD;
-            final boolean isRedirect = navigationParams.isRedirect;
-
-            boolean ignoreNavigation = false;
-
-            // Any navigation from loadUrl, goBack/Forward, or reload, are considered application
-            // initiated and hence will not yield a shouldOverrideUrlLoading() callback.
-            // TODO(joth): Using PageTransitionTypes should be sufficient to determine all app
-            // initiated navigations, and so mLastLoadUrlAddress should be removed.
-            if ((isLoadUrl && !isRedirect) || isBackForward || isReload ||
-                    mLastLoadUrlAddress != null && mLastLoadUrlAddress.equals(url)) {
-                // Support the case where the user clicks on a link that takes them back to the
-                // same page.
-                mLastLoadUrlAddress = null;
-
-                // If the embedder requested the load of a certain URL via the loadUrl API, then we
-                // do not offer it to AwContentsClient.shouldOverrideUrlLoading.
-                // The embedder is also not allowed to intercept POST requests because of
-                // crbug.com/155250.
-            } else if (!navigationParams.isPost) {
-                ignoreNavigation = mContentsClient.shouldOverrideUrlLoading(url);
-            }
-
             // The existing contract is that shouldOverrideUrlLoading callbacks are delivered before

[mkosiba (inactive), 2013/10/17 10:27:45]

This comment is out of date

[sgurun-gerrit only, 2013/12/06 00:17:48]

Done.

             // onPageStarted callbacks; third party apps depend on this behavior.
             // Using a ResouceThrottle to implement the navigation interception feature results in
@@ -339,14 +306,13 @@ public class AwContents {
             // shouldOverrideUrlLoading, and only if the navigation was not ignored (this
             // balances out with the onPageFinished callback, which is suppressed in the
             // AwContentsClient if the navigation was ignored).
-            if (!ignoreNavigation) {
-                // The shouldOverrideUrlLoading call might have resulted in posting messages to the
-                // UI thread. Using sendMessage here (instead of calling onPageStarted directly)
-                // will allow those to run in order.
-                mContentsClient.getCallbackHelper().postOnPageStarted(url);
-            }
 
-            return ignoreNavigation;
+            // The shouldOverrideUrlLoading call might have resulted in posting messages to the
+            // UI thread. Using sendMessage here (instead of calling onPageStarted directly)
+            // will allow those to run in order.
+            mContentsClient.getCallbackHelper().postOnPageStarted(url);

[mkosiba (inactive), 2013/10/17 10:27:45]

per comment above the post is no longer needed. just call the method directly
and remove the postOnPageStarted from the CallbackHelper

[sgurun-gerrit only, 2013/12/06 00:17:48]

We now (again) call shouldoverrideurlloading here due to popup case so leaving
the way it is.
On 2013/10/17 10:27:45, mkosiba wrote:

+
+            return false;
         }
     }
 
@@ -903,8 +869,6 @@ public class AwContents {
 
         mContentViewCore.loadUrl(params);
 
-        suppressInterceptionForThisNavigation();
-
         // The behavior of WebViewClassic uses the populateVisitedLinks callback in WebKit.
         // Chromium does not use this use code path and the best emulation of this behavior to call
         // request visited links once on the first URL load of the WebView.
@@ -914,14 +878,6 @@ public class AwContents {
         }
     }
 
-    private void suppressInterceptionForThisNavigation() {
-        if (mInterceptNavigationDelegate != null) {
-            // getUrl returns a sanitized address in the same format that will be used for
-            // callbacks, so it's safe to use string comparison as an equality check later on.
-            mInterceptNavigationDelegate.onUrlLoadRequested(mContentViewCore.getUrl());
-        }
-    }
-
     /**
      * Get the URL of the current page.
      *
@@ -1133,8 +1089,6 @@ public class AwContents {
      */
     public void goBack() {
         mContentViewCore.goBack();
-
-        suppressInterceptionForThisNavigation();
     }
 
     /**
@@ -1149,8 +1103,6 @@ public class AwContents {
      */
     public void goForward() {
         mContentViewCore.goForward();
-
-        suppressInterceptionForThisNavigation();
     }
 
     /**
@@ -1165,8 +1117,6 @@ public class AwContents {
      */
     public void goBackOrForward(int steps) {
         mContentViewCore.goToOffset(steps);
-
-        suppressInterceptionForThisNavigation();
     }
 
     /**