Web Inspector: webkit reload policy should match default behavior
authorbburg@apple.com <bburg@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 22 May 2017 22:06:47 +0000 (22:06 +0000)
committerbburg@apple.com <bburg@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 22 May 2017 22:06:47 +0000 (22:06 +0000)
https://bugs.webkit.org/show_bug.cgi?id=171385
<rdar://problem/31871515>

Reviewed by Joseph Pecoraro.

Source/JavaScriptCore:

Add a new option to Page.reload that allows the test harness
to reload its test page using the old reload behavior.

The new behavior of revalidating expired cached subresources only
is the current default, since only the test harness needs the old behavior.

* inspector/protocol/Page.json:

Source/WebCore:

Add an option to PageAgent.reload that tells the backend to use the old
behavior that revalidates unexpired cached subresources. This used by tests.

Covered by existing network/memory/disk cache tests.

* inspector/InspectorPageAgent.h:
* inspector/InspectorPageAgent.cpp:
(WebCore::InspectorPageAgent::reload):

Source/WebInspectorUI:

* UserInterface/Base/Main.js:
Use PageAgent.reload.invoke to make the reload options more explicit.

* UserInterface/Test/FrontendTestHarness.js:
(FrontendTestHarness.prototype.reloadPage):
Convert this method to take an options dictionary rather than positional
boolean arguments. Update call sites to pass correct options.

When running tests, we want to revalidate unexpired resources, as there
does not seem to be another reliable way to trigger revalidated cached
resources from a Web Inspector layout test. Make this behavior the default.

LayoutTests:

* http/tests/inspector/replay/replay-test.js:
* http/tests/inspector/network/resource-sizes-memory-cache.html:
Update tests to match new API.

* http/tests/inspector/network/resource-response-source-memory-cache.html:
* http/tests/inspector/network/set-resource-caching-disabled-memory-cache.html:
Added. This is a copy of the above test but uses the option to do a "legacy" reload.
With this reload type, the resource returned with HTTP 200.

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

16 files changed:
LayoutTests/ChangeLog
LayoutTests/http/tests/inspector/network/resource-response-source-memory-cache-expected.txt
LayoutTests/http/tests/inspector/network/resource-response-source-memory-cache-revalidate-expired-only-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/inspector/network/resource-response-source-memory-cache-revalidate-expired-only.html [new file with mode: 0644]
LayoutTests/http/tests/inspector/network/resource-response-source-memory-cache.html
LayoutTests/http/tests/inspector/network/resource-sizes-memory-cache.html
LayoutTests/http/tests/inspector/network/set-resource-caching-disabled-memory-cache.html
LayoutTests/http/tests/inspector/replay/replay-test.js
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/inspector/protocol/Page.json
Source/WebCore/ChangeLog
Source/WebCore/inspector/InspectorPageAgent.cpp
Source/WebCore/inspector/InspectorPageAgent.h
Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/Base/Main.js
Source/WebInspectorUI/UserInterface/Test/FrontendTestHarness.js

index 6335537..b06afb5 100644 (file)
@@ -1,3 +1,20 @@
+2017-05-22  Brian Burg  <bburg@apple.com>
+
+        Web Inspector: webkit reload policy should match default behavior
+        https://bugs.webkit.org/show_bug.cgi?id=171385
+        <rdar://problem/31871515>
+
+        Reviewed by Joseph Pecoraro.
+
+        * http/tests/inspector/replay/replay-test.js:
+        * http/tests/inspector/network/resource-sizes-memory-cache.html:
+        Update tests to match new API.
+
+        * http/tests/inspector/network/resource-response-source-memory-cache.html:
+        * http/tests/inspector/network/set-resource-caching-disabled-memory-cache.html:
+        Added. This is a copy of the above test but uses the option to do a "legacy" reload.
+        With this reload type, the resource returned with HTTP 200.
+
 2017-05-22  Yoav Weiss  <yoav@yoav.ws>
 
         [preload] Add media and type attribute support.
index 0056975..041c4d6 100644 (file)
@@ -4,6 +4,6 @@ Test for `Resource.ResponseSource.MemoryCache`.
 == Running test suite: Resource.ResponseSource.MemoryCache
 -- Running test case: Resource.ResponseSource.MemoryCache
 PASS: Resource should be exist.
-PASS: statusCode should be 304
+PASS: statusCode should be 200
 PASS: responseSource should be Symbol(memory-cache)
 
