From 5498351dca14a6380ef3174a0afa0bb950b92d68 Mon Sep 17 00:00:00 2001 From: Steve Block Date: Thu, 25 Aug 2011 12:41:57 +0100 Subject: 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 --- .../WebCoreSupport/FrameLoaderClientAndroid.cpp | 37 +++++++++++++--------- .../WebCoreSupport/FrameLoaderClientAndroid.h | 11 ++++--- Source/WebKit/android/jni/WebCoreFrameBridge.cpp | 12 ++++--- Source/WebKit/android/jni/WebCoreFrameBridge.h | 2 ++ 4 files changed, 37 insertions(+), 25 deletions(-) (limited to 'Source/WebKit/android') 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); -- cgit v1.1