Changeset 94630 in webkit

Timestamp:

Sep 6, 2011 8:00:55 PM (13 years ago)

Author:

commit-queue@webkit.org

Message:

[chromium] Fix WebFrameClient receiving isRedirect when reloading
​https://bugs.webkit.org/show_bug.cgi?id=63591

Patch by Ben Smith <​binji@chromium.org> on 2011-09-06
Reviewed by Adam Barth.

Fix bug when quickly reloading a WebFrame where the
WebFrameClient would get called with decidePolicyForNavigation
isRedirect=true.

• src/FrameLoaderClientImpl.cpp:

(WebKit::FrameLoaderClientImpl::dispatchDecidePolicyForNavigationAction):

• src/WebDataSourceImpl.h:

(WebKit::WebDataSourceImpl::isRedirect):

• tests/WebFrameTest.cpp:

(WebKit::TestReloadDoesntRedirectWebFrameClient::decidePolicyForNavigation):
(WebKit::TestReloadDoesntRedirectWebFrameClient::cancelledError):
(WebKit::TEST_F):

Location:

trunk/Source/WebKit/chromium

Files:

• ChangeLog (modified) (1 diff)
• src/FrameLoaderClientImpl.cpp (modified) (1 diff)
• src/WebDataSourceImpl.h (modified) (1 diff)
• tests/WebFrameTest.cpp (modified) (2 diffs)

trunk/Source/WebKit/chromium/ChangeLog

@@ +1 @@
+2011-09-06  Ben Smith  <binji@chromium.org>
+
+        [chromium] Fix WebFrameClient receiving isRedirect when reloading
+        https://bugs.webkit.org/show_bug.cgi?id=63591
+
+        Reviewed by Adam Barth.
+
+        Fix bug when quickly reloading a WebFrame where the
+        WebFrameClient would get called with decidePolicyForNavigation
+        isRedirect=true.
+
+        * src/FrameLoaderClientImpl.cpp:
+        (WebKit::FrameLoaderClientImpl::dispatchDecidePolicyForNavigationAction):
+        * src/WebDataSourceImpl.h:
+        (WebKit::WebDataSourceImpl::isRedirect):
+        * tests/WebFrameTest.cpp:
+        (WebKit::TestReloadDoesntRedirectWebFrameClient::decidePolicyForNavigation):
+        (WebKit::TestReloadDoesntRedirectWebFrameClient::cancelledError):
+        (WebKit::TEST_F):
+
 2011-09-06  Ryosuke Niwa  <rniwa@webkit.org>
 

trunk/Source/WebKit/chromium/src/FrameLoaderClientImpl.cpp

@@ -967 +967 @@
             ASSERT(!url.protocolIs(backForwardNavigationScheme));
 
-            bool isRedirect = ds->hasRedirectChain();
+            bool isRedirect = ds->isRedirect();
 
             WebNavigationType webnavType =

trunk/Source/WebKit/chromium/src/WebDataSourceImpl.h

@@ -75 +75 @@
 
     bool hasRedirectChain() const { return !m_redirectChain.isEmpty(); }
+    bool isRedirect() const { return m_redirectChain.size() > 1; }
     const WebCore::KURL& endOfRedirectChain() const;
     void clearRedirectChain();

trunk/Source/WebKit/chromium/tests/WebFrameTest.cpp

@@ -31 +31 @@
 #include "config.h"
 
+#include "ResourceError.h"
 #include "WebDocument.h"
 #include "WebFormElement.h"
@@ -216 +217 @@
 }
 
+class TestReloadDoesntRedirectWebFrameClient : public WebFrameClient {
+public:
+    virtual WebNavigationPolicy decidePolicyForNavigation(
+        WebFrame*, const WebURLRequest&, WebNavigationType,
+        const WebNode& originatingNode,
+        WebNavigationPolicy defaultPolicy, bool isRedirect)
+    {
+        EXPECT_EQ(false, isRedirect);
+        return WebNavigationPolicyCurrentTab;
+    }
+
+    virtual WebURLError cancelledError(WebFrame*, const WebURLRequest& request)
+    {
+        // Return a dummy error so the DocumentLoader doesn't assert when
+        // the reload cancels it.
+        return WebURLError(WebCore::ResourceError("", 1, "", "cancelled"));
+    }
+};
+
+TEST_F(WebFrameTest, ReloadDoesntSetRedirect)
+{
+    // Test for case in http://crbug.com/73104. Reloading a frame very quickly
+    // would sometimes call decidePolicyForNavigation with isRedirect=true
+    registerMockedHttpURLLoad("form.html");
+
+    TestReloadDoesntRedirectWebFrameClient webFrameClient;
+    WebView* webView = WebView::create(0);
+    webView->initializeMainFrame(&webFrameClient);
+
+    loadHttpFrame(webView->mainFrame(), "form.html");
+    serveRequests();
+    // Frame is loaded.
+
+    webView->mainFrame()->reload(true);
+    // start reload before request is delivered.
+    webView->mainFrame()->reload(true);
+    serveRequests();
+}
+
 } // namespace