Reviewed by Darin.
authorap <ap@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 20 Jun 2006 05:17:14 +0000 (05:17 +0000)
committerap <ap@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 20 Jun 2006 05:17:14 +0000 (05:17 +0000)
        - http://bugzilla.opendarwin.org/show_bug.cgi?id=5499
        Page reload does not send any cache control headers

WebCore:
        * bindings/js/kjs_window.cpp:
        (KJS::LocationFunc::callAsFunction): Call scheduleRefresh() for Location::Reload.

        * page/Frame.h: Add scheduleRefresh(). Don't derive from TransferJob or implement its methods - that was
        used only for storing response HTTP headers, which was an overkill.

        * page/FramePrivate.h: Replace TransferJob with a HashMap for storing response headers.

        * page/Frame.cpp:
        (WebCore::Frame::didOpenURL): Don't needlessly change d->m_cachePolicy. Don't create a TransferJob.
        (WebCore::Frame::stopLoading): Directly access the metadata map, instead of going through a TransferJob.
        (WebCore::Frame::receivedFirstData): Ditto.
        (WebCore::Frame::addMetaData): Ditto.
        (WebCore::Frame::scheduleRefresh): A new function that schedules a refresh, similarly to what
        scheduleRedirection() does.
        (WebCore::Frame::changeLocation): Set request.reload attribute based on the current cache policy.

WebKit:
        * WebView/WebFrame.m:
        (-[WebFrame _addExtraFieldsToRequest:mainResource:alwaysFromRequest:]): Set a proper Cache-Control header for
        reload requests.
        (-[WebFrame loadRequest:]): Reset loadType to WebFrameLoadTypeStandard (after a reload, it stayed at
        WebFrameLoadTypeReload, so _addExtraFieldsToRequest erroneously added a Cache-Control header to them).

LayoutTests:
        * http/tests/misc/refresh-headers-expected.txt: Added.
        * http/tests/misc/refresh-headers.php: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@14920 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/http/tests/misc/refresh-headers-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/misc/refresh-headers.php [new file with mode: 0644]
WebCore/ChangeLog
WebCore/bindings/js/kjs_window.cpp
WebCore/page/Frame.cpp
WebCore/page/Frame.h
WebCore/page/FramePrivate.h
WebKit/ChangeLog
WebKit/WebView/WebFrame.m

index ed479cc902b59c17fc44800858aad63288112f31..4c8edbcdcc997e686bd72eead33beae01edbea37 100644 (file)
@@ -1,3 +1,13 @@
+2006-06-19  Alexey Proskuryakov  <ap@nypop.com>
+
+        Reviewed by Darin.
+
+        - test for http://bugzilla.opendarwin.org/show_bug.cgi?id=5499
+        Page reload does not send any cache control headers
+
+        * http/tests/misc/refresh-headers-expected.txt: Added.
+        * http/tests/misc/refresh-headers.php: Added.
+
 2006-06-19  Alexey Proskuryakov  <ap@nypop.com>
 
         Reviewed by Darin.
