2042100 - Batch fetchBookmarksByURL calls in Bookmarks.insert tag flow

Status: RESOLVED INVALID

(Toolkit :: Places, task, P3)

Description

[Marco Bonardo [:mak]]

Bookmarks.insert calls fetchBookmarksByURL per item during the tag flow. Same shape as the other two caller-batching bugs.

Comment 1

[CalebChan]

Looking at Bookmarks.insert() in toolkit/components/places/Bookmarks.sys.mjs (around line 365), the tag-notification flow only issues a single fetchBookmarksByURL(item, {concurrent: true}) call per insert() invocation:

if (isTagging) {
for (let entry of await fetchBookmarksByURL(item, {
concurrent: true,
})) {
notifications.push(...)
}
}

The for loop here iterates over the entries returned by that one call (other bookmarks that share the same URL), not over multiple fetchBookmarksByURL calls. Since insert() only ever handles a single bookmark per invocation, there's no per-item loop of round-trips to batch this differs from bug 2042098 (removeFoldersContents, loops over multiple descendants) and bug 2042099 (Bookmarks.remove, loops over multiple removed items), which do have that shape.

Unless I'm missing a caller-side loop that repeatedly invokes insert() for the same batch of URLs (in which case the fix would need to live at that call site, not inside insert() itself), I don't think there's a fetchBookmarksByURLs batching opportunity within insert() as currently written. Let me know if you had a different call site in mind.

Comment 2

[Marco Bonardo [:mak]]

Agree, I asked Claude to split bugs and likely this was an hallucination