Title: [274040] trunk
Revision
274040
Author
wei...@apple.com
Date
2021-03-06 10:45:35 -0800 (Sat, 06 Mar 2021)

Log Message

Simplify DumpRenderTree WebPreferences reset
https://bugs.webkit.org/show_bug.cgi?id=218024
<rdar://problem/70764568>

Reviewed by Simon Fraser.

Source/WebKitLegacy/mac:

* WebView/WebPreferences.mm:
(WebPreferencesPrivate::WebPreferencesPrivate):
(-[WebPreferences encodeWithCoder:]):
(-[WebPreferences _valueForKey:]):
(-[WebPreferences _setStringValue:forKey:]):
(-[WebPreferences _setStringArrayValueForKey:forKey:]):
(-[WebPreferences _setIntegerValue:forKey:]):
(-[WebPreferences _setUnsignedIntValue:forKey:]):
(-[WebPreferences _setFloatValue:forKey:]):
(-[WebPreferences _setBoolValue:forKey:]):
(-[WebPreferences _setLongLongValue:forKey:]):
(-[WebPreferences _setUnsignedLongLongValue:forKey:]):
(-[WebPreferences _startBatchingUpdates]):
(-[WebPreferences _stopBatchingUpdates]):
(-[WebPreferences _batchUpdatePreferencesInBlock:]):
(-[WebPreferences _resetForTesting]):
(-[WebPreferences _postPreferencesChangedNotification]):
(-[WebPreferences _postPreferencesChangedAPINotification]):
(-[WebPreferences _invalidateCachedPreferences]):
(WebPreferencesPrivate::~WebPreferencesPrivate): Deleted.
* WebView/WebPreferencesPrivate.h:
Add new helpers for testing to reset and batch updates.

Tools:

Use new _batchUpdatePreferencesInBlock and _resetForTesting to simplify and
improve the performance of resetting preferences.

_batchUpdatePreferencesInBlock makes it so we only trigger the recalculation
of WebCore::Settings once per test, rather than linearly with the number of
settings that we reset.

_resetForTesting will allow us to remove explicit resetting of preferences
that use the existing default value (though this change does not utilize
that yet, to limit the number of changes in this commit).

* DumpRenderTree/mac/DumpRenderTree.mm:
(resetWebPreferencesToConsistentValues):
(setWebPreferencesForTestOptions):
(resetWebViewToConsistentState):

Modified Paths

Diff

Modified: trunk/Source/WebKitLegacy/mac/ChangeLog (274039 => 274040)


--- trunk/Source/WebKitLegacy/mac/ChangeLog	2021-03-06 18:21:25 UTC (rev 274039)
+++ trunk/Source/WebKitLegacy/mac/ChangeLog	2021-03-06 18:45:35 UTC (rev 274040)
@@ -1,3 +1,34 @@
+2021-03-06  Sam Weinig  <wei...@apple.com>
+
+        Simplify DumpRenderTree WebPreferences reset
+        https://bugs.webkit.org/show_bug.cgi?id=218024
+        <rdar://problem/70764568>
+
+        Reviewed by Simon Fraser.
+
+        * WebView/WebPreferences.mm:
+        (WebPreferencesPrivate::WebPreferencesPrivate):
+        (-[WebPreferences encodeWithCoder:]):
+        (-[WebPreferences _valueForKey:]):
+        (-[WebPreferences _setStringValue:forKey:]):
+        (-[WebPreferences _setStringArrayValueForKey:forKey:]):
+        (-[WebPreferences _setIntegerValue:forKey:]):
+        (-[WebPreferences _setUnsignedIntValue:forKey:]):
+        (-[WebPreferences _setFloatValue:forKey:]):
+        (-[WebPreferences _setBoolValue:forKey:]):
+        (-[WebPreferences _setLongLongValue:forKey:]):
+        (-[WebPreferences _setUnsignedLongLongValue:forKey:]):
+        (-[WebPreferences _startBatchingUpdates]):
+        (-[WebPreferences _stopBatchingUpdates]):
+        (-[WebPreferences _batchUpdatePreferencesInBlock:]):
+        (-[WebPreferences _resetForTesting]):
+        (-[WebPreferences _postPreferencesChangedNotification]):
+        (-[WebPreferences _postPreferencesChangedAPINotification]):
+        (-[WebPreferences _invalidateCachedPreferences]):
+        (WebPreferencesPrivate::~WebPreferencesPrivate): Deleted.
+        * WebView/WebPreferencesPrivate.h:
+        Add new helpers for testing to reset and batch updates.
+
 2021-03-05  Chris Dumez  <cdu...@apple.com>
 
         Update ApplicationCacheStorage::originsWithCache() to return a HashSet<SecurityOriginData> instead of a Vector<Ref<SecurityOrigin>>

