[webkit-reviews] review granted: [Bug 221802] Update data path for App Highlights : [Attachment 420381] Patch

[bugzilla-daemon at webkit.org]

Tim Horton <thorton at apple.com> has granted Megan Gardner
<megan_gardner at apple.com>'s request for review:
Bug 221802: Update data path for App Highlights
https://bugs.webkit.org/show_bug.cgi?id=221802

Attachment 420381: Patch

https://bugs.webkit.org/attachment.cgi?id=420381&action=review




--- Comment #8 from Tim Horton <thorton at apple.com> ---
Comment on attachment 420381
  --> https://bugs.webkit.org/attachment.cgi?id=420381
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=420381&action=review

Please fix the title, still.

> Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:1434
> -	   [delegate _webView:self updateAppHighlightsStorage:data];
> +	   [delegate _webView:self
updateAppHighlightsStorage:highlightInformation.highlight->createNSData().get()
];

This should become "_webView:didCreateAppHighlight:" eventually.

> Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:2044
> +    for (NSData * highlight in highlights) {

Star is floating

> Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:2046
> +	   if (sharedMemory) {

Wonder if "just move on and ignore it" is the right thing here, or if we should
bail entirely, or what?

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:33
> +

Don't think this newline belongs here!