Bug 1876165 - Create BookmarkList class that internally stays up-to-date to indicate bookmarked open tabs in Fx View r=mak,fxview-reviewers,places-reviewers,fluent-reviewers,bolsson,sfoster,desktop-theme-reviewers

Differential Revision: https://phabricator.services.mozilla.com/D201759
This commit is contained in:
Jonathan Sudiaman 2024-03-01 13:34:08 +00:00
parent dfbd373993
commit 69051d0e86
10 changed files with 548 additions and 20 deletions

View File

@ -331,17 +331,21 @@ class OpenTabsTarget extends EventTarget {
return [];
}
/**
* Get an aggregated list of tabs from all the same-privateness browser windows.
*
* @returns {MozTabbrowserTab[]}
*/
getAllTabs() {
return this.currentWindows.flatMap(win => this.getTabsForWindow(win));
}
/*
* @returns {Array<Tab>}
* A by-recency-sorted, aggregated list of tabs from all the same-privateness browser windows.
*/
getRecentTabs() {
const tabs = [];
for (let win of this.currentWindows) {
tabs.push(...this.getTabsForWindow(win));
}
tabs.sort(lastSeenActiveSort);
return tabs;
return this.getAllTabs().sort(lastSeenActiveSort);
}
handleEvent({ detail, target, type }) {

View File

@ -729,6 +729,7 @@ export class FxviewTabRow extends MozLitElement {
pinned: this.indicators?.includes("pinned"),
pinnedOnNewTab: this.indicators?.includes("pinnedOnNewTab"),
attention: this.indicators?.includes("attention"),
bookmark: this.indicators?.includes("bookmark"),
})}"
>
<span

View File