diff --git a/LayoutTests/http/tests/misc/refresh-headers-expected.txt b/LayoutTests/http/tests/misc/refresh-headers-expected.txt
new file mode 100644 (file)
index 0000000..289f8be
--- /dev/null
@@ -0,0 +1,3 @@
+SUCCESS
+
+Test for bug 5499: Page reload does not send any cache control headers.
diff --git a/LayoutTests/http/tests/misc/refresh-headers.php b/LayoutTests/http/tests/misc/refresh-headers.php
new file mode 100644 (file)
index 0000000..e2d055e
--- /dev/null
@@ -0,0 +1,34 @@
+<?php
+  $gotMaxAge=false;
+  $gotNoCache=false;
+  $headers = getallheaders(); 
+
+  foreach ($headers as $name => $content) {
+    if (0 == strcasecmp($name, "Cache-Control") && 0 == strcasecmp($content, "max-age=0"))
+    {
+      $gotMaxAge = true;
+    }
+    
+    if ((0 == strcasecmp($name, "Cache-Control") && 0 == strcasecmp($content, "no-cache")) ||
+        (0 == strcasecmp($name, "Pragma") && 0 == strcasecmp($content, "no-cache")))
+    {
+      $gotNoCache = true;
+    }
+  }
+  
+  if ($gotNoCache) {
+    echo '<p>Got a no-cache directive; FAILURE!</p>';
+    echo '<script>if (window.layoutTestController) { layoutTestController.notifyDone(); }</script>';
+  } else if ($gotMaxAge) {
+    echo '<p>SUCCESS</p>';
+    echo '<script>if (window.layoutTestController) { layoutTestController.notifyDone(); }</script>';
+  } else {
+    echo '<body onload="window.location.reload();">';
+    echo '<p>No cache control headers, reloading...</p>';
+    echo '<script>if (window.layoutTestController) { layoutTestController.waitUntilDone(); }</script>';
+    echo '<script>function test() {window.location.reload();}</script>';
+  }
+
+  echo '<script>if (window.layoutTestController) { layoutTestController.dumpAsText(); }</script>';
+  echo '<p>Test for <a href="http://bugzilla.opendarwin.org/show_bug.cgi?id=5499">bug 5499</a>: Page reload does not send any cache control headers.</p>';
+?>
index bcd608e1d0b0327dec53258c6ef331838ff08cb6..2017d5a6a8489e0d5be6cca8575fdb2264bd6046 100644 (file)
@@ -1,3 +1,29 @@
+2006-06-19  Alexey Proskuryakov  <ap@nypop.com>
+
+        Reviewed by Darin.
+
+        - http://bugzilla.opendarwin.org/show_bug.cgi?id=5499
+        Page reload does not send any cache control headers
+
+        Test: http/tests/misc/refresh-headers.php
+
+        * bindings/js/kjs_window.cpp:
+        (KJS::LocationFunc::callAsFunction): Call scheduleRefresh() for Location::Reload.
+
+        * page/Frame.h: Add scheduleRefresh(). Don't derive from TransferJob or implement its methods - that was
+        used only for storing response HTTP headers, which was an overkill.
+
+        * page/FramePrivate.h: Replace TransferJob with a HashMap for storing response headers.
+
+        * page/Frame.cpp:
+        (WebCore::Frame::didOpenURL): Don't needlessly change d->m_cachePolicy. Don't create a TransferJob.
+        (WebCore::Frame::stopLoading): Directly access the metadata map, instead of going through a TransferJob.
+        (WebCore::Frame::receivedFirstData): Ditto.
+        (WebCore::Frame::addMetaData): Ditto.
+        (WebCore::Frame::scheduleRefresh): A new function that schedules a refresh, similarly to what
+        scheduleRedirection() does.
+        (WebCore::Frame::changeLocation): Set request.reload attribute based on the current cache policy.
+
 2006-06-19  Ben Goodger  <bengoodger@gmail.com>
 
         Reviewed by Darin.
