Skip to content

Web Extensions: Parse getRecent() bookmark function #47086

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jun 26, 2025
Merged

Web Extensions: Parse getRecent() bookmark function #47086

merged 1 commit into from
Jun 26, 2025
+110 −8

Conversation

akutira-umich
Copy link
Contributor

@akutira-umich akutira-umich commented Jun 23, 2025 •
edited by webkit-early-warning-system
Loading

6663438

Web Extensions: Parse getRecent() bookmark function
rdar://153924872
https://bugs.webkit.org/show_bug.cgi?id=294766

Reviewed by Timothy Hatcher.

Added Parsing for getRecent that uses mock BookmarkTreeNodes created by Create()
to test its functionality. Added tests for this as well.

* Source/WebKit/WebProcess/Extensions/API/Cocoa/WebExtensionAPIBookmarksCocoa.mm:
(WebKit::WebExtensionAPIBookmarks::createDictionaryFromNode):
(WebKit::WebExtensionAPIBookmarks::createBookmark):
(WebKit::WebExtensionAPIBookmarks::getRecent):
* Source/WebKit/WebProcess/Extensions/API/WebExtensionAPIBookmarks.h:
* Source/WebKit/WebProcess/Extensions/Interfaces/WebExtensionAPIBookmarks.idl:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebExtensionAPIBookmarks.mm:
(TestWebKitAPI::TEST_F(WKWebExtensionAPIBookmarks, BookmarksAPICheckgetRecent)):
(TestWebKitAPI::TEST_F(WKWebExtensionAPIBookmarks, BookmarksAPIMockNodeWithgetRecent)):

Canonical link: https://commits.webkit.org/296640@main

66e36a3

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
❌ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe   🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug   🧪 wpe-wk2   🧪 win-tests
✅ 🧪 webkitperl   🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
  🧪 ios-wk2-wpt ✅ 🧪 mac-wk1   🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision   🧪 mac-AS-debug-wk2   🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2   🛠 playstation
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv   🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 23, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 878f98b)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
❌ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ❌ 🧪 ios-wk2 ✅ 🧪 api-mac ❌ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2   🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

AriYoung00
AriYoung00 suggested changes Jun 24, 2025
View reviewed changes
Copy link
Contributor

@AriYoung00 AriYoung00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes based on previous comments

xeenon
xeenon requested changes Jun 24, 2025
View reviewed changes
Source/WebKit/WebProcess/Extensions/API/Cocoa/WebExtensionAPIBookmarksCocoa.mm Outdated
@@ -35,9 +35,50 @@

namespace WebKit {

NSDictionary* WebExtensionAPIBookmarks::createDictionaryFromNode(const MockBookmarkNode& node)
{
double msSinceEpoch = (node.dateAdded + kCFAbsoluteTimeIntervalSince1970) * 1000.0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should use toImpl() from CocoaHelpers.h.

WallTime toImpl(NSDate *);

Then toImpl(node.dateAdded).secondsSinceEpoch().milliseconds().

Source/WebKit/WebProcess/Extensions/API/WebExtensionAPIBookmarks.h Outdated
std::string id;
std::string title;
std::string url;
CFAbsoluteTime dateAdded;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer WTF::WallTime.

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 24, 2025
@akutira-umich akutira-umich force-pushed the eng/Web-Extensions-Parse-getRecent-bookmark-function branch from 878f98b to 5e559ac Compare June 24, 2025 18:04
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 24, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 5e559ac)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
❌ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ⏳ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug   🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision   🧪 mac-AS-debug-wk2   🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2   🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@akutira-umich akutira-umich force-pushed the eng/Web-Extensions-Parse-getRecent-bookmark-function branch from 5e559ac to 256aa88 Compare June 24, 2025 18:53
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 24, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 256aa88)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
❌ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2   🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

xeenon
xeenon requested changes Jun 24, 2025
View reviewed changes
@akutira-umich akutira-umich force-pushed the eng/Web-Extensions-Parse-getRecent-bookmark-function branch from 256aa88 to 9a37c49 Compare June 24, 2025 22:35
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 24, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 9a37c49)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
❌ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2   🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

xeenon
xeenon requested changes Jun 25, 2025
View reviewed changes
@akutira-umich akutira-umich force-pushed the eng/Web-Extensions-Parse-getRecent-bookmark-function branch from 9a37c49 to f4e8617 Compare June 25, 2025 21:04
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 25, 2025 •
edited
Loading

