Bug 221123

Summary: Add plumbing to allow AppHighlights to be restored.
Product: WebKit Reporter: Megan Gardner <megan_gardner>
Component: New BugsAssignee: Megan Gardner <megan_gardner>
Status: RESOLVED FIXED    
Severity: Normal CC: hi, thorton, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Megan Gardner
Reported 2021-01-28 23:56:26 PST
Add plumbing to allow AppHighlights to be restored.
Attachments
Patch (9.26 KB, patch)
2021-01-29 00:00 PST, Megan Gardner 		no flags
DetailsFormatted DiffDiff
Patch (9.21 KB, patch)
2021-01-29 09:36 PST, Megan Gardner 		no flags
DetailsFormatted DiffDiff
Patch for landing (9.20 KB, patch)
2021-01-29 18:03 PST, Megan Gardner 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Megan Gardner
Comment 1 2021-01-29 00:00:06 PST
Created attachment 418710 [details] Patch
Tim Horton
Comment 2 2021-01-29 00:07:37 PST
Comment on attachment 418710 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=418710&action=review > Source/WebCore/Modules/highlight/AppHighlightStorage.cpp:255 > + return; You can just delete this now > Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h:366 > +- (void)_restoreAppHighlights:(NSData *)data WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA)); I would put a newline between this and the family of methods above. > Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm:547 > + IPC::SharedBufferDataReference dataReference {data}; Spaces inside the { } > Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm:550 > + if (!hasRunningProcess()) > + return; Seems like this should come first. > Source/WebKit/WebProcess/WebPage/WebPage.cpp:7191 > +void WebPage::restoreAppHighlights(const IPC::DataReference& data) Newline before this.
Tim Horton
Comment 3 2021-01-29 00:09:28 PST
Comment on attachment 418710 [details] Patch I'm not sure all the SharedBuffer machinations are right, it would be good if someone who'd touched it recently could peek (things look slightly different than I remember).
Wenson Hsieh
Comment 4 2021-01-29 08:27:10 PST
Comment on attachment 418710 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=418710&action=review LGTM with Tim's comments. > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:2048 > + _page->restoreAppHighlights(WTFMove(buffer)); Nit - I would just write this as `_page->restoreAppHighlights(WebCore::SharedBuffer::create(data));`.
Megan Gardner
Comment 5 2021-01-29 09:36:01 PST
Created attachment 418738 [details] Patch
Megan Gardner
Comment 6 2021-01-29 18:03:22 PST
Created attachment 418796 [details] Patch for landing
EWS
Comment 7 2021-01-29 18:39:29 PST
Committed r272096: <https://trac.webkit.org/changeset/272096> All reviewed patches have been landed. Closing bug and clearing flags on attachment 418796 [details].
Radar WebKit Bug Importer
Comment 8 2021-01-29 18:41:29 PST
<rdar://problem/73781161>
Note You need to log in before you can comment on or make changes to this bug.