WebKit Bugzilla
New
Browse
Search+
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
221123
Add plumbing to allow AppHighlights to be restored.
https://bugs.webkit.org/show_bug.cgi?id=221123
Summary
Add plumbing to allow AppHighlights to be restored.
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
Details
Formatted Diff
Diff
Patch
(9.21 KB, patch)
2021-01-29 09:36 PST
,
Megan Gardner
no flags
Details
Formatted Diff
Diff
Patch for landing
(9.20 KB, patch)
2021-01-29 18:03 PST
,
Megan Gardner
no flags
Details
Formatted Diff
Diff
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.
Top of Page
Format For Printing
XML
Clone This Bug