Only report resource timing to parent frame for the first iframe load
authorjiewen_tan@apple.com <jiewen_tan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 24 Oct 2018 18:49:03 +0000 (18:49 +0000)
committerjiewen_tan@apple.com <jiewen_tan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 24 Oct 2018 18:49:03 +0000 (18:49 +0000)
https://bugs.webkit.org/show_bug.cgi?id=190498
<rdar://problem/44347398>

Reviewed by Youenn Fablet.

Source/WebCore:

Only the first iframe navigation or the first iframe navigation after about:blank should be reported.
https://www.w3.org/TR/resource-timing-2/#resources-included-in-the-performanceresourcetiming-interface

Test: http/tests/misc/resource-timing-navigation-in-restored-iframe.html

* loader/FrameLoader.cpp:
(WebCore::FrameLoader::loadWithDocumentLoader):
* loader/FrameLoader.h:
(WebCore::FrameLoader::shouldReportResourceTimingToParentFrame):
(WebCore::FrameLoader::setShouldReportResourceTimingToParentFrame): Deleted.
* loader/ResourceTimingInformation.cpp:
(WebCore::ResourceTimingInformation::addResourceTiming):

LayoutTests:

The layout test is from Chromium change:
https://chromium-review.googlesource.com/c/chromium/src/+/1186215.

* http/tests/misc/resource-timing-navigation-in-restored-iframe-expected.txt: Added.
* http/tests/misc/resource-timing-navigation-in-restored-iframe.html: Added.
* http/tests/misc/resources/alert-then-back.html: Added.
* http/tests/misc/resources/navigate-on-message.html: Added.
* http/tests/misc/resources/post-message-to-parent.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/http/tests/misc/resource-timing-navigation-in-restored-iframe-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/misc/resource-timing-navigation-in-restored-iframe.html [new file with mode: 0644]
LayoutTests/http/tests/misc/resources/alert-then-back.html [new file with mode: 0644]
LayoutTests/http/tests/misc/resources/navigate-on-message.html [new file with mode: 0644]
LayoutTests/http/tests/misc/resources/post-message-to-parent.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/loader/FrameLoader.cpp
Source/WebCore/loader/FrameLoader.h
Source/WebCore/loader/ResourceTimingInformation.cpp

index 6484d9f..3d0fa06 100644 (file)
@@ -1,3 +1,20 @@
+2018-10-11  Jiewen Tan  <jiewen_tan@apple.com>
+
+        Only report resource timing to parent frame for the first iframe load
+        https://bugs.webkit.org/show_bug.cgi?id=190498
+        <rdar://problem/44347398>
+
+        Reviewed by Youenn Fablet.
+
+        The layout test is from Chromium change:
+        https://chromium-review.googlesource.com/c/chromium/src/+/1186215.
+
+        * http/tests/misc/resource-timing-navigation-in-restored-iframe-expected.txt: Added.
+        * http/tests/misc/resource-timing-navigation-in-restored-iframe.html: Added.
+        * http/tests/misc/resources/alert-then-back.html: Added.
+        * http/tests/misc/resources/navigate-on-message.html: Added.
+        * http/tests/misc/resources/post-message-to-parent.html: Added.
+
 2018-10-24  Ryan Haddad  <ryanhaddad@apple.com>
 
         [macOS] Layout Test legacy-animation-engine/animations/suspend-resume-animation.html is a flaky failure