EWS run on previous version of this PR (hash f4e8617)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
❌ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim   🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
⏳ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ⏳ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
loading 🛠 vision ❌ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ⏳ 🛠 playstation
✅ 🛠 tv   🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

xeenon
xeenon requested changes Jun 25, 2025
View reviewed changes
Source/WebKit/WebProcess/Extensions/API/Cocoa/WebExtensionAPIBookmarksCocoa.mm Outdated
Comment on lines 57 to 65
NSArray *requiredKeys = @[ @"id", @"url", @"dateAdded" ];
NSDictionary *types = @{
WebExtensionBookmarkIDKey: [NSString class],
WebExtensionBookmarkURLKey: [NSString class],
WebExtensionBookmarkDateAddedKey: [NSNumber class],
WebExtensionBookmarkTitleKey: [NSString class]
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use constants defined above here. NSString.class, NSNumber.class.

Source/WebKit/WebProcess/Extensions/API/Cocoa/WebExtensionAPIBookmarksCocoa.mm Outdated
MockBookmarkNode newNode;
newNode.id = bookmark[@"id"];
newNode.url = bookmark[@"url"];
id dateAddedValue = bookmark[@"dateAdded"];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for a local. Should use auto * instead of id.

@akutira-umich akutira-umich force-pushed the eng/Web-Extensions-Parse-getRecent-bookmark-function branch from f4e8617 to 66e36a3 Compare June 25, 2025 22:55
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 25, 2025 •
edited
Loading

EWS run on current version of this PR (hash 66e36a3)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
❌ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe   🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug   🧪 wpe-wk2   🧪 win-tests
✅ 🧪 webkitperl   🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
  🧪 ios-wk2-wpt ✅ 🧪 mac-wk1   🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision   🧪 mac-AS-debug-wk2   🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2   🛠 playstation
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv   🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@xeenon xeenon added unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing WebKit Extensions Bugs related to extension support. and removed merging-blocked Applied to prevent a change from being merged labels Jun 26, 2025
@akutira-umich @xeenon
Web Extensions: Parse getRecent() bookmark function
6663438 
rdar://153924872
https://bugs.webkit.org/show_bug.cgi?id=294766

Reviewed by Timothy Hatcher.

Added Parsing for getRecent that uses mock BookmarkTreeNodes created by Create()
to test its functionality. Added tests for this as well.

* Source/WebKit/WebProcess/Extensions/API/Cocoa/WebExtensionAPIBookmarksCocoa.mm:
(WebKit::WebExtensionAPIBookmarks::createDictionaryFromNode):
(WebKit::WebExtensionAPIBookmarks::createBookmark):
(WebKit::WebExtensionAPIBookmarks::getRecent):
* Source/WebKit/WebProcess/Extensions/API/WebExtensionAPIBookmarks.h:
* Source/WebKit/WebProcess/Extensions/Interfaces/WebExtensionAPIBookmarks.idl:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebExtensionAPIBookmarks.mm:
(TestWebKitAPI::TEST_F(WKWebExtensionAPIBookmarks, BookmarksAPICheckgetRecent)):
(TestWebKitAPI::TEST_F(WKWebExtensionAPIBookmarks, BookmarksAPIMockNodeWithgetRecent)):

Canonical link: https://commits.webkit.org/296640@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Web-Extensions-Parse-getRecent-bookmark-function branch from 66e36a3 to 6663438 Compare June 26, 2025 00:17
@webkit-commit-queue
Copy link
Collaborator

Committed 296640@main (6663438): https://commits.webkit.org/296640@main

Reviewed commits have been landed. Closing PR #47086 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 6663438 into WebKit:main Jun 26, 2025
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jun 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AriYoung00 AriYoung00 AriYoung00 requested changes

@xeenon xeenon xeenon approved these changes

@b-weinstein b-weinstein Awaiting requested review from b-weinstein b-weinstein is a code owner

Assignees

@akutira-umich akutira-umich

Labels
WebKit Extensions Bugs related to extension support.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@akutira-umich @webkit-early-warning-system @webkit-commit-queue @xeenon @AriYoung00 @webkit-ews-buildbot