diff --git a/LayoutTests/http/tests/inspector/network/resource-response-source-memory-cache-revalidate-expired-only-expected.txt b/LayoutTests/http/tests/inspector/network/resource-response-source-memory-cache-revalidate-expired-only-expected.txt
new file mode 100644 (file)
index 0000000..0056975
--- /dev/null
@@ -0,0 +1,9 @@
+Test for `Resource.ResponseSource.MemoryCache`.
+
+
+== Running test suite: Resource.ResponseSource.MemoryCache
+-- Running test case: Resource.ResponseSource.MemoryCache
+PASS: Resource should be exist.
+PASS: statusCode should be 304
+PASS: responseSource should be Symbol(memory-cache)
+
diff --git a/LayoutTests/http/tests/inspector/network/resource-response-source-memory-cache-revalidate-expired-only.html b/LayoutTests/http/tests/inspector/network/resource-response-source-memory-cache-revalidate-expired-only.html
new file mode 100644 (file)
index 0000000..deb48dd
--- /dev/null
@@ -0,0 +1,56 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+<script src="resources/cached-script.js"></script>
+<script src="../resources/inspector-test.js"></script>
+<script>
+TestPage.dispatchEventToFrontend("LoadComplete");
+
+function test()
+{
+    let suite = InspectorTest.createAsyncSuite("Resource.ResponseSource.MemoryCache");
+
+    function addReloadTestCase({name, description, expression, pattern, ignoreCache, statusCode, responseSource}) {
+        suite.addTestCase({
+            name, description,
+            test(resolve, reject) {
+                InspectorTest.reloadPage({ignoreCache, revalidateAllResources: true});
+                InspectorTest.awaitEvent("LoadComplete").then((event) => {
+                    let resource = null;
+                    for (let item of WebInspector.frameResourceManager.mainFrame.resourceCollection.items) {
+                        if (pattern.test(item.url)) {
+                            resource = item;
+                            break;
+                        }
+                    }
+                    if (!resource) {
+                        InspectorTest.fail("Failed to find specific resource.");
+                        reject();
+                        return;
+                    }
+                    InspectorTest.expectThat(resource instanceof WebInspector.Resource, "Resource should be exist.");
+                    InspectorTest.expectEqual(resource.statusCode, statusCode, `statusCode should be ${statusCode}`);
+                    InspectorTest.expectEqual(resource.responseSource, responseSource, `responseSource should be ${String(responseSource)}`);
+                }).then(resolve, reject);
+            }
+        });
+    }
+
+    addReloadTestCase({
+        name: "Resource.ResponseSource.MemoryCache",
+        description: "Load a resource from the memory cache by reloading this page.",
+        pattern: /cached-script\.js$/,
+        ignoreCache: false,
+        responseSource: WebInspector.Resource.ResponseSource.MemoryCache,
+        statusCode: 304,
+    });
+
+    suite.runTestCasesAndFinish();
+}
+</script>
+</head>
+<body onload="runTest()">
+<p>Test for `Resource.ResponseSource.MemoryCache`.</p>
+</body>
+</html>
index 1914f7b..1768308 100644 (file)
@@ -15,7 +15,7 @@ function test()
         suite.addTestCase({
             name, description,
             test(resolve, reject) {
-                InspectorTest.reloadPage(ignoreCache);
+                InspectorTest.reloadPage({ignoreCache});
                 InspectorTest.awaitEvent("LoadComplete").then((event) => {
                     let resource = null;
                     for (let item of WebInspector.frameResourceManager.mainFrame.resourceCollection.items) {
@@ -31,7 +31,7 @@ function test()
                     }
                     InspectorTest.expectThat(resource instanceof WebInspector.Resource, "Resource should be exist.");
                     InspectorTest.expectEqual(resource.statusCode, statusCode, `statusCode should be ${statusCode}`);
-                    InspectorTest.expectEqual(resource.responseSource, responseSource, `responseSource should be ${String(responseSource)}`);                    
+                    InspectorTest.expectEqual(resource.responseSource, responseSource, `responseSource should be ${String(responseSource)}`);
                 }).then(resolve, reject);
             }
         });
@@ -43,7 +43,7 @@ function test()
         pattern: /cached-script\.js$/,
         ignoreCache: false,
         responseSource: WebInspector.Resource.ResponseSource.MemoryCache,
-        statusCode: 304,
+        statusCode: 200,
     });
 
     suite.runTestCasesAndFinish();