diff --git a/LayoutTests/http/tests/misc/resource-timing-navigation-in-restored-iframe-expected.txt b/LayoutTests/http/tests/misc/resource-timing-navigation-in-restored-iframe-expected.txt
new file mode 100644 (file)
index 0000000..b864cde
--- /dev/null
@@ -0,0 +1,12 @@
+ALERT: Going back.
+Tests that subsequent navigation in an iframe restored from history does not report resource timing.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS resources.length is 1
+PASS resources[0].name is "http://127.0.0.1:8000/js-test-resources/js-test.js"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/http/tests/misc/resource-timing-navigation-in-restored-iframe.html b/LayoutTests/http/tests/misc/resource-timing-navigation-in-restored-iframe.html
new file mode 100644 (file)
index 0000000..980a3b6
--- /dev/null
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<script src="/js-test-resources/js-test.js"></script>
+<script>
+    description('Tests that subsequent navigation in an iframe restored from history does not report resource timing.');
+    window.jsTestIsAsync = true;
+    function runTest() {
+        if (!sessionStorage.didNav) {
+            sessionStorage.didNav = true;
+            // Navigate a timeout to make sure we generate a history entry that we can go back to.
+            setTimeout(function() {
+                location.href = 'resources/alert-then-back.html';
+            }, 0);
+        } else {
+            delete sessionStorage.didNav;
+            window.addEventListener('message', (event) => {
+                resources = performance.getEntriesByType('resource');
+                shouldBe('resources.length', '1');
+                shouldBeEqualToString('resources[0].name', 'http://127.0.0.1:8000/js-test-resources/js-test.js');
+                if (window.testRunner)
+                    finishJSTest();
+            });
+            document.getElementById('target-iframe').contentWindow.postMessage('navigate', '*');
+        }
+    }
+    window.onload = runTest;
+</script>
+<iframe id="target-iframe" src="http://localhost:8080/misc/resources/navigate-on-message.html"></iframe>
diff --git a/LayoutTests/http/tests/misc/resources/alert-then-back.html b/LayoutTests/http/tests/misc/resources/alert-then-back.html
new file mode 100644 (file)
index 0000000..070b2aa
--- /dev/null
@@ -0,0 +1 @@
+<script>alert("Going back.");history.back();</script>
diff --git a/LayoutTests/http/tests/misc/resources/navigate-on-message.html b/LayoutTests/http/tests/misc/resources/navigate-on-message.html
new file mode 100644 (file)
index 0000000..d410668
--- /dev/null
@@ -0,0 +1,8 @@
+<!DOCTYPE html>
+<script>
+function handleMessage() {
+    window.location = 'post-message-to-parent.html';
+}
+
+window.addEventListener("message", handleMessage);
+</script>
diff --git a/LayoutTests/http/tests/misc/resources/post-message-to-parent.html b/LayoutTests/http/tests/misc/resources/post-message-to-parent.html
new file mode 100644 (file)
index 0000000..136a5a5
--- /dev/null
@@ -0,0 +1,4 @@
+<!DOCTYPE html>
+<script>
+  window.parent.postMessage('navigated', '*');
+</script>
index 47bc5d6..609a1cf 100644 (file)
@@ -1,3 +1,24 @@
+2018-10-11  Jiewen Tan  <jiewen_tan@apple.com>
+
+        Only report resource timing to parent frame for the first iframe load
+        https://bugs.webkit.org/show_bug.cgi?id=190498
+        <rdar://problem/44347398>
+
+        Reviewed by Youenn Fablet.
+
+        Only the first iframe navigation or the first iframe navigation after about:blank should be reported.
+        https://www.w3.org/TR/resource-timing-2/#resources-included-in-the-performanceresourcetiming-interface
+
+        Test: http/tests/misc/resource-timing-navigation-in-restored-iframe.html
+
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::loadWithDocumentLoader):
+        * loader/FrameLoader.h:
+        (WebCore::FrameLoader::shouldReportResourceTimingToParentFrame):
+        (WebCore::FrameLoader::setShouldReportResourceTimingToParentFrame): Deleted.
+        * loader/ResourceTimingInformation.cpp:
+        (WebCore::ResourceTimingInformation::addResourceTiming):
+
 2018-10-24  Brent Fulgham  <bfulgham@apple.com>
 
         Cure Windows Direct2D Backend of a nasty case of bitrot
index c432bde..c5f9206 100644 (file)
 #include "SubframeLoader.h"
 #include "SubresourceLoader.h"
 #include "TextResourceDecoder.h"
+#include "URL.h"
 #include "UserContentController.h"
 #include "UserGestureIndicator.h"
 #include "WindowFeatures.h"
@@ -1554,6 +1555,11 @@ void FrameLoader::loadWithDocumentLoader(DocumentLoader* loader, FrameLoadType t
 
     const URL& newURL = loader->request().url();
 
+    // Only the first iframe navigation or the first iframe navigation after about:blank should be reported.
+    // https://www.w3.org/TR/resource-timing-2/#resources-included-in-the-performanceresourcetiming-interface
+    if (m_shouldReportResourceTimingToParentFrame && !m_previousURL.isNull() && m_previousURL != blankURL())
+        m_shouldReportResourceTimingToParentFrame = false;
+
     // Log main frame navigation types.
     if (m_frame.isMainFrame()) {
         if (auto* page = m_frame.page()) {
index 4493bd5..29319e3 100644 (file)
@@ -169,8 +169,7 @@ public:
     DocumentLoader* provisionalDocumentLoader() const { return m_provisionalDocumentLoader.get(); }
     FrameState state() const { return m_state; }
 
-    void setShouldReportResourceTimingToParentFrame(bool value) { m_shouldReportResourceTimingToParentFrame = value; }
-    bool shouldReportResourceTimingToParentFrame() { return m_shouldReportResourceTimingToParentFrame; };
+    bool shouldReportResourceTimingToParentFrame() const { return m_shouldReportResourceTimingToParentFrame; };
     
 #if PLATFORM(IOS_FAMILY)
     RetainPtr<CFDictionaryRef> connectionProperties(ResourceLoader*);
index b9260f1..6d28919 100644 (file)
@@ -70,10 +70,8 @@ void ResourceTimingInformation::addResourceTiming(CachedResource& resource, Docu
         return;
 
     Document* initiatorDocument = &document;
-    if (resource.type() == CachedResource::Type::MainResource && document.frame() && document.frame()->loader().shouldReportResourceTimingToParentFrame()) {
-        document.frame()->loader().setShouldReportResourceTimingToParentFrame(false);
+    if (resource.type() == CachedResource::Type::MainResource && document.frame() && document.frame()->loader().shouldReportResourceTimingToParentFrame())
         initiatorDocument = document.parentDocument();
-    }
     if (!initiatorDocument)
         return;