@ -102,17 +102,17 @@
inset-inline-end: -6px;
}
&.pinnedOnNewTab .fxview-tab-row-favicon::after,
&.pinnedOnNewTab .fxview-tab-row-button::after {
background-image: url("chrome://browser/skin/pin-12.svg");
}
&.bookmark .fxview-tab-row-favicon::after,
&.bookmark .fxview-tab-row-button::after {
background-image: url("chrome://browser/skin/bookmark-12.svg");
fill: var(--fxview-primary-action-background);
}
&.pinnedOnNewTab .fxview-tab-row-favicon::after,
&.pinnedOnNewTab .fxview-tab-row-button::after {
background-image: url("chrome://browser/skin/pin-12.svg");
}
&.attention .fxview-tab-row-favicon::after,
&.attention .fxview-tab-row-button::after {
background-image: radial-gradient(circle, light-dark(rgb(42, 195, 162), rgb(84, 255, 189)), light-dark(rgb(42, 195, 162), rgb(84, 255, 189)) 2px, transparent 2px);

View File

@ -21,6 +21,7 @@ import { ViewPage, ViewPageContent } from "./viewpage.mjs";
const lazy = {};
ChromeUtils.defineESModuleGetters(lazy, {
BookmarkList: "resource://gre/modules/BookmarkList.sys.mjs",
ContextualIdentityService:
"resource://gre/modules/ContextualIdentityService.sys.mjs",
NewTabUtils: "resource://gre/modules/NewTabUtils.sys.mjs",
@ -107,6 +108,10 @@ class OpenTabsInView extends ViewPage {
this
);
}
this.bookmarkList = new lazy.BookmarkList(this.#getAllTabUrls(), () =>
this.viewCards.forEach(card => card.requestUpdate())
);
}
shouldUpdate(changedProperties) {
@ -142,6 +147,8 @@ class OpenTabsInView extends ViewPage {
this
);
}
this.bookmarkList.removeListeners();
}
viewVisibleCallback() {
@ -162,6 +169,13 @@ class OpenTabsInView extends ViewPage {
}
}
#getAllTabUrls() {
return this.openTabsTarget
.getAllTabs()
.map(({ linkedBrowser }) => linkedBrowser?.currentURI?.spec)
.filter(Boolean);
}
render() {
if (this.recentBrowsing) {
return this.getRecentBrowsingTemplate();
@ -269,6 +283,7 @@ class OpenTabsInView extends ViewPage {
winID: currentWindowIndex,
})}"
.searchQuery=${this.searchQuery}
.bookmarkList=${this.bookmarkList}
></view-opentabs-card>
`
)}
@ -283,6 +298,7 @@ class OpenTabsInView extends ViewPage {
data-l10n-id="firefoxview-opentabs-window-header"
data-l10n-args="${JSON.stringify({ winID })}"
.searchQuery=${this.searchQuery}
.bookmarkList=${this.bookmarkList}
></view-opentabs-card>
`
)}
@ -319,6 +335,7 @@ class OpenTabsInView extends ViewPage {
.recentBrowsing=${true}
.paused=${this.paused}
.searchQuery=${this.searchQuery}
.bookmarkList=${this.bookmarkList}
></view-opentabs-card>`;
}
@ -338,6 +355,7 @@ class OpenTabsInView extends ViewPage {
}
windowIds = detail.windowIds;
this._updateWindowList();
this.bookmarkList.setTrackedUrls(this.#getAllTabUrls());
break;
}
if (this.recentBrowsing) {
@ -391,6 +409,7 @@ class OpenTabsInViewCard extends ViewPageContent {
searchResults: { type: Array },
showAll: { type: Boolean },
cumulativeSearches: { type: Number },
bookmarkList: { type: Object },
};
static MAX_TABS_FOR_COMPACT_HEIGHT = 7;
@ -614,6 +633,28 @@ class OpenTabsInViewCard extends ViewPageContent {
? searchTabList(this.searchQuery, getTabListItems(this.tabs))
: null;
}
updated() {
this.updateBookmarkStars();
}
async updateBookmarkStars() {
const tabItems = [...this.tabList.tabItems];
for (const row of tabItems) {
const isBookmark = await this.bookmarkList.isBookmark(row.url);
if (isBookmark && !row.indicators.includes("bookmark")) {
row.indicators.push("bookmark");
}
if (!isBookmark && row.indicators.includes("bookmark")) {
row.indicators = row.indicators.filter(i => i !== "bookmark");
}
row.primaryL10nId = getPrimaryL10nId(
this.isRecentBrowsing,
row.indicators
);
}
this.tabList.tabItems = tabItems;
}
}
customElements.define("view-opentabs-card", OpenTabsInViewCard);
@ -902,8 +943,17 @@ function getIndicatorsForTab(tab) {
*/
function getPrimaryL10nId(isRecentBrowsing, tabIndicators) {
let indicatorL10nId = null;
if (tabIndicators?.includes("pinned") && !isRecentBrowsing) {
indicatorL10nId = "firefoxview-opentabs-pinned-tab";
if (!isRecentBrowsing) {
if (
tabIndicators?.includes("pinned") &&
tabIndicators?.includes("bookmark")
) {
indicatorL10nId = "firefoxview-opentabs-bookmarked-pinned-tab";
} else if (tabIndicators?.includes("pinned")) {
indicatorL10nId = "firefoxview-opentabs-pinned-tab";
} else if (tabIndicators?.includes("bookmark")) {
indicatorL10nId = "firefoxview-opentabs-bookmarked-tab";
}
}
return indicatorL10nId;
}
@ -921,13 +971,7 @@ function getPrimaryL10nId(isRecentBrowsing, tabIndicators) {
* L10n ID args
*/
function getPrimaryL10nArgs(tab, isRecentBrowsing, url) {
let indicatorArgs = null;
if (tab.pinned && !isRecentBrowsing) {
indicatorArgs = JSON.stringify({ tabTitle: tab.label });
} else {
indicatorArgs = JSON.stringify({ url });
}
return indicatorArgs;
return JSON.stringify({ tabTitle: tab.label, url });
}
/**

View File

@ -223,3 +223,90 @@ add_task(async function test_sound_playing_muted_indicator() {
cleanup();
});
});
add_task(async function test_bookmark_indicator() {
const tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, URLs[0]);
await withFirefoxView({}, async browser => {
const { document } = browser.contentWindow;
await navigateToViewAndWait(document, "opentabs");
const openTabs = document.querySelector("view-opentabs[name=opentabs]");
setSortOption(openTabs, "recency");
let rowEl = await TestUtils.waitForCondition(
() => openTabs.viewCards[0]?.tabList.rowEls[0]
);
info("Bookmark a tab while Firefox View is active.");
let bookmark = await PlacesUtils.bookmarks.insert({
parentGuid: PlacesUtils.bookmarks.toolbarGuid,
url: URLs[0],
});
await TestUtils.waitForCondition(
() => rowEl.shadowRoot.querySelector(".bookmark"),
"Tab shows the bookmark star."
);
await PlacesUtils.bookmarks.update({
guid: bookmark.guid,
url: URLs[1],
});
await TestUtils.waitForCondition(
() => !rowEl.shadowRoot.querySelector(".bookmark"),
"The bookmark star is removed."
);
bookmark = await PlacesUtils.bookmarks.insert({
parentGuid: PlacesUtils.bookmarks.toolbarGuid,
url: URLs[0],
});
await TestUtils.waitForCondition(
() => rowEl.shadowRoot.querySelector(".bookmark"),
"The bookmark star is restored."
);
await PlacesUtils.bookmarks.remove(bookmark.guid);
await TestUtils.waitForCondition(
() => !rowEl.shadowRoot.querySelector(".bookmark"),
"The bookmark star is removed again."
);
info("Bookmark a tab while Firefox View is inactive.");
await BrowserTestUtils.switchTab(gBrowser, tab);
bookmark = await PlacesUtils.bookmarks.insert({
parentGuid: PlacesUtils.bookmarks.toolbarGuid,
url: URLs[0],
});
await switchToFxViewTab();
await TestUtils.waitForCondition(
() => rowEl.shadowRoot.querySelector(".bookmark"),
"Tab shows the bookmark star."
);
await BrowserTestUtils.switchTab(gBrowser, tab);
await PlacesUtils.bookmarks.remove(bookmark.guid);
await switchToFxViewTab();
await TestUtils.waitForCondition(
() => !rowEl.shadowRoot.querySelector(".bookmark"),
"The bookmark star is removed."
);
info("Bookmark a tab in a new window.");
const win = await BrowserTestUtils.openNewBrowserWindow();
const tabChangeRaised = BrowserTestUtils.waitForEvent(
NonPrivateTabs,
"TabChange"
);
await BrowserTestUtils.openNewForegroundTab(win.gBrowser, URLs[2]);
await tabChangeRaised;
await openTabs.updateComplete;
rowEl = await TestUtils.waitForCondition(
() => openTabs.viewCards[1]?.tabList.rowEls[0]
);
await PlacesUtils.bookmarks.insert({
parentGuid: PlacesUtils.bookmarks.toolbarGuid,
url: URLs[2],
});
await TestUtils.waitForCondition(
() => rowEl.shadowRoot.querySelector(".bookmark"),
"Tab in the new window shows the bookmark star."
);
await BrowserTestUtils.closeWindow(win);
});
await cleanup();
await PlacesUtils.bookmarks.eraseEverything();
});

View File

@ -308,3 +308,15 @@ firefoxview-tabs =
firefoxview-opentabs-pinned-tab =
.title = Switch to { $tabTitle }
# This tooltip will be shown for a pinned tab whose URL is currently bookmarked.
firefoxview-opentabs-bookmarked-pinned-tab =
.title = Switch to (Bookmarked) { $tabTitle }
## These tooltips will be displayed when hovering over an unpinned Open Tab
## Variables:
## $url (string) - URL of tab that will be opened when selected
# This tooltip will be shown for an unpinned tab whose URL is currently bookmarked.
firefoxview-opentabs-bookmarked-tab =
.title = (Bookmarked) { $url }

View File

@ -0,0 +1,262 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */
const lazy = {};
ChromeUtils.defineESModuleGetters(lazy, {
DeferredTask: "resource://gre/modules/DeferredTask.sys.mjs",
PlacesUtils: "resource://gre/modules/PlacesUtils.sys.mjs",
});
const OBSERVER_DEBOUNCE_RATE_MS = 500;
const OBSERVER_DEBOUNCE_TIMEOUT_MS = 5000;
/**
* A collection of bookmarks that internally stays up-to-date in order to
* efficiently query whether certain URLs are bookmarked.
*/
export class BookmarkList {
/**
* The set of hashed URLs that need to be fetched from the database.
*
* @type {Set<string>}
*/
#urlsToFetch = new Set();
/**
* The function to call when changes are made.
*
* @type {function}
*/
#observer;
/**
* Cached mapping of hashed URLs to how many bookmarks they are used in.
*
* @type {Map<string, number>}
*/
#bookmarkCount = new Map();
/**
* Cached mapping of bookmark GUIDs to their respective URL hashes.
*
* @type {Map<string, string>}
*/
#guidToUrl = new Map();
/**
* @type {DeferredTask}
*/
#observerTask;
/**
* Construct a new BookmarkList.
*
* @param {string[]} urls
* The initial set of URLs to track.
* @param {function} [observer]
* The function to call when changes are made.
* @param {number} [debounceRate]
* Time between observer executions, in milliseconds.
* @param {number} [debounceTimeout]
* The maximum time to wait for an idle callback, in milliseconds.
*/
constructor(urls, observer, debounceRate, debounceTimeout) {
this.setTrackedUrls(urls);
this.#observer = observer;
this.handlePlacesEvents = this.handlePlacesEvents.bind(this);
this.addListeners(debounceRate, debounceTimeout);
}
/**
* Add places listeners to this bookmark list. The observer (if one was
* provided) will be called after processing any events.
*
* @param {number} [debounceRate]
* Time between observer executions, in milliseconds.
* @param {number} [debounceTimeout]
* The maximum time to wait for an idle callback, in milliseconds.
*/
addListeners(
debounceRate = OBSERVER_DEBOUNCE_RATE_MS,
debounceTimeout = OBSERVER_DEBOUNCE_TIMEOUT_MS
) {
lazy.PlacesUtils.observers.addListener(
["bookmark-added", "bookmark-removed", "bookmark-url-changed"],
this.handlePlacesEvents
);
this.#observerTask = new lazy.DeferredTask(
() => this.#observer?.(),
debounceRate,
debounceTimeout
);
}
/**
* Update the set of URLs to track.
*
* @param {string[]} urls
*/
async setTrackedUrls(urls) {
const updatedBookmarkCount = new Map();
for (const url of urls) {
// Use cached value if possible. Otherwise, it must be fetched from db.
const urlHash = lazy.PlacesUtils.history.hashURL(url);
const count = this.#bookmarkCount.get(urlHash);
if (count != undefined) {
updatedBookmarkCount.set(urlHash, count);
} else {
this.#urlsToFetch.add(urlHash);
}
}
this.#bookmarkCount = updatedBookmarkCount;
const updateGuidToUrl = new Map();
for (const [guid, urlHash] of this.#guidToUrl.entries()) {
if (updatedBookmarkCount.has(urlHash)) {
updateGuidToUrl.set(guid, urlHash);
}
}
this.#guidToUrl = updateGuidToUrl;
}
/**
* Check whether the given URL is bookmarked.
*
* @param {string} url
* @returns {boolean}
* The result, or `undefined` if the URL is not tracked.
*/
async isBookmark(url) {
if (this.#urlsToFetch.size) {
await this.#fetchTrackedUrls();
}
const urlHash = lazy.PlacesUtils.history.hashURL(url);
const count = this.#bookmarkCount.get(urlHash);
return count != undefined ? Boolean(count) : count;
}
/**
* Run the database query and populate the bookmarks cache with the URLs
* that are waiting to be fetched.
*/
async #fetchTrackedUrls() {
const urls = [...this.#urlsToFetch];
this.#urlsToFetch = new Set();
for (const urlHash of urls) {
this.#bookmarkCount.set(urlHash, 0);
}
const db = await lazy.PlacesUtils.promiseDBConnection();
for (const chunk of lazy.PlacesUtils.chunkArray(urls, db.variableLimit)) {
// Note that this query does not *explicitly* filter out tags, but we
// should not expect to find any, unless the db is somehow malformed.
const sql = `SELECT b.guid, p.url_hash
FROM moz_bookmarks b
JOIN moz_places p
ON b.fk = p.id
WHERE p.url_hash IN (${Array(chunk.length).fill("?").join(",")})`;
const rows = await db.executeCached(sql, chunk);
for (const row of rows) {
this.#cacheBookmark(
row.getResultByName("guid"),
row.getResultByName("url_hash")
);
}
}
}
/**
* Handle bookmark events and update the cache accordingly.
*
* @param {PlacesEvent[]} events
*/
async handlePlacesEvents(events) {
let cacheUpdated = false;
let needsFetch = false;
for (const { guid, type, url } of events) {
const urlHash = lazy.PlacesUtils.history.hashURL(url);
if (this.#urlsToFetch.has(urlHash)) {
needsFetch = true;
continue;
}
const isUrlTracked = this.#bookmarkCount.has(urlHash);
switch (type) {
case "bookmark-added":
if (isUrlTracked) {
this.#cacheBookmark(guid, urlHash);
cacheUpdated = true;
}
break;
case "bookmark-removed":
if (isUrlTracked) {
this.#removeCachedBookmark(guid, urlHash);
cacheUpdated = true;
}
break;
case "bookmark-url-changed": {
const oldUrlHash = this.#guidToUrl.get(guid);
if (oldUrlHash) {
this.#removeCachedBookmark(guid, oldUrlHash);
cacheUpdated = true;
}
if (isUrlTracked) {
this.#cacheBookmark(guid, urlHash);
cacheUpdated = true;
}
break;
}
}
}
if (needsFetch) {
await this.#fetchTrackedUrls();
cacheUpdated = true;
}
if (cacheUpdated) {
this.#observerTask.arm();
}
}
/**
* Remove places listeners from this bookmark list. URLs are no longer
* tracked.
*
* In order to resume tracking, you must call `setTrackedUrls()` followed by
* `addListeners()`.
*/
removeListeners() {
lazy.PlacesUtils.observers.removeListener(
["bookmark-added", "bookmark-removed", "bookmark-url-changed"],
this.handlePlacesEvents
);
if (!this.#observerTask.isFinalized) {
this.#observerTask.disarm();
this.#observerTask.finalize();
}
this.setTrackedUrls([]);
}
/**
* Store a bookmark in the cache.
*
* @param {string} guid
* @param {string} urlHash
*/
#cacheBookmark(guid, urlHash) {
const count = this.#bookmarkCount.get(urlHash);
this.#bookmarkCount.set(urlHash, count + 1);
this.#guidToUrl.set(guid, urlHash);
}
/**
* Remove a bookmark from the cache.
*
* @param {string} guid
* @param {string} urlHash
*/
#removeCachedBookmark(guid, urlHash) {
const count = this.#bookmarkCount.get(urlHash);
this.#bookmarkCount.set(urlHash, count - 1);
this.#guidToUrl.delete(guid);
}
}

View File

@ -60,6 +60,7 @@ if CONFIG["MOZ_PLACES"]:
EXTRA_JS_MODULES += [
"BookmarkHTMLUtils.sys.mjs",
"BookmarkJSONUtils.sys.mjs",
"BookmarkList.sys.mjs",
"Bookmarks.sys.mjs",
"ExtensionSearchHandler.sys.mjs",
"History.sys.mjs",

View File

@ -0,0 +1,115 @@
/* Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ */
"use strict";
const { sinon } = ChromeUtils.importESModule(
"resource://testing-common/Sinon.sys.mjs"
);
const { BookmarkList } = ChromeUtils.importESModule(
"resource://gre/modules/BookmarkList.sys.mjs"
);
registerCleanupFunction(
async () => await PlacesUtils.bookmarks.eraseEverything()
);
add_task(async function test_url_tracking() {
const firstBookmark = await PlacesUtils.bookmarks.insert({
parentGuid: PlacesUtils.bookmarks.toolbarGuid,
url: "https://www.example.com/",
});
const secondBookmark = await PlacesUtils.bookmarks.insert({
parentGuid: PlacesUtils.bookmarks.toolbarGuid,
url: "https://www.reddit.com/",
});
let deferredUpdate;
const bookmarkList = new BookmarkList(
[
"https://www.example.com/",
"https://www.reddit.com/",
"https://www.youtube.com/",
],
() => deferredUpdate?.resolve()
);
async function waitForUpdateBookmarksTask(updateTask) {
deferredUpdate = Promise.withResolvers();
await updateTask();
return deferredUpdate.promise;
}
info("Check bookmark status of tracked URLs.");
equal(await bookmarkList.isBookmark("https://www.example.com/"), true);
equal(await bookmarkList.isBookmark("https://www.reddit.com/"), true);
equal(await bookmarkList.isBookmark("https://www.youtube.com/"), false);
info("Add a bookmark.");
await waitForUpdateBookmarksTask(() =>
PlacesUtils.bookmarks.insert({
parentGuid: PlacesUtils.bookmarks.toolbarGuid,
url: "https://www.youtube.com/",
})
);
equal(await bookmarkList.isBookmark("https://www.youtube.com/"), true);
info("Remove a bookmark.");
await waitForUpdateBookmarksTask(() =>
PlacesUtils.bookmarks.remove(firstBookmark.guid)
);
equal(await bookmarkList.isBookmark("https://www.example.com/"), false);
info("Update a bookmark's URL.");
await waitForUpdateBookmarksTask(() =>
PlacesUtils.bookmarks.update({
guid: secondBookmark.guid,
url: "https://www.wikipedia.org/",
})
);
equal(await bookmarkList.isBookmark("https://www.reddit.com/"), false);
info("Add a bookmark after removing listeners.");
bookmarkList.removeListeners();
await PlacesUtils.bookmarks.insert({
parentGuid: PlacesUtils.bookmarks.toolbarGuid,
url: "https://www.example.com/",
});
info("Reinitialize the list and validate bookmark status.");
bookmarkList.setTrackedUrls(["https://www.example.com/"]);
bookmarkList.addListeners();
equal(await bookmarkList.isBookmark("https://www.example.com/"), true);
info("Cleanup.");
bookmarkList.removeListeners();
await PlacesUtils.bookmarks.eraseEverything();
});
add_task(async function test_no_unnecessary_observer_notifications() {
const spy = sinon.spy();
const bookmarkList = new BookmarkList(
["https://www.example.com/"],
spy,
0,
0
);
info("Add a bookmark with an untracked URL.");
await PlacesUtils.bookmarks.insert({
parentGuid: PlacesUtils.bookmarks.toolbarGuid,
url: "https://www.reddit.com/",
});
await new Promise(resolve => ChromeUtils.idleDispatch(resolve));
ok(spy.notCalled, "Observer was not notified.");
equal(await bookmarkList.isBookmark("https://www.reddit.com"), undefined);
info("Add a bookmark after removing listeners.");
bookmarkList.removeListeners();
await PlacesUtils.bookmarks.insert({
parentGuid: PlacesUtils.bookmarks.toolbarGuid,
url: "https://www.example.com/",
});
await new Promise(resolve => ChromeUtils.idleDispatch(resolve));
ok(spy.notCalled, "Observer was not notified.");
});

View File

@ -78,6 +78,8 @@ skip-if = ["os == 'linux'"] # Bug 821781
["test_bookmark-tags-changed_frequency.js"]
["test_bookmark_list.js"]
["test_bookmarks_html.js"]
["test_bookmarks_html_corrupt.js"]