index 5a76525..703c307 100644 (file)
@@ -24,8 +24,9 @@ function test()
         size: 27,
         resourceLoader() {
             const ignoreCache = false;
+            const revalidateAllResources = true;
             const pattern = /cached-script\.js$/;
-            InspectorTest.reloadPage(ignoreCache);
+            InspectorTest.reloadPage({ignoreCache, revalidateAllResources});
             return InspectorTest.awaitEvent("LoadComplete").then((event) => {
                 let resource = null;
                 for (let item of WebInspector.frameResourceManager.mainFrame.resourceCollection.items) {
index 1a70e22..4f922c1 100644 (file)
@@ -18,7 +18,7 @@ function test()
             name, description, setup,
             test(resolve, reject) {
                 NetworkAgent.setResourceCachingDisabled(true);
-                InspectorTest.reloadPage(ignoreCache);
+                InspectorTest.reloadPage({ignoreCache});
                 InspectorTest.awaitEvent("LoadComplete").then((event) => {
                     let resource = null;
                     for (let item of WebInspector.frameResourceManager.mainFrame.resourceCollection.items) {
index c8f4a83..6e2357a 100644 (file)
@@ -7,8 +7,7 @@ InspectorTest.Replay.runSingleSegmentRefTest = function(stateComparator)
     var stateDuringCapturing = null;
     var stateDuringReplaying = null;
 
-    var ignoreCacheOnReload = true;
-    InspectorTest.reloadPage(ignoreCacheOnReload)
+    InspectorTest.reloadPage({ignoreCache: true})
     .then(function() {
         return new Promise(function waitForMainResourceBeforeStarting(resolve, reject) {
             InspectorTest.eventDispatcher.addEventListener(InspectorTest.EventDispatcher.Event.TestPageDidLoad, resolve);
index 14fe96a..e15d9a1 100644 (file)
@@ -1,3 +1,19 @@
+2017-05-22  Brian Burg  <bburg@apple.com>
+
+        Web Inspector: webkit reload policy should match default behavior
+        https://bugs.webkit.org/show_bug.cgi?id=171385
+        <rdar://problem/31871515>
+
+        Reviewed by Joseph Pecoraro.
+
+        Add a new option to Page.reload that allows the test harness
+        to reload its test page using the old reload behavior.
+
+        The new behavior of revalidating expired cached subresources only
+        is the current default, since only the test harness needs the old behavior.
+
+        * inspector/protocol/Page.json:
+
 2017-05-22  Keith Miller  <keith_miller@apple.com>
 
         [Cocoa] An exported Objective C class’s prototype and constructor don't persist across JSContext deallocation
index 8df0dbb..a7649ce 100644 (file)
             "name": "reload",
             "parameters": [
                 { "name": "ignoreCache", "type": "boolean", "optional": true, "description": "If true, browser cache is ignored (as if the user pressed Shift+refresh)." },
+                { "name": "revalidateAllResources", "type": "boolean", "optional": true, "description": "If true, all cached subresources will be revalidated when the main resource loads. Otherwise, only expired cached subresources will be revalidated (the default behavior for most WebKit clients)." },
                 { "name": "scriptToEvaluateOnLoad", "type": "string", "optional": true, "description": "If set, the script will be injected into all frames of the inspected page after reload." }
             ],
-            "description": "Reloads given page optionally ignoring the cache."
+            "description": "Reloads the main frame of the inspected page."
         },
         {
             "name": "navigate",
index a87cc4e..0d48183 100644 (file)
@@ -1,3 +1,20 @@
+2017-05-22  Brian Burg  <bburg@apple.com>
+
+        Web Inspector: webkit reload policy should match default behavior
+        https://bugs.webkit.org/show_bug.cgi?id=171385
+        <rdar://problem/31871515>
+
+        Reviewed by Joseph Pecoraro.
+
+        Add an option to PageAgent.reload that tells the backend to use the old
+        behavior that revalidates unexpired cached subresources. This used by tests.
+
+        Covered by existing network/memory/disk cache tests.
+
+        * inspector/InspectorPageAgent.h:
+        * inspector/InspectorPageAgent.cpp:
+        (WebCore::InspectorPageAgent::reload):
+
 2017-05-22  Yoav Weiss  <yoav@yoav.ws>
 
         [preload] Add media and type attribute support.
index c434931..83206bc 100644 (file)
@@ -399,13 +399,19 @@ void InspectorPageAgent::removeScriptToEvaluateOnLoad(ErrorString& error, const
     m_scriptsToEvaluateOnLoad->remove(identifier);
 }
 
-void InspectorPageAgent::reload(ErrorString&, const bool* const optionalIgnoreCache, const String* optionalScriptToEvaluateOnLoad)
+void InspectorPageAgent::reload(ErrorString&, const bool* const optionalIgnoreCache, const bool* const optionalRevalidateAllResources, const String* optionalScriptToEvaluateOnLoad)
 {
     m_pendingScriptToEvaluateOnLoadOnce = optionalScriptToEvaluateOnLoad ? *optionalScriptToEvaluateOnLoad : emptyString();
 
+    bool reloadFromOrigin = optionalIgnoreCache && *optionalIgnoreCache;
+    bool revalidateAllResources = optionalRevalidateAllResources && *optionalRevalidateAllResources;
+
     OptionSet<ReloadOption> reloadOptions;
-    if (optionalIgnoreCache && *optionalIgnoreCache)
+    if (reloadFromOrigin)
         reloadOptions |= ReloadOption::FromOrigin;
+    if (!revalidateAllResources)
+        reloadOptions |= ReloadOption::ExpiredOnly;
+
     m_page.mainFrame().loader().reload(reloadOptions);
 }
 
index 939e70c..93c904d 100644 (file)
@@ -93,7 +93,7 @@ public:
     void disable(ErrorString&) override;
     void addScriptToEvaluateOnLoad(ErrorString&, const String& source, String* result) override;
     void removeScriptToEvaluateOnLoad(ErrorString&, const String& identifier) override;
-    void reload(ErrorString&, const bool* const optionalIgnoreCache, const String* const optionalScriptToEvaluateOnLoad) override;
+    void reload(ErrorString&, const bool* const optionalIgnoreCache, const bool* const optionalRevalidateAllResources, const String* const optionalScriptToEvaluateOnLoad) override;
     void navigate(ErrorString&, const String& url) override;
     void getCookies(ErrorString&, RefPtr<Inspector::Protocol::Array<Inspector::Protocol::Page::Cookie>>& cookies) override;
     void deleteCookie(ErrorString&, const String& cookieName, const String& url) override;
index 3c6f3c3..227161f 100644 (file)
@@ -1,3 +1,23 @@
+2017-05-22  Brian Burg  <bburg@apple.com>
+
+        Web Inspector: webkit reload policy should match default behavior
+        https://bugs.webkit.org/show_bug.cgi?id=171385
+        <rdar://problem/31871515>
+
+        Reviewed by Joseph Pecoraro.
+
+        * UserInterface/Base/Main.js:
+        Use PageAgent.reload.invoke to make the reload options more explicit.
+
+        * UserInterface/Test/FrontendTestHarness.js:
+        (FrontendTestHarness.prototype.reloadPage):
+        Convert this method to take an options dictionary rather than positional
+        boolean arguments. Update call sites to pass correct options.
+
+        When running tests, we want to revalidate unexpired resources, as there
+        does not seem to be another reliable way to trigger revalidated cached
+        resources from a Web Inspector layout test. Make this behavior the default.
+
 2017-05-22  Simon Fraser  <simon.fraser@apple.com>
 
         Support transform-box to switch sizing box in SVG
index d78e7ae..3a51ebd 100644 (file)
@@ -1909,7 +1909,7 @@ WebInspector._reloadPage = function(event)
 WebInspector._reloadPageClicked = function(event)
 {
     // Ignore cache when the shift key is pressed.
-    PageAgent.reload(window.event ? window.event.shiftKey : false);
+    PageAgent.reload.invoke({shouldIgnoreCache: window.event ? window.event.shiftKey : false});
 };
 
 WebInspector._reloadPageIgnoringCache = function(event)
index 5d9bb20..8757ab6 100644 (file)
@@ -120,14 +120,18 @@ FrontendTestHarness = class FrontendTestHarness extends TestHarness
             this.completeTest();
     }
 
-    reloadPage(shouldIgnoreCache)
+    reloadPage(options={})
     {
         console.assert(!this._testPageIsReloading);
         console.assert(!this._testPageReloadedOnce);
 
         this._testPageIsReloading = true;
 
-        return PageAgent.reload(!!shouldIgnoreCache)
+        let {ignoreCache, revalidateAllResources} = options;
+        let shouldIgnoreCache = !!ignoreCache;
+        revalidateAllResources = !!revalidateAllResources;
+
+        return PageAgent.reload.invoke({shouldIgnoreCache, revalidateAllResources})
             .then(() => {
                 this._shouldResendResults = true;
                 this._testPageReloadedOnce = true;