Make sure WebViewClient.onPageStarted() doesn't preceed WebViewClient.shouldOverrideUrlLoading()

In the case of a server-side redirect, we were triggering a callback to
WebViewClient.onPageStarted() before we'd called
WebViewClient.shouldOverrideUrlLoading().

This change delays calling WebViewClient.onPageStarted() until after
WebViewClient.shouldOverrideUrlLoading() has returned and has indicated that the
WebView should continue with the load.

Bug: 5191031
Change-Id: I43d12c4df46e5d6e2defad10e80efbf0d5c30aed

4 files changed, 37 insertions, 25 deletions

diff --git a/Source/WebKit/android/WebCoreSupport/FrameLoaderClientAndroid.cpp b/Source/WebKit/android/WebCoreSupport/FrameLoaderClientAndroid.cpp
index b79b550..9de6c09 100644
--- a/Source/WebKit/android/WebCoreSupport/FrameLoaderClientAndroid.cpp
+++ b/Source/WebKit/android/WebCoreSupport/FrameLoaderClientAndroid.cpp
@@ -95,7 +95,8 @@ FrameLoaderClientAndroid::FrameLoaderClientAndroid(WebFrame* webframe)
     , m_webFrame(webframe)
     , m_manualLoader(NULL)
     , m_hasSentResponseToPlugin(false)
-    , m_onDemandPluginsEnabled(false) {
+    , m_onDemandPluginsEnabled(false)
+    , m_didReceiveServerRedirect(false) {
     Retain(m_webFrame);
 }
 
@@ -210,9 +211,8 @@ void FrameLoaderClientAndroid::dispatchDidHandleOnloadEvents() {
 }
 
 void FrameLoaderClientAndroid::dispatchDidReceiveServerRedirectForProvisionalLoad() {
-    ASSERT(m_frame);
-    // Tell the load it was a redirect.
-    m_webFrame->loadStarted(m_frame);
+    ASSERT(!m_didReceiveServerRedirect);
+    m_didReceiveServerRedirect = true;
 }
 
 void FrameLoaderClientAndroid::dispatchDidCancelClientRedirect() {
@@ -768,17 +768,24 @@ bool FrameLoaderClientAndroid::shouldFallBack(const ResourceError&) {
 }
 
 bool FrameLoaderClientAndroid::canHandleRequest(const ResourceRequest& request) const {
-    ASSERT(m_frame);
-    // Don't allow hijacking of intrapage navigation
-    if (WebCore::equalIgnoringFragmentIdentifier(request.url(), m_frame->document()->url()))
-        return true;
-
-    // Don't allow hijacking of iframe urls that are http or https
-    if (request.url().protocol().startsWith("http", false) &&
-            m_frame->tree() && m_frame->tree()->parent())
-        return true;
-
-    return m_webFrame->canHandleRequest(request);
+    // This is called by WebCore to determine if this load can be handled by the
+    // WebView. In general, we delegate to the WebFrame, which may ask the
+    // embedding application whether it wishes to hijack the load. However, we
+    // don't allow this if the load is ...
+    // - An intrapage navigation
+    // - An iframe with a HTTP or HTTPS scheme URL
+    bool canHandle = WebCore::equalIgnoringFragmentIdentifier(request.url(), m_frame->document()->url()) ||
+            (request.url().protocol().startsWith("http", false) && m_frame->tree() && m_frame->tree()->parent()) ||
+            m_webFrame->canHandleRequest(request);
+
+    // If this is a server-side redirect and the WebView will handle loading it,
+    // notify the WebFrame, which may notify the embedding application that
+    // we're loading a new URL.
+    if (m_didReceiveServerRedirect && canHandle)
+        m_webFrame->loadStarted(m_frame);
+    m_didReceiveServerRedirect = false;
+
+    return canHandle;
 }
 
 bool FrameLoaderClientAndroid::canShowMIMEType(const String& mimeType) const {
diff --git a/Source/WebKit/android/WebCoreSupport/FrameLoaderClientAndroid.h b/Source/WebKit/android/WebCoreSupport/FrameLoaderClientAndroid.h
index e325e01..2464c58 100644
--- a/Source/WebKit/android/WebCoreSupport/FrameLoaderClientAndroid.h
+++ b/Source/WebKit/android/WebCoreSupport/FrameLoaderClientAndroid.h
@@ -229,12 +229,13 @@ namespace android {
         virtual void didSaveToPageCache() { }
         virtual void didRestoreFromPageCache() { }
     private:
-        CacheBuilder        m_cacheBuilder;
-        Frame*              m_frame;
-        WebFrame*           m_webFrame;
+        CacheBuilder m_cacheBuilder;
+        Frame* m_frame;
+        WebFrame* m_webFrame;
         PluginManualLoader* m_manualLoader;
-        bool                m_hasSentResponseToPlugin;
-        bool                m_onDemandPluginsEnabled;
+        bool m_hasSentResponseToPlugin;
+        bool m_onDemandPluginsEnabled;
+        mutable bool m_didReceiveServerRedirect;
 
         enum ResourceErrors {
             InternalErrorCancelled = -99,
diff --git a/Source/WebKit/android/jni/WebCoreFrameBridge.cpp b/Source/WebKit/android/jni/WebCoreFrameBridge.cpp
index c1c8d96..f243b09 100644
--- a/Source/WebKit/android/jni/WebCoreFrameBridge.cpp
+++ b/Source/WebKit/android/jni/WebCoreFrameBridge.cpp
@@ -839,22 +839,24 @@ WebFrame::canHandleRequest(const WebCore::ResourceRequest& request)
     if (!javaFrame.get())
         return true;
 
-    // always handle "POST" in place
+    // Always handle "POST" in place
     if (equalIgnoringCase(request.httpMethod(), "POST"))
         return true;
     const WebCore::KURL& requestUrl = request.url();
     const WTF::String& url = requestUrl.string();
-    // Empty urls should not be sent to java
+    // Empty URLs should not be sent to Java
     if (url.isEmpty())
         return true;
     jstring jUrlStr = wtfStringToJstring(env, url);
 
-    // check to see whether browser app wants to hijack url loading.
-    // if browser app handles the url, we will return false to bail out WebCore loading
+    // Delegate to the Java side to make the decision. Note that the sense of
+    // the return value of the Java method is reversed. It will return true if
+    // the embedding application wishes to hijack the load and hence the WebView
+    // should _not_ proceed with the load.
     jboolean ret = env->CallBooleanMethod(javaFrame.get(), mJavaFrame->mHandleUrl, jUrlStr);
     checkException(env);
     env->DeleteLocalRef(jUrlStr);
-    return (ret == 0);
+    return ret == JNI_FALSE;
 }
 
 bool
diff --git a/Source/WebKit/android/jni/WebCoreFrameBridge.h b/Source/WebKit/android/jni/WebCoreFrameBridge.h
index 99ca459..e49bf35 100644
--- a/Source/WebKit/android/jni/WebCoreFrameBridge.h
+++ b/Source/WebKit/android/jni/WebCoreFrameBridge.h
@@ -99,6 +99,8 @@ class WebFrame : public WebCoreRefObject {
     
     void updateVisitedHistory(const WebCore::KURL& url, bool reload);
 
+    // Used to determine whether the WebView should handle the given request.
+    // Returns true if it should handle it, otherwise false.
     virtual bool canHandleRequest(const WebCore::ResourceRequest& request);
 
     WebCore::Frame* createWindow(bool dialog, bool userGesture);