Modified: trunk/Source/WebKitLegacy/mac/WebView/WebPreferences.mm (274039 => 274040)


--- trunk/Source/WebKitLegacy/mac/WebView/WebPreferences.mm	2021-03-06 18:21:25 UTC (rev 274039)
+++ trunk/Source/WebKitLegacy/mac/WebView/WebPreferences.mm	2021-03-06 18:45:35 UTC (rev 274040)
@@ -54,6 +54,7 @@
 #import <pal/spi/cf/CFNetworkSPI.h>
 #import <wtf/Compiler.h>
 #import <wtf/MainThread.h>
+#import <wtf/OptionSet.h>
 #import <wtf/RetainPtr.h>
 #import <wtf/RunLoop.h>
 #import <wtf/cocoa/RuntimeApplicationChecksCocoa.h>
@@ -191,36 +192,32 @@
 + (NSString *)_IBCreatorID;
 @end
 
+enum class UpdateAfterBatchType : uint8_t {
+    API         = 1 << 0,
+    Internal    = 1 << 1
+};
+
 struct WebPreferencesPrivate
 {
 public:
     WebPreferencesPrivate()
-    : inPrivateBrowsing(NO)
-    , autosaves(NO)
-    , automaticallyDetectsCacheModel(NO)
-    , numWebViews(0)
 #if PLATFORM(IOS_FAMILY)
-    , readWriteQueue(dispatch_queue_create("com.apple.WebPreferences.ReadWriteQueue", DISPATCH_QUEUE_CONCURRENT))
+        : readWriteQueue { adoptNS(dispatch_queue_create("com.apple.WebPreferences.ReadWriteQueue", DISPATCH_QUEUE_CONCURRENT)) }
 #endif
     {
     }
 
 #if PLATFORM(IOS_FAMILY)
-    ~WebPreferencesPrivate()
-    {
-        dispatch_release(readWriteQueue);
-    }
+    RetainPtr<dispatch_queue_t> readWriteQueue;
 #endif
-
     RetainPtr<NSMutableDictionary> values;
-    BOOL inPrivateBrowsing;
     RetainPtr<NSString> identifier;
-    BOOL autosaves;
-    BOOL automaticallyDetectsCacheModel;
-    unsigned numWebViews;
-#if PLATFORM(IOS_FAMILY)
-    dispatch_queue_t readWriteQueue;
-#endif
+    BOOL inPrivateBrowsing { NO };
+    BOOL autosaves { NO };
+    BOOL automaticallyDetectsCacheModel { NO };
+    unsigned numWebViews { 0 };
+    unsigned updateBatchCount { 0 };
+    OptionSet<UpdateAfterBatchType> updateAfterBatchType;
 };
 
 #if PLATFORM(IOS_FAMILY)
@@ -345,7 +342,7 @@
     if ([encoder allowsKeyedCoding]){
         [encoder encodeObject:_private->identifier.get() forKey:@"Identifier"];
 #if PLATFORM(IOS_FAMILY)
-        dispatch_sync(_private->readWriteQueue, ^{
+        dispatch_sync(_private->readWriteQueue.get(), ^{
 #endif
         [encoder encodeObject:_private->values.get() forKey:@"Values"];
         LOG (Encoding, "Identifier = %@, Values = %@\n", _private->identifier.get(), _private->values.get());
@@ -358,7 +355,7 @@
         [encoder encodeValueOfObjCType:@encode(int) at:&version];
         [encoder encodeObject:_private->identifier.get()];
 #if PLATFORM(IOS_FAMILY)
-        dispatch_sync(_private->readWriteQueue, ^{
+        dispatch_sync(_private->readWriteQueue.get(), ^{
 #endif
         [encoder encodeObject:_private->values.get()];
 #if PLATFORM(IOS_FAMILY)
@@ -467,7 +464,7 @@
     NSString *_key = KEY(key);
 #if PLATFORM(IOS_FAMILY)
     __block id o = nil;
-    dispatch_sync(_private->readWriteQueue, ^{
+    dispatch_sync(_private->readWriteQueue.get(), ^{
         o = [_private->values.get() objectForKey:_key];
     });
 #else
@@ -493,7 +490,7 @@
         return;
     NSString *_key = KEY(key);
 #if PLATFORM(IOS_FAMILY)
-    dispatch_barrier_sync(_private->readWriteQueue, ^{
+    dispatch_barrier_sync(_private->readWriteQueue.get(), ^{
 #endif
     [_private->values.get() setObject:value forKey:_key];
 #if PLATFORM(IOS_FAMILY)
@@ -523,7 +520,7 @@
 {
     NSString *_key = KEY(key);
 #if PLATFORM(IOS_FAMILY)
-    dispatch_barrier_sync(_private->readWriteQueue, ^{
+    dispatch_barrier_sync(_private->readWriteQueue.get(), ^{
 #endif
         [_private->values.get() setObject:value forKey:_key];
 #if PLATFORM(IOS_FAMILY)
@@ -546,7 +543,7 @@
         return;
     NSString *_key = KEY(key);
 #if PLATFORM(IOS_FAMILY)
-    dispatch_barrier_sync(_private->readWriteQueue, ^{
+    dispatch_barrier_sync(_private->readWriteQueue.get(), ^{
 #endif
     [_private->values.get() setObject:@(value) forKey:_key];
 #if PLATFORM(IOS_FAMILY)
@@ -569,7 +566,7 @@
         return;
     NSString *_key = KEY(key);
 #if PLATFORM(IOS_FAMILY)
-    dispatch_barrier_sync(_private->readWriteQueue, ^{
+    dispatch_barrier_sync(_private->readWriteQueue.get(), ^{
 #endif
     [_private->values.get() setObject:@(value) forKey:_key];
 #if PLATFORM(IOS_FAMILY)
@@ -592,7 +589,7 @@
         return;
     NSString *_key = KEY(key);
 #if PLATFORM(IOS_FAMILY)
-    dispatch_barrier_sync(_private->readWriteQueue, ^{
+    dispatch_barrier_sync(_private->readWriteQueue.get(), ^{
 #endif
     [_private->values.get() setObject:@(value) forKey:_key];
 #if PLATFORM(IOS_FAMILY)
@@ -614,7 +611,7 @@
         return;
     NSString *_key = KEY(key);
 #if PLATFORM(IOS_FAMILY)
-    dispatch_barrier_sync(_private->readWriteQueue, ^{
+    dispatch_barrier_sync(_private->readWriteQueue.get(), ^{
 #endif
     [_private->values.get() setObject:@(value) forKey:_key];
 #if PLATFORM(IOS_FAMILY)
@@ -637,7 +634,7 @@
         return;
     NSString *_key = KEY(key);
 #if PLATFORM(IOS_FAMILY)
-    dispatch_barrier_sync(_private->readWriteQueue, ^{
+    dispatch_barrier_sync(_private->readWriteQueue.get(), ^{
 #endif
     [_private->values.get() setObject:@(value) forKey:_key];
 #if PLATFORM(IOS_FAMILY)
@@ -660,7 +657,7 @@
         return;
     NSString *_key = KEY(key);
 #if PLATFORM(IOS_FAMILY)
-    dispatch_barrier_sync(_private->readWriteQueue, ^{
+    dispatch_barrier_sync(_private->readWriteQueue.get(), ^{
 #endif
     [_private->values.get() setObject:@(value) forKey:_key];
 #if PLATFORM(IOS_FAMILY)
@@ -1620,6 +1617,44 @@
         [self performSelector:@selector(_checkLastReferenceForIdentifier:) withObject:[self _concatenateKeyWithIBCreatorID:ident] afterDelay:0.1];
 }
 
+- (void)_startBatchingUpdates
+{
+    if (!_private->updateBatchCount)
+        _private->updateAfterBatchType = { };
+
+    _private->updateBatchCount++;
+}
+
+- (void)_stopBatchingUpdates
+{
+    ASSERT(_private->updateBatchCount > 0);
+    if (_private->updateBatchCount <= 0)
+        NSLog(@"ERROR: Unbalanced _startBatchingUpdates/_stopBatchingUpdates.");
+
+    _private->updateBatchCount--;
+    if (!_private->updateBatchCount) {
+        if (_private->updateAfterBatchType.contains(UpdateAfterBatchType::Internal)) {
+            if (_private->updateAfterBatchType.contains(UpdateAfterBatchType::API))
+                [self _postPreferencesChangedNotification];
+            else
+                [self _postPreferencesChangedAPINotification];
+        }
+    }
+}
+
+- (void)_batchUpdatePreferencesInBlock:(void (^)(WebPreferences *))block
+{
+    [self _startBatchingUpdates];
+    block(self);
+    [self _stopBatchingUpdates];
+}
+
+- (void)_resetForTesting
+{
+    _private->values = adoptNS([[NSMutableDictionary alloc] init]);
+    [self _postPreferencesChangedNotification];
+}
+
 - (void)_postPreferencesChangedNotification
 {
 #if !PLATFORM(IOS_FAMILY)
@@ -1629,6 +1664,11 @@
     }
 #endif
 
+    if (_private->updateBatchCount) {
+        _private->updateAfterBatchType.add({ UpdateAfterBatchType::API, UpdateAfterBatchType::Internal });
+        return;
+    }
+
     [[NSNotificationCenter defaultCenter] postNotificationName:WebPreferencesChangedInternalNotification object:self userInfo:nil];
     [[NSNotificationCenter defaultCenter] postNotificationName:WebPreferencesChangedNotification object:self userInfo:nil];
 }
@@ -1640,6 +1680,11 @@
         return;
     }
 
+    if (_private->updateBatchCount) {
+        _private->updateAfterBatchType.add({ UpdateAfterBatchType::Internal });
+        return;
+    }
+
     [[NSNotificationCenter defaultCenter] postNotificationName:WebPreferencesChangedNotification object:self userInfo:nil];
 }
 
@@ -2224,7 +2269,7 @@
 #if PLATFORM(IOS_FAMILY)
 - (void)_invalidateCachedPreferences
 {
-    dispatch_barrier_sync(_private->readWriteQueue, ^{
+    dispatch_barrier_sync(_private->readWriteQueue.get(), ^{
         if (_private->values)
             _private->values = adoptNS([[NSMutableDictionary alloc] init]);
     });

Modified: trunk/Source/WebKitLegacy/mac/WebView/WebPreferencesPrivate.h (274039 => 274040)


--- trunk/Source/WebKitLegacy/mac/WebView/WebPreferencesPrivate.h	2021-03-06 18:21:25 UTC (rev 274039)
+++ trunk/Source/WebKitLegacy/mac/WebView/WebPreferencesPrivate.h	2021-03-06 18:45:35 UTC (rev 274040)
@@ -86,6 +86,12 @@
 
 @interface WebPreferences (WebPrivate)
 
+- (void)_startBatchingUpdates;
+- (void)_stopBatchingUpdates;
+- (void)_batchUpdatePreferencesInBlock:(void (^)(WebPreferences *))block;
+
+- (void)_resetForTesting;
+
 - (void)_postPreferencesChangedNotification;
 - (void)_postPreferencesChangedAPINotification;
 + (WebPreferences *)_getInstanceForIdentifier:(NSString *)identifier;

Modified: trunk/Tools/ChangeLog (274039 => 274040)


--- trunk/Tools/ChangeLog	2021-03-06 18:21:25 UTC (rev 274039)
+++ trunk/Tools/ChangeLog	2021-03-06 18:45:35 UTC (rev 274040)
@@ -1,3 +1,27 @@
+2021-03-06  Sam Weinig  <wei...@apple.com>
+
+        Simplify DumpRenderTree WebPreferences reset
+        https://bugs.webkit.org/show_bug.cgi?id=218024
+        <rdar://problem/70764568>
+
+        Reviewed by Simon Fraser.
+
+        Use new _batchUpdatePreferencesInBlock and _resetForTesting to simplify and
+        improve the performance of resetting preferences.
+   
+        _batchUpdatePreferencesInBlock makes it so we only trigger the recalculation
+        of WebCore::Settings once per test, rather than linearly with the number of
+        settings that we reset.
+
+        _resetForTesting will allow us to remove explicit resetting of preferences 
+        that use the existing default value (though this change does not utilize
+        that yet, to limit the number of changes in this commit).
+ 
+        * DumpRenderTree/mac/DumpRenderTree.mm:
+        (resetWebPreferencesToConsistentValues):
+        (setWebPreferencesForTestOptions):
+        (resetWebViewToConsistentState):
+
 2021-03-05  Michael Catanzaro  <mcatanz...@gnome.org>
 
         [GTK] Clean up GTK-specific text checker stuff

Modified: trunk/Tools/DumpRenderTree/mac/DumpRenderTree.mm (274039 => 274040)


--- trunk/Tools/DumpRenderTree/mac/DumpRenderTree.mm	2021-03-06 18:21:25 UTC (rev 274039)
+++ trunk/Tools/DumpRenderTree/mac/DumpRenderTree.mm	2021-03-06 18:45:35 UTC (rev 274040)
@@ -859,6 +859,8 @@
 // Called before each test.
 static void resetWebPreferencesToConsistentValues(WebPreferences *preferences)
 {
+    [preferences _resetForTesting];
+
     for (WebFeature *feature in [WebPreferences _experimentalFeatures])
         [preferences _setEnabled:YES forFeature:feature];
 
@@ -950,14 +952,18 @@
 
 static void setWebPreferencesForTestOptions(WebPreferences *preferences, const WTR::TestOptions& options)
 {
-    preferences.privateBrowsingEnabled = options.useEphemeralSession();
+    [preferences _batchUpdatePreferencesInBlock:^(WebPreferences *preferences) {
+        resetWebPreferencesToConsistentValues(preferences);
 
-    // FIXME: Remove these once there is a viable mechanism for reseting WebPreferences between tests,
-    // at which point, we will not need to manually reset every supported preference for each test.
-    for (const auto& key : options.supportedBoolWebPreferenceFeatures())
-        [preferences _setBoolPreferenceForTestingWithValue:webPreferenceFeatureValue(key, options.boolWebPreferenceFeatures()) forKey:toNS(WTR::TestOptions::toWebKitLegacyPreferenceKey(key)).get()];
-    for (const auto& key : options.supportedUInt32WebPreferenceFeatures())
-        [preferences _setUInt32PreferenceForTestingWithValue:webPreferenceFeatureValue(key, options.uint32WebPreferenceFeatures()) forKey:toNS(WTR::TestOptions::toWebKitLegacyPreferenceKey(key)).get()];
+        preferences.privateBrowsingEnabled = options.useEphemeralSession();
+
+        // FIXME: Remove these once there is a viable mechanism for reseting WebPreferences between tests,
+        // at which point, we will not need to manually reset every supported preference for each test.
+        for (const auto& key : options.supportedBoolWebPreferenceFeatures())
+            [preferences _setBoolPreferenceForTestingWithValue:webPreferenceFeatureValue(key, options.boolWebPreferenceFeatures()) forKey:toNS(WTR::TestOptions::toWebKitLegacyPreferenceKey(key)).get()];
+        for (const auto& key : options.supportedUInt32WebPreferenceFeatures())
+            [preferences _setUInt32PreferenceForTestingWithValue:webPreferenceFeatureValue(key, options.uint32WebPreferenceFeatures()) forKey:toNS(WTR::TestOptions::toWebKitLegacyPreferenceKey(key)).get()];
+    }];
 }
 
 // Called once on DumpRenderTree startup.
@@ -1807,7 +1813,6 @@
 
     [WebCache clearCachedCredentials];
 
-    resetWebPreferencesToConsistentValues(webView.preferences);
     setWebPreferencesForTestOptions(webView.preferences, options);
 #if PLATFORM(MAC)
     [webView setWantsLayer:options.layerBackedWebView()];
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to