index da2dbc82d4be4464df682cd6f150b50582813140..9c524d72e7de81919d45f1266adbcfb00c80ab81 100644 (file)
@@ -2214,10 +2214,9 @@ JSValue *LocationFunc::callAsFunction(ExecState *exec, JSObject *thisObj, const
     case Location::Reload:
     {
       const Window* window = Window::retrieveWindow(frame);
-      Frame* activePart = Window::retrieveActive(exec)->frame();
       if (!frame->url().url().startsWith("javascript:", false) || (window && window->isSafeScript(exec))) {
         bool userGesture = static_cast<ScriptInterpreter *>(exec->dynamicInterpreter())->wasRunByUserGesture();
-        frame->scheduleLocationChange(frame->url().url(), activePart->referrer(), true/*lock history*/, userGesture);
+        frame->scheduleRefresh(userGesture);
       }
       break;
     }
index 24fbe1e084b4bced6f403152dc01ffc95a212b66..0d81259bdbb5d15f7f639d8e8905f6c664255fd1 100644 (file)
@@ -66,7 +66,6 @@
 #include "SegmentedString.h"
 #include "TextDocument.h"
 #include "TextIterator.h"
-#include "TransferJob.h"
 #include "TypingCommand.h"
 #include "cssstyleselector.h"
 #include "htmlediting.h"
@@ -237,17 +236,6 @@ bool Frame::didOpenURL(const KURL& url)
   
   closeURL();
 
-  if (d->m_request.reload)
-     d->m_cachePolicy = KIO::CC_Refresh;
-  else
-     d->m_cachePolicy = KIO::CC_Verify;
-
-  if (d->m_request.doPost() && url.protocol().startsWith("http")) {
-      d->m_job = new TransferJob(this, "POST", url, d->m_request.postData);
-      d->m_job->addMetaData("content-type", d->m_request.contentType());
-  } else
-      d->m_job = new TransferJob(this, "GET", url);
-
   d->m_bComplete = false;
   d->m_bLoadingMainResource = true;
   d->m_bLoadEventEmitted = false;
@@ -287,12 +275,8 @@ void Frame::stopLoading(bool sendUnload)
 {
   if (d->m_doc && d->m_doc->tokenizer())
     d->m_doc->tokenizer()->stopParsing();
-    
-  if (d->m_job)
-  {
-    d->m_job->kill();
-    d->m_job = 0;
-  }
+  
+  d->m_metaData.clear();
 
   if (sendUnload) {
     if (d->m_doc) {
@@ -508,7 +492,7 @@ void Frame::receivedFirstData()
     DeprecatedString qData;
 
     // Support for http-refresh
-    qData = d->m_job->queryMetaData("http-refresh").deprecatedString();
+    qData = d->m_metaData.get("http-refresh").deprecatedString();
     if (!qData.isEmpty()) {
       double delay;
       int pos = qData.find(';');
@@ -546,22 +530,7 @@ void Frame::receivedFirstData()
     }
 
     // Support for http last-modified
-    d->m_lastModified = d->m_job->queryMetaData("modified");
-}
-
-void Frame::receivedAllData(TransferJob* job)
-{
-    d->m_job = 0;
-
-    if (job->error()) {
-        checkCompleted();
-        return;
-    }
-
-    d->m_workingURL = KURL();
-
-    if (d->m_doc->parsing())
-        end(); // will call completed()
+    d->m_lastModified = d->m_metaData.get("modified");
 }
 
 void Frame::childBegin()
@@ -935,6 +904,29 @@ void Frame::scheduleLocationChange(const DeprecatedString& url, const Deprecated
         startRedirectionTimer();
 }
 
+void Frame::scheduleRefresh(bool userGesture)
+{
+    // Handle a location change of a page with no document as a special case.
+    // This may happen when a frame requests a refresh of another frame.
+    d->m_scheduledRedirection = d->m_doc ? locationChangeScheduled : locationChangeScheduledDuringLoad;
+    
+    // If a refresh was scheduled during a load, then stop the current load.
+    // Otherwise when the current load transitions from a provisional to a 
+    // committed state, pending redirects may be cancelled. 
+    if (d->m_scheduledRedirection == locationChangeScheduledDuringLoad)
+        stopLoading(true);   
+
+    d->m_delayRedirect = 0;
+    d->m_redirectURL = url().url();
+    d->m_redirectReferrer = referrer();
+    d->m_redirectLockHistory = true;
+    d->m_redirectUserGesture = userGesture;
+    d->m_cachePolicy = KIO::CC_Refresh;
+    stopRedirectionTimer();
+    if (d->m_bComplete)
+        startRedirectionTimer();
+}
+
 bool Frame::isScheduledLocationChangePending() const
 {
     switch (d->m_scheduledRedirection) {
@@ -1007,6 +999,8 @@ void Frame::changeLocation(const DeprecatedString& URL, const DeprecatedString&
     if (!referrer.isEmpty())
         request.setReferrer(referrer);
 
+    request.reload = (d->m_cachePolicy == KIO::CC_Reload) || (d->m_cachePolicy == KIO::CC_Refresh);
+    
     urlSelected(request, "_self");
 }
 
@@ -1042,11 +1036,6 @@ void Frame::redirectionTimerFired(Timer<Frame>*)
     changeLocation(URL, referrer, lockHistory, userGesture);
 }
 
-void Frame::receivedRedirect(TransferJob*, const KURL& url)
-{
-    d->m_workingURL = url;
-}
-
 DeprecatedString Frame::encoding() const
 {
     if (d->m_haveEncoding && !d->m_encoding.isEmpty())
@@ -3104,7 +3093,7 @@ bool Frame::scrollbarsVisible()
 
 void Frame::addMetaData(const String& key, const String& value)
 {
-    d->m_job->addMetaData(key, value);
+    d->m_metaData.set(key, value);
 }
 
 // This does the same kind of work that Frame::openURL does, except it relies on the fact
index 773e275af2f0b5bcc4ea8052ce0b914cfbc8c2cb..5adacf701717ef927c64837964bd52f6edae6c16 100644 (file)
@@ -37,7 +37,6 @@
 #include "Node.h"
 #include "TextAffinity.h"
 #include "TextGranularity.h"
-#include "TransferJobClient.h"
 #include <wtf/Vector.h>
 #include "RenderObject.h"
 
@@ -92,7 +91,7 @@ enum ObjectContentType {
     ObjectContentPlugin,
 };
 
-class Frame : public Shared<Frame>, Noncopyable, TransferJobClient {
+class Frame : public Shared<Frame>, Noncopyable {
 
 public:
   enum { NoXPosForVerticalArrowNavigation = INT_MIN };
@@ -178,6 +177,7 @@ public:
    */
   void changeLocation(const DeprecatedString& URL, const DeprecatedString& referrer, bool lockHistory = true, bool userGesture = false);
   void scheduleLocationChange(const DeprecatedString& url, const DeprecatedString& referrer, bool lockHistory = true, bool userGesture = false);
+  void scheduleRefresh(bool userGesture = false);
   bool isScheduledLocationChangePending() const;
 
   /**
@@ -587,8 +587,6 @@ public:
   void reparseConfiguration();
 
 private:
-    virtual void receivedRedirect(TransferJob*, const KURL&);
-    virtual void receivedAllData(TransferJob*);
 
   void childBegin();
 
index cbaf485ae209d3f3f814bffcccbf90a83000ebcc..8c87f97673c5eb7052e05dc20eb3edf91e9856b9 100644 (file)
@@ -66,7 +66,6 @@ namespace WebCore {
             , m_bJavaEnabled(true)
             , m_bPluginsEnabled(true)
             , m_settings(0)
-            , m_job(0)
             , m_bComplete(true)
             , m_bLoadingMainResource(false)
             , m_bLoadEventEmitted(true)
@@ -135,7 +134,7 @@ namespace WebCore {
 
         KHTMLSettings* m_settings;
 
-        TransferJob* m_job;
+        HashMap<String, String> m_metaData;
 
         String m_kjsStatusBarText;
         String m_kjsDefaultStatusBarText;
index 96ee521bd77e13ae1b992ff8b231cd44461501a7..2c87589ccb622345f89e15ab0806d7fdbac7cf75 100644 (file)
@@ -1,3 +1,16 @@
+2006-06-19  Alexey Proskuryakov  <ap@nypop.com>
+
+        Reviewed by Darin.
+
+        - http://bugzilla.opendarwin.org/show_bug.cgi?id=5499
+        Page reload does not send any cache control headers
+
+        * WebView/WebFrame.m:
+        (-[WebFrame _addExtraFieldsToRequest:mainResource:alwaysFromRequest:]): Set a proper Cache-Control header for 
+        reload requests.
+        (-[WebFrame loadRequest:]): Reset loadType to WebFrameLoadTypeStandard (after a reload, it stayed at 
+        WebFrameLoadTypeReload, so _addExtraFieldsToRequest erroneously added a Cache-Control header to them).
+
 2006-06-19  John Sullivan  <sullivan@apple.com>
 
         Reviewed by Darin.
index 849b945a1f5da3fa83c54dcd7b616efcdf89561c..b2ae283e8a2cbf21a7ee0b1575f53b090aab1bc9 100644 (file)
@@ -2754,6 +2754,9 @@ static CFAbsoluteTime _timeOfLastCompletedLoad;
 {
     [request _web_setHTTPUserAgent:[[self webView] userAgentForURL:[request URL]]];
     
+    if (_private->loadType == WebFrameLoadTypeReload)
+        [request setValue:@"max-age=0" forHTTPHeaderField:@"Cache-Control"];
+    
     // Don't set the cookie policy URL if it's already been set.
     if ([request mainDocumentURL] == nil) {
         if (mainResource && (self == [[self webView] mainFrame] || f))
@@ -2892,6 +2895,10 @@ static CFAbsoluteTime _timeOfLastCompletedLoad;
 
 - (void)loadRequest:(NSURLRequest *)request
 {
+    // FIXME: is this the right place to reset loadType? Perhaps, this should be done
+    // after loading is finished or aborted.
+    _private->loadType = WebFrameLoadTypeStandard;
+    
     [self _loadRequest:request archive:nil];
 }