Work towards <rdar://problem/47228232>
Created attachment 365312 [details] WIP
Created attachment 365314 [details] WIP
Created attachment 365343 [details] Work in progress
Comment on attachment 365343 [details] Work in progress Attachment 365343 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/11589360 New failing tests: http/wpt/mediarecorder/MediaRecorder-AV-audio-video-dataavailable.html
Created attachment 365461 [details] Archive of layout-test-results from ews104 for mac-highsierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Nope, changing course...
(In reply to Wenson Hsieh from comment #6) > Nope, changing course... 🔙
Created attachment 365998 [details] Patch
Comment on attachment 365998 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=365998&action=review > Source/WebKit/UIProcess/API/Cocoa/WKWebpagePreferences.mm:191 > + NSMutableDictionary *dictionary = [[[NSMutableDictionary alloc] initWithCapacity:fields.size()] autorelease]; RetainPtr? > Source/WebKit/UIProcess/API/Cocoa/WKWebpagePreferences.mm:193 > + [dictionary setObject:field.value() forKey:field.name()]; Or subscript operator? > Source/WebKit/UIProcess/API/Cocoa/WKWebpagePreferences.mm:252 > + return [NSString stringWithFormat:@"<%@: %p; contentBlockersEnabled = %d>", NSStringFromClass(self.class), self, self.contentBlockersEnabled]; Don't do this. It should only print API properties, and either all or none.
Comment on attachment 365998 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=365998&action=review >> Source/WebKit/UIProcess/API/Cocoa/WKWebpagePreferences.mm:191 >> + NSMutableDictionary *dictionary = [[[NSMutableDictionary alloc] initWithCapacity:fields.size()] autorelease]; > > RetainPtr? 👍🏻 >> Source/WebKit/UIProcess/API/Cocoa/WKWebpagePreferences.mm:193 >> + [dictionary setObject:field.value() forKey:field.name()]; > > Or subscript operator? Subscript operator gets a tiny bit annoying if I go with RetainPtr above :/ In other places, I think we might prefer just using -setObject:forKey: since subscripting would look like: dictionary.get()[(NSString *)field.name()] = field.value(); >> Source/WebKit/UIProcess/API/Cocoa/WKWebpagePreferences.mm:252 >> + return [NSString stringWithFormat:@"<%@: %p; contentBlockersEnabled = %d>", NSStringFromClass(self.class), self, self.contentBlockersEnabled]; > > Don't do this. It should only print API properties, and either all or none. Good call!
(In reply to Wenson Hsieh from comment #10) > Comment on attachment 365998 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=365998&action=review > > >> Source/WebKit/UIProcess/API/Cocoa/WKWebpagePreferences.mm:191 > >> + NSMutableDictionary *dictionary = [[[NSMutableDictionary alloc] initWithCapacity:fields.size()] autorelease]; > > > > RetainPtr? > > 👍🏻 > > >> Source/WebKit/UIProcess/API/Cocoa/WKWebpagePreferences.mm:193 > >> + [dictionary setObject:field.value() forKey:field.name()]; > > > > Or subscript operator? > > Subscript operator gets a tiny bit annoying if I go with RetainPtr above :/ > In other places, I think we might prefer just using -setObject:forKey: since > subscripting would look like: > > dictionary.get()[(NSString *)field.name()] = field.value(); Ah, yes, these are definitely mutually exclusive suggestions :) > >> Source/WebKit/UIProcess/API/Cocoa/WKWebpagePreferences.mm:252 > >> + return [NSString stringWithFormat:@"<%@: %p; contentBlockersEnabled = %d>", NSStringFromClass(self.class), self, self.contentBlockersEnabled]; > > > > Don't do this. It should only print API properties, and either all or none. > > Good call!
Created attachment 366238 [details] Patch
Comment on attachment 366238 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=366238&action=review > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:5133 > + auto data = [websitePolicies webPagePreferences]->_websitePolicies->data(); Dots? > Source/WebKit/UIProcess/API/Cocoa/WKWebpagePreferences.h:30 > + @helps Contains properties used to determine web page preferences. What's @helps for? I never know anything about this doc comment format. > Source/WebKit/UIProcess/API/Cocoa/WKWebpagePreferencesPrivate.h:69 > +@property (nonatomic) BOOL contentBlockersEnabled; > +@property (nonatomic) _WKWebsiteAutoplayQuirk allowedAutoplayQuirks; > +@property (nonatomic) _WKWebsiteAutoplayPolicy autoplayPolicy; > +@property (nonatomic, copy) NSDictionary<NSString *, NSString *> *customHeaderFields; > +@property (nonatomic) _WKWebsitePopUpPolicy popUpPolicy; > +@property (nonatomic, strong) WKWebsiteDataStore *websiteDataStore; > +@property (nonatomic, copy) NSString *customUserAgent; > +@property (nonatomic, copy) NSString *customJavaScriptUserAgentAsSiteSpecificQuirks; > +@property (nonatomic, copy) NSString *customNavigatorPlatform; > +@property (nonatomic) _WKWebsiteDeviceOrientationAndMotionAccessPolicy deviceOrientationAndMotionAccessPolicy; These all need to gain underscores now that they're on a public class, right? > Source/WebKit/UIProcess/API/Cocoa/_WKWebsitePolicies.mm:43 > +- (WKWebpagePreferences *)webPagePreferences Capitalization consistency!
Comment on attachment 366238 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=366238&action=review >> Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:5133 >> + auto data = [websitePolicies webPagePreferences]->_websitePolicies->data(); > > Dots? More dots! >> Source/WebKit/UIProcess/API/Cocoa/WKWebpagePreferences.h:30 >> + @helps Contains properties used to determine web page preferences. > > What's @helps for? I never know anything about this doc comment format. Oh, interesting. I took this from WKWebViewConfiguration's public header. As far as I can tell though, WKWebViewConfiguration.h is the only place in the entire public Cocoa API surface (macOS and iOS) that uses @helps, so it might be a mistake. I changed this to @discussion instead, which is much more commonly used. >> Source/WebKit/UIProcess/API/Cocoa/WKWebpagePreferencesPrivate.h:69 >> +@property (nonatomic) _WKWebsiteDeviceOrientationAndMotionAccessPolicy deviceOrientationAndMotionAccessPolicy; > > These all need to gain underscores now that they're on a public class, right? They rather should, but I was hoping to do that later, to maintain bincompat with Safari. I could also just add the underscore-prefixed versions here in this patch, alongside these non-underscore-prefixed versions. I'll go ahead and do that, so we can get Safari on the underscore-prefixed versions sooner, and then finally remove these existing ones. >> Source/WebKit/UIProcess/API/Cocoa/_WKWebsitePolicies.mm:43 >> +- (WKWebpagePreferences *)webPagePreferences > > Capitalization consistency! Fixed!
Comment on attachment 366238 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=366238&action=review >>> Source/WebKit/UIProcess/API/Cocoa/WKWebpagePreferencesPrivate.h:69 >>> +@property (nonatomic) _WKWebsiteDeviceOrientationAndMotionAccessPolicy deviceOrientationAndMotionAccessPolicy; >> >> These all need to gain underscores now that they're on a public class, right? > > They rather should, but I was hoping to do that later, to maintain bincompat with Safari. > > I could also just add the underscore-prefixed versions here in this patch, alongside these non-underscore-prefixed versions. I'll go ahead and do that, so we can get Safari on the underscore-prefixed versions sooner, and then finally remove these existing ones. Nope, never mind, this is on the new WKWebpagePreferences so this should totally be _-prefixed from the get-go.
Created attachment 366431 [details] Take 2
Created attachment 366439 [details] Fix a * on the wrong side
Comment on attachment 366439 [details] Fix a * on the wrong side Clearing flags on attachment: 366439 Committed r243726: <https://trac.webkit.org/changeset/243726>
All reviewed patches have been landed. Closing bug.