performance-api/performance-observer-no-document-leak.html is flaky
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Jul 2018 00:11:45 +0000 (00:11 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Jul 2018 00:11:45 +0000 (00:11 +0000)
https://bugs.webkit.org/show_bug.cgi?id=186938
<rdar://problem/41379336>

Reviewed by Simon Fraser.

Source/WebCore:

Add internals API to get the identifier of a document and to ask if the document with
a given identifier is still alive. This is helpful to write tests for document leaking
fixes.

* testing/Internals.cpp:
(WebCore::Internals::documentIdentifier const):
(WebCore::Internals::isDocumentAlive const):
* testing/Internals.h:
* testing/Internals.idl:

LayoutTests:

Update test to stop relying on internals.numberOfLiveDocuments() and instead rely on the new
internals.documentIdentifier() / internals.isDocumentAlive(documentIdentifier) API in order
to address the flakiness. Relying on the number of live documents to check if a particular
document was destroyed is unreliable and flaky given that WebKit constructs documents for
various reasons.

* TestExpectations:
* performance-api/performance-observer-no-document-leak-expected.txt:
* performance-api/performance-observer-no-document-leak.html:

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

LayoutTests/ChangeLog
LayoutTests/TestExpectations
LayoutTests/performance-api/performance-observer-no-document-leak-expected.txt
LayoutTests/performance-api/performance-observer-no-document-leak.html
Source/WebCore/ChangeLog
Source/WebCore/testing/Internals.cpp
Source/WebCore/testing/Internals.h
Source/WebCore/testing/Internals.idl

index cf5d366..46f8539 100644 (file)
@@ -1,3 +1,21 @@
+2018-07-03  Chris Dumez  <cdumez@apple.com>
+
+        performance-api/performance-observer-no-document-leak.html is flaky
+        https://bugs.webkit.org/show_bug.cgi?id=186938
+        <rdar://problem/41379336>
+
+        Reviewed by Simon Fraser.
+
+        Update test to stop relying on internals.numberOfLiveDocuments() and instead rely on the new
+        internals.documentIdentifier() / internals.isDocumentAlive(documentIdentifier) API in order
+        to address the flakiness. Relying on the number of live documents to check if a particular
+        document was destroyed is unreliable and flaky given that WebKit constructs documents for
+        various reasons.
+
+        * TestExpectations:
+        * performance-api/performance-observer-no-document-leak-expected.txt:
+        * performance-api/performance-observer-no-document-leak.html:
+
 2018-07-03  Truitt Savell  <tsavell@apple.com>
 
         Re-enabling canvas tests for canvas/philip/tests/initial.reset.gradient.html
index 8ced3ca..b852416 100644 (file)
@@ -2196,8 +2196,6 @@ imported/w3c/web-platform-tests/css/css-pseudo/first-letter-property-whitelist.h
 imported/w3c/web-platform-tests/WebCryptoAPI/generateKey/successes_RSA-OAEP.https.any.html [ Pass Failure ]
 imported/w3c/web-platform-tests/WebCryptoAPI/generateKey/successes_RSA-OAEP.https.any.worker.html [ Pass Failure ]
 
-webkit.org/b/186938 performance-api/performance-observer-no-document-leak.html [ Skip ]
-
 webkit.org/b/175609 imported/w3c/web-platform-tests/IndexedDB/idbobjectstore_getAll.html [ Pass Failure ]
 
 webkit.org/b/186848 imported/w3c/web-platform-tests/FileAPI/blob/Blob-slice.html [ Pass Failure ]
index 4d32cbb..33d64d7 100644 (file)
@@ -3,8 +3,6 @@ Tests that using PerformanceObserver does not cause the document to get leaked.
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-PASS Number of document is 2
-PASS Number of document is 1
 PASS Document did not leak
 PASS successfullyParsed is true
 
index 5aeff07..0ecc46e 100644 (file)
@@ -9,13 +9,12 @@
 description("Tests that using PerformanceObserver does not cause the document to get leaked.");
 window.jsTestIsAsync = true;
 
-function numberOfDocumentsShouldBecome(count)
+function documentShouldDie(documentIdentifier)
 {
     return new Promise(function(resolve, reject) {
         handle = setInterval(function() {
             gc();
-            if (internals.numberOfLiveDocuments() == count) {
-                testPassed("Number of document is " + count);
+            if (!internals.isDocumentAlive(documentIdentifier)) {
                 clearInterval(handle);
                 resolve();
             }
@@ -24,13 +23,14 @@ function numberOfDocumentsShouldBecome(count)
 }
 
 onload = function() {
-    numberOfDocumentsShouldBecome(2).then(function() {
+    setTimeout(function() {
+        let frameDocumentIdentifier = internals.documentIdentifier(testFrame.contentDocument);
         testFrame.remove();
-        numberOfDocumentsShouldBecome(1).then(function() {
+        documentShouldDie(frameDocumentIdentifier).then(function() {
             testPassed("Document did not leak");
             finishJSTest();
         });
-    });
+    }, 10);
 }
 </script>
 <script src="../resources/js-test-post.js"></script>
index d74a803..853245e 100644 (file)
@@ -1,5 +1,23 @@
 2018-07-03  Chris Dumez  <cdumez@apple.com>
 
+        performance-api/performance-observer-no-document-leak.html is flaky
+        https://bugs.webkit.org/show_bug.cgi?id=186938
+        <rdar://problem/41379336>
+
+        Reviewed by Simon Fraser.
+
+        Add internals API to get the identifier of a document and to ask if the document with
+        a given identifier is still alive. This is helpful to write tests for document leaking
+        fixes.
+
+        * testing/Internals.cpp:
+        (WebCore::Internals::documentIdentifier const):
+        (WebCore::Internals::isDocumentAlive const):
+        * testing/Internals.h:
+        * testing/Internals.idl:
+
+2018-07-03  Chris Dumez  <cdumez@apple.com>
+
         Improve window.event compliance: Should not be set when target is in shadow tree
         https://bugs.webkit.org/show_bug.cgi?id=186266
 
index 76b0c7d..84c5044 100644 (file)
@@ -2324,6 +2324,16 @@ unsigned Internals::referencingNodeCount(const Document& document) const
     return document.referencingNodeCount();
 }
 
+uint64_t Internals::documentIdentifier(const Document& document) const
+{
+    return document.identifier().toUInt64();
+}
+
+bool Internals::isDocumentAlive(uint64_t documentIdentifier) const
+{
+    return Document::allDocumentsMap().contains(makeObjectIdentifier<DocumentIdentifierType>(documentIdentifier));
+}
+
 RefPtr<WindowProxy> Internals::openDummyInspectorFrontend(const String& url)
 {
     auto* inspectedPage = contextDocument()->frame()->page();
index 71bb7c0..bac9004 100644 (file)
@@ -364,6 +364,9 @@ public:
     unsigned numberOfLiveDocuments() const;
     unsigned referencingNodeCount(const Document&) const;
 
+    uint64_t documentIdentifier(const Document&) const;
+    bool isDocumentAlive(uint64_t documentIdentifier) const;
+
     RefPtr<WindowProxy> openDummyInspectorFrontend(const String& url);
     void closeDummyInspectorFrontend();
     ExceptionOr<void> setInspectorIsUnderTest(bool);
index 26d0810..32cb58c 100644 (file)
@@ -613,6 +613,9 @@ enum EventThrottlingBehavior {
     [Conditional=MEDIA_STREAM] void simulateMediaStreamTrackCaptureSourceFailure(MediaStreamTrack track);
     [Conditional=MEDIA_STREAM] void setMediaStreamTrackIdentifier(MediaStreamTrack track, DOMString identifier);
 
+    unsigned long long documentIdentifier(Document document);
+    boolean isDocumentAlive(unsigned long long documentIdentifier);
+
     Promise<void> clearCacheStorageMemoryRepresentation();
     Promise<DOMString> cacheStorageEngineRepresentation();
     void setResponseSizeWithPadding(FetchResponse response, unsigned long long size);