Add IconLoadingDelegate functionality to WKView This is a followup to https://trac.webkit.org/changeset/209640
Created attachment 298630 [details] Patch
Attachment 298630 [details] did not pass style-queue: ERROR: Source/WebKit2/UIProcess/API/mac/WKView.mm:872: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebKit2/UIProcess/API/mac/WKView.mm:872: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/UIProcess/API/mac/WKView.mm:875: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebKit2/UIProcess/API/mac/WKView.mm:875: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 4 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 298630 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=298630&action=review > Source/WebKit2/UIProcess/API/mac/WKView.mm:877 > + completionHandler([loadCompletionHandler = Block_copy(loadCompletionHandler)](API::Data* data, WebKit::CallbackBase::Error error) { Plz to BlockPtr.
Created attachment 298636 [details] Patch
Running EWS before landing
Attachment 298636 [details] did not pass style-queue: ERROR: Source/WebKit2/UIProcess/API/mac/WKView.mm:873: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebKit2/UIProcess/API/mac/WKView.mm:873: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/UIProcess/API/mac/WKView.mm:876: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebKit2/UIProcess/API/mac/WKView.mm:876: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/UIProcess/API/mac/WKView.mm:878: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 5 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 298654 [details] Patch
Attachment 298654 [details] did not pass style-queue: ERROR: Source/WebKit2/UIProcess/API/mac/WKView.mm:873: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebKit2/UIProcess/API/mac/WKView.mm:873: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/UIProcess/API/mac/WKView.mm:876: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebKit2/UIProcess/API/mac/WKView.mm:876: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/UIProcess/API/mac/WKView.mm:878: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 5 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 298659 [details] Patch
Attachment 298659 [details] did not pass style-queue: ERROR: Source/WebKit2/UIProcess/API/mac/WKView.mm:874: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebKit2/UIProcess/API/mac/WKView.mm:874: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/UIProcess/API/mac/WKView.mm:877: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebKit2/UIProcess/API/mac/WKView.mm:877: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/UIProcess/API/mac/WKView.mm:879: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 5 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
https://trac.webkit.org/changeset/210624
Comment on attachment 298659 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=298659&action=review > Source/WebKit2/UIProcess/API/mac/WKView.mm:868 > + return sel_registerName("_shouldLoadIconWithParameters:completionHandler:"); Doesn’t seem good to keep calling sel_registerName every time this function is called. On a related note, why not use @selector instead of an explicit call to set_registerName? > Source/WebKit2/UIProcess/API/mac/WKView.mm:872 > + typedef void (^IconLoadCompletionHandler)(NSData*); In new code, I suggest using instead of typedef. Also, coding style document says we should use a space between NSData and the *. > Source/WebKit2/UIProcess/API/mac/WKView.mm:875 > + RetainPtr<_WKLinkIconParameters> parameters = adoptNS([[_WKLinkIconParameters alloc] _initWithLinkIcon:linkIcon]); This is an example of a place where auto looks much better to me than writing out RetainPtr<xxx>. > Source/WebKit2/UIProcess/API/mac/WKView.mm:877 > + [m_wkView performSelector:delegateSelector() withObject:parameters.get() withObject:^void (IconLoadCompletionHandler loadCompletionHandler) { Is the ".get() needed here? > Source/WebKit2/UIProcess/API/mac/WKView.mm:890 > + WKView *m_wkView; What guarantees this pointer is not used after the WKView has been deallocated?