Title: [252123] trunk
Revision
252123
Author
katherine_che...@apple.com
Date
2019-11-05 21:17:37 -0800 (Tue, 05 Nov 2019)

Log Message

Layout test website-data-removal-for-site-navigated-to-with-link-decoration.html is a flaky failure
https://bugs.webkit.org/show_bug.cgi?id=203706
Source/WebKit:

<rdar://problem/56801615>

Reviewed by Chris Dumez.

No new tests, this change is tested by the existing resourceLoadStatistics
tests.

This test started flaking when a new memory store was being created
between tests to maintain consistency. The call to grandfatherExistingWebsiteData
from populateMemoryStoreFromDisk in the persistent storage was
async, causing a race condition that led to occasional failures.
Adding a completion handler and changing the callsite of
populateMemoryStoreFromDisk should fix this problem.

* NetworkProcess/Classifier/ResourceLoadStatisticsPersistentStorage.cpp:
(WebKit::ResourceLoadStatisticsPersistentStorage::populateMemoryStoreFromDisk):
* NetworkProcess/Classifier/ResourceLoadStatisticsPersistentStorage.h:
* NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp:
(WebKit::WebResourceLoadStatisticsStore::populateMemoryStoreFromDisk):

LayoutTests:

<rdar://problem/56801615>

Reviewed by Chris Dumez.

Since the state is reset between tests, the call to
setUseITPDatabase(false) is redundant.

* http/tests/resourceLoadStatistics/log-cross-site-load-with-link-decoration.html:
* http/tests/resourceLoadStatistics/website-data-removal-for-site-navigated-to-with-link-decoration.html:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (252122 => 252123)


--- trunk/LayoutTests/ChangeLog	2019-11-06 03:56:05 UTC (rev 252122)
+++ trunk/LayoutTests/ChangeLog	2019-11-06 05:17:37 UTC (rev 252123)
@@ -1,3 +1,17 @@
+2019-11-05  Kate Cheney  <katherine_che...@apple.com>
+
+        Layout test website-data-removal-for-site-navigated-to-with-link-decoration.html is a flaky failure
+        https://bugs.webkit.org/show_bug.cgi?id=203706
+        <rdar://problem/56801615> 
+
+        Reviewed by Chris Dumez. 
+
+        Since the state is reset between tests, the call to 
+        setUseITPDatabase(false) is redundant. 
+
+        * http/tests/resourceLoadStatistics/log-cross-site-load-with-link-decoration.html:
+        * http/tests/resourceLoadStatistics/website-data-removal-for-site-navigated-to-with-link-decoration.html:
+
 2019-11-05  Ryosuke Niwa  <rn...@webkit.org>
 
         Integrate visualViewport's resize event with HTML5 event loop

Modified: trunk/LayoutTests/http/tests/resourceLoadStatistics/log-cross-site-load-with-link-decoration.html (252122 => 252123)


--- trunk/LayoutTests/http/tests/resourceLoadStatistics/log-cross-site-load-with-link-decoration.html	2019-11-06 03:56:05 UTC (rev 252122)
+++ trunk/LayoutTests/http/tests/resourceLoadStatistics/log-cross-site-load-with-link-decoration.html	2019-11-06 05:17:37 UTC (rev 252123)
@@ -8,7 +8,6 @@
 <div id="output"></div>
 <script>
     if (window.testRunner) {
-        testRunner.setUseITPDatabase(false);
         testRunner.waitUntilDone();
         testRunner.dumpAsText();
     }

Modified: trunk/LayoutTests/http/tests/resourceLoadStatistics/website-data-removal-for-site-navigated-to-with-link-decoration.html (252122 => 252123)


--- trunk/LayoutTests/http/tests/resourceLoadStatistics/website-data-removal-for-site-navigated-to-with-link-decoration.html	2019-11-06 03:56:05 UTC (rev 252122)
+++ trunk/LayoutTests/http/tests/resourceLoadStatistics/website-data-removal-for-site-navigated-to-with-link-decoration.html	2019-11-06 05:17:37 UTC (rev 252123)
@@ -10,7 +10,6 @@
 <div id="output"></div>
 <br>
 <script>
-    testRunner.setUseITPDatabase(false);
     testRunner.waitUntilDone();
     testRunner.dumpAsText();
 

Modified: trunk/Source/WebKit/ChangeLog (252122 => 252123)


--- trunk/Source/WebKit/ChangeLog	2019-11-06 03:56:05 UTC (rev 252122)
+++ trunk/Source/WebKit/ChangeLog	2019-11-06 05:17:37 UTC (rev 252123)
@@ -1,3 +1,27 @@
+2019-11-05  Kate Cheney  <katherine_che...@apple.com>
+
+        Layout test website-data-removal-for-site-navigated-to-with-link-decoration.html is a flaky failure
+        https://bugs.webkit.org/show_bug.cgi?id=203706
+        <rdar://problem/56801615>
+
+        Reviewed by Chris Dumez. 
+
+        No new tests, this change is tested by the existing resourceLoadStatistics
+        tests.
+
+        This test started flaking when a new memory store was being created
+        between tests to maintain consistency. The call to grandfatherExistingWebsiteData
+        from populateMemoryStoreFromDisk in the persistent storage was
+        async, causing a race condition that led to occasional failures.
+        Adding a completion handler and changing the callsite of
+        populateMemoryStoreFromDisk should fix this problem.
+
+        * NetworkProcess/Classifier/ResourceLoadStatisticsPersistentStorage.cpp:
+        (WebKit::ResourceLoadStatisticsPersistentStorage::populateMemoryStoreFromDisk):
+        * NetworkProcess/Classifier/ResourceLoadStatisticsPersistentStorage.h:
+        * NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp:
+        (WebKit::WebResourceLoadStatisticsStore::populateMemoryStoreFromDisk):
+
 2019-11-05  John Wilander  <wilan...@apple.com>
 
         Temporarily turn off NSURLSession isolation

Modified: trunk/Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsPersistentStorage.cpp (252122 => 252123)


--- trunk/Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsPersistentStorage.cpp	2019-11-06 03:56:05 UTC (rev 252122)
+++ trunk/Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsPersistentStorage.cpp	2019-11-06 05:17:37 UTC (rev 252123)
@@ -183,13 +183,13 @@
     m_lastStatisticsFileSyncTime = readTime;
 }
 
-void ResourceLoadStatisticsPersistentStorage::populateMemoryStoreFromDisk()
+void ResourceLoadStatisticsPersistentStorage::populateMemoryStoreFromDisk(CompletionHandler<void()>&& completionHandler)
 {
     ASSERT(!RunLoop::isMain());
 
     String filePath = resourceLogFilePath();
     if (filePath.isEmpty() || !FileSystem::fileExists(filePath)) {
-        m_memoryStore.grandfatherExistingWebsiteData([]() { });
+        m_memoryStore.grandfatherExistingWebsiteData(WTFMove(completionHandler));
         monitorDirectoryForNewStatistics();
         return;
     }
@@ -196,6 +196,7 @@
 
     if (!hasFileChangedSince(filePath, m_lastStatisticsFileSyncTime)) {
         // No need to grandfather in this case.
+        completionHandler();
         return;
     }
 
@@ -203,7 +204,7 @@
 
     auto decoder = createForFile(filePath);
     if (!decoder) {
-        m_memoryStore.grandfatherExistingWebsiteData([]() { });
+        m_memoryStore.grandfatherExistingWebsiteData(WTFMove(completionHandler));
         return;
     }
 
@@ -214,6 +215,7 @@
     m_lastStatisticsFileSyncTime = readTime;
 
     m_memoryStore.logTestingEvent("PopulatedWithoutGrandfathering"_s);
+    completionHandler();
 }
 
 void ResourceLoadStatisticsPersistentStorage::writeMemoryStoreToDisk()

Modified: trunk/Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsPersistentStorage.h (252122 => 252123)


--- trunk/Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsPersistentStorage.h	2019-11-06 03:56:05 UTC (rev 252122)
+++ trunk/Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsPersistentStorage.h	2019-11-06 05:17:37 UTC (rev 252123)
@@ -54,7 +54,7 @@
 
     enum class ForceImmediateWrite { No, Yes, };
     void scheduleOrWriteMemoryStore(ForceImmediateWrite);
-    void populateMemoryStoreFromDisk();
+    void populateMemoryStoreFromDisk(CompletionHandler<void()>&&);
 
 private:
     String storageDirectoryPathIsolatedCopy() const;

Modified: trunk/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp (252122 => 252123)


--- trunk/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp	2019-11-06 03:56:05 UTC (rev 252122)
+++ trunk/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp	2019-11-06 05:17:37 UTC (rev 252123)
@@ -225,11 +225,13 @@
 {
     ASSERT(RunLoop::isMain());
     
-    postTask([this, completionHandler = WTFMove(completionHandler)]() mutable {
+    postTask([this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)]() mutable {
         if (m_persistentStorage)
-            m_persistentStorage->populateMemoryStoreFromDisk();
-
-        postTaskReply(WTFMove(completionHandler));
+            m_persistentStorage->populateMemoryStoreFromDisk([protectedThis = WTFMove(protectedThis), completionHandler = WTFMove(completionHandler)]() mutable {
+                postTaskReply(WTFMove(completionHandler));
+            });
+        else
+            postTaskReply(WTFMove(completionHandler));
     });
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to