From 411e522def473e329934d8c3cc2e64a0e7b6912c Mon Sep 17 00:00:00 2001 From: Lina Cambridge Date: Tue, 27 Aug 2019 20:25:05 +0000 Subject: [PATCH 01/41] Bug 1576796 - Report local and remote tree sizes in bookmarks mirror shutdown hang crash reports. r=tcsc Differential Revision: https://phabricator.services.mozilla.com/D43575 --HG-- extra : moz-landing-system : lando --- .../places/SyncedBookmarksMirror.jsm | 47 ++++++++---- .../tests/sync/test_bookmark_abort_merging.js | 75 ++++++++++++++++++- .../sync/test_bookmark_structure_changes.js | 10 +-- 3 files changed, 108 insertions(+), 24 deletions(-) diff --git a/toolkit/components/places/SyncedBookmarksMirror.jsm b/toolkit/components/places/SyncedBookmarksMirror.jsm index 3c8d0c055a21..cc180ef99034 100644 --- a/toolkit/components/places/SyncedBookmarksMirror.jsm +++ b/toolkit/components/places/SyncedBookmarksMirror.jsm @@ -147,9 +147,19 @@ class ProgressTracker { * @param {String} name A step name from `ProgressTracker.STEPS`. This is * included in shutdown hang crash reports, along with the timestamp * the step was recorded. + * @param {Number} [took] The time taken, in milliseconds. + * @param {Array} [counts] An array of additional counts to report in the + * shutdown blocker state. */ - step(name) { - this.steps.push({ step: name, at: Date.now() }); + step(name, took = -1, counts = null) { + let info = { step: name, at: Date.now() }; + if (took > -1) { + info.took = took; + } + if (counts) { + info.counts = counts; + } + this.steps.push(info); } /** @@ -161,7 +171,7 @@ class ProgressTracker { * record in telemetry for this step. */ stepWithTelemetry(name, took, counts = null) { - this.step(name); + this.step(name, took, counts); this.recordStepTelemetry(name, took, counts); } @@ -645,23 +655,25 @@ class SyncedBookmarksMirror { ) ); - return withTiming( + let { changeRecords } = await withTiming( "Fetching records for local items to upload", async () => { try { - let changeRecords = await this.fetchLocalChangeRecords(signal); - return changeRecords; + let result = await this.fetchLocalChangeRecords(signal); + return result; } finally { await this.db.execute(`DELETE FROM itemsToUpload`); } }, - (time, records) => + (time, result) => this.progress.stepWithItemCount( ProgressTracker.STEPS.FETCH_LOCAL_CHANGE_RECORDS, time, - Object.keys(records).length + result.count ) ); + + return changeRecords; } merge( @@ -1066,9 +1078,11 @@ class SyncedBookmarksMirror { * @param {AbortSignal} signal * Stops fetching records when the associated `AbortController` * is aborted. - * @return {Object.} - * A changeset containing Sync record cleartexts for outgoing items - * and tombstones, keyed by their Sync record IDs. + * @return {Object} + * A `{ changeRecords, count }` tuple, where `changeRecords` is a + * changeset containing Sync record cleartexts for outgoing items and + * tombstones, keyed by their Sync record IDs, and `count` is the + * number of records. */ async fetchLocalChangeRecords(signal) { let changeRecords = {}; @@ -1299,7 +1313,7 @@ class SyncedBookmarksMirror { yieldState ); - return changeRecords; + return { changeRecords, count: itemRows.length }; } /** @@ -2446,7 +2460,14 @@ async function updateFrecencies(db, limit) { } function bagToNamedCounts(bag, names) { - return names.map(name => ({ name, count: bag.getProperty(name) })); + let counts = []; + for (let name of names) { + let count = bag.getProperty(name); + if (count > 0) { + counts.push({ name, count }); + } + } + return counts; } /** diff --git a/toolkit/components/places/tests/sync/test_bookmark_abort_merging.js b/toolkit/components/places/tests/sync/test_bookmark_abort_merging.js index f0b0cdc73694..acad627001dd 100644 --- a/toolkit/components/places/tests/sync/test_bookmark_abort_merging.js +++ b/toolkit/components/places/tests/sync/test_bookmark_abort_merging.js @@ -52,9 +52,80 @@ add_task(async function test_blocker_state() { await barrier.wait(); let state = buf.progress.fetchState(); - let steps = state.steps; + let names = []; + for (let s of state.steps) { + equal(typeof s.at, "number", `Should report timestamp for ${s.step}`); + switch (s.step) { + case "fetchLocalTree": + greaterOrEqual( + s.took, + 0, + "Should report time taken to fetch local tree" + ); + deepEqual( + s.counts, + [{ name: "items", count: 6 }], + "Should report number of items in local tree" + ); + break; + + case "fetchRemoteTree": + greaterOrEqual( + s.took, + 0, + "Should report time taken to fetch remote tree" + ); + deepEqual( + s.counts, + [{ name: "items", count: 6 }], + "Should report number of items in remote tree" + ); + break; + + case "merge": + greaterOrEqual(s.took, 0, "Should report time taken to merge"); + deepEqual( + s.counts, + [{ name: "items", count: 6 }], + "Should report merge stats" + ); + break; + + case "apply": + greaterOrEqual(s.took, 0, "Should report time taken to apply"); + ok(!("counts" in s), "Should not report counts for applying"); + break; + + case "notifyObservers": + greaterOrEqual( + s.took, + 0, + "Should report time taken to notify observers" + ); + ok(!("counts" in s), "Should not report counts for observers"); + break; + + case "fetchLocalChangeRecords": + greaterOrEqual( + s.took, + 0, + "Should report time taken to fetch records for upload" + ); + deepEqual( + s.counts, + [{ name: "items", count: 4 }], + "Should report number of records to upload" + ); + break; + + case "finalize": + ok(!("took" in s), "Should not report time taken to finalize"); + ok(!("counts" in s), "Should not report counts for finalizing"); + } + names.push(s.step); + } deepEqual( - steps.map(s => s.step), + names, [ "fetchLocalTree", "fetchRemoteTree", diff --git a/toolkit/components/places/tests/sync/test_bookmark_structure_changes.js b/toolkit/components/places/tests/sync/test_bookmark_structure_changes.js index ae8641a3c818..603a4d979d2f 100644 --- a/toolkit/components/places/tests/sync/test_bookmark_structure_changes.js +++ b/toolkit/components/places/tests/sync/test_bookmark_structure_changes.js @@ -911,15 +911,7 @@ add_task(async function test_complex_move_with_additions() { ); deepEqual( mergeTelemetryCounts, - [ - { name: "items", count: 10 }, - { name: "deletes", count: 0 }, - { name: "dupes", count: 0 }, - { name: "remoteRevives", count: 0 }, - { name: "localDeletes", count: 0 }, - { name: "localRevives", count: 0 }, - { name: "remoteDeletes", count: 0 }, - ], + [{ name: "items", count: 10 }], "Should record telemetry with structure change counts" ); From a115057f796defa78e1444e7f59cb63acabcde24 Mon Sep 17 00:00:00 2001 From: Lina Cambridge Date: Tue, 27 Aug 2019 20:24:12 +0000 Subject: [PATCH 02/41] Bug 1576810 - Avoid temp B-trees when fetching bookmarks mirror observer notifications. r=tcsc The queries we use to fetch observer notifications need to sort all rows in every notification table. This is expensive, so this commit avoids sorting by adding indexes on item levels to all tables, and removing the parent and position from the `ORDER BY` clause in non-test code. Differential Revision: https://phabricator.services.mozilla.com/D43576 --HG-- extra : moz-landing-system : lando --- .../places/SyncedBookmarksMirror.jsm | 88 +++++++++++++------ .../sync/test_bookmark_observer_recorder.js | 1 + .../sync/test_bookmark_structure_changes.js | 13 ++- .../tests/sync/test_bookmark_value_changes.js | 16 +++- 4 files changed, 82 insertions(+), 36 deletions(-) diff --git a/toolkit/components/places/SyncedBookmarksMirror.jsm b/toolkit/components/places/SyncedBookmarksMirror.jsm index cc180ef99034..0266510019a9 100644 --- a/toolkit/components/places/SyncedBookmarksMirror.jsm +++ b/toolkit/components/places/SyncedBookmarksMirror.jsm @@ -555,6 +555,11 @@ class SyncedBookmarksMirror { * the next set of URLs after the next merge, or all remaining URLs * when Places automatically fixes invalid frecencies on idle; * whichever comes first. + * @param {Boolean} [options.notifyInStableOrder] + * If `true`, fire observer notifications for items in the same folder + * in a stable order. This is disabled by default, to avoid the cost + * of sorting the notifications, but enabled in some tests to simplify + * their checks. * @param {AbortSignal} [options.signal] * An abort signal that can be used to interrupt a merge when its * associated `AbortController` is aborted. If omitted, the merge can @@ -569,6 +574,7 @@ class SyncedBookmarksMirror { remoteTimeSeconds, weakUpload, maxFrecenciesToRecalculate, + notifyInStableOrder, signal = null, } = {}) { // We intentionally don't use `executeBeforeShutdown` in this function, @@ -588,7 +594,8 @@ class SyncedBookmarksMirror { localTimeSeconds, remoteTimeSeconds, weakUpload, - maxFrecenciesToRecalculate + maxFrecenciesToRecalculate, + notifyInStableOrder ); } finally { this.progress.reset(); @@ -602,7 +609,8 @@ class SyncedBookmarksMirror { localTimeSeconds, remoteTimeSeconds, weakUpload, - maxFrecenciesToRecalculate = DEFAULT_MAX_FRECENCIES_TO_RECALCULATE + maxFrecenciesToRecalculate = DEFAULT_MAX_FRECENCIES_TO_RECALCULATE, + notifyInStableOrder = false ) { let wasMerged = await withTiming("Merging bookmarks in Rust", () => this.merge(signal, localTimeSeconds, remoteTimeSeconds, weakUpload) @@ -620,6 +628,7 @@ class SyncedBookmarksMirror { let observersToNotify = new BookmarkObserverRecorder(this.db, { maxFrecenciesToRecalculate, signal, + notifyInStableOrder, }); await withTiming( @@ -1838,12 +1847,15 @@ async function initializeTempMirrorEntities(db) { level INTEGER NOT NULL DEFAULT -1 ) WITHOUT ROWID`); + await db.execute(`CREATE INDEX addedItemLevels ON itemsAdded(level)`); + await db.execute(`CREATE TEMP TABLE guidsChanged( - itemId INTEGER NOT NULL, + itemId INTEGER PRIMARY KEY, oldGuid TEXT NOT NULL, - level INTEGER NOT NULL DEFAULT -1, - PRIMARY KEY(itemId, oldGuid) - ) WITHOUT ROWID`); + level INTEGER NOT NULL DEFAULT -1 + )`); + + await db.execute(`CREATE INDEX changedGuidLevels ON guidsChanged(level)`); await db.execute(`CREATE TEMP TABLE itemsChanged( itemId INTEGER PRIMARY KEY, @@ -1853,6 +1865,8 @@ async function initializeTempMirrorEntities(db) { level INTEGER NOT NULL DEFAULT -1 )`); + await db.execute(`CREATE INDEX changedItemLevels ON itemsChanged(level)`); + await db.execute(`CREATE TEMP TABLE itemsMoved( itemId INTEGER PRIMARY KEY, oldParentId INTEGER NOT NULL, @@ -1861,9 +1875,11 @@ async function initializeTempMirrorEntities(db) { level INTEGER NOT NULL DEFAULT -1 )`); + await db.execute(`CREATE INDEX movedItemLevels ON itemsMoved(level)`); + await db.execute(`CREATE TEMP TABLE itemsRemoved( - guid TEXT PRIMARY KEY, - itemId INTEGER NOT NULL, + itemId INTEGER PRIMARY KEY, + guid TEXT NOT NULL, parentId INTEGER NOT NULL, position INTEGER NOT NULL, type INTEGER NOT NULL, @@ -1873,7 +1889,11 @@ async function initializeTempMirrorEntities(db) { can notify children before parents. */ level INTEGER NOT NULL DEFAULT -1, isUntagging BOOLEAN NOT NULL DEFAULT 0 - ) WITHOUT ROWID`); + )`); + + await db.execute( + `CREATE INDEX removedItemLevels ON itemsRemoved(level DESC)` + ); // Stores locally changed items staged for upload. await db.execute(`CREATE TEMP TABLE itemsToUpload( @@ -1901,6 +1921,10 @@ async function initializeTempMirrorEntities(db) { position INTEGER NOT NULL ) WITHOUT ROWID`); + await db.execute( + `CREATE INDEX parentsToUpload ON structureToUpload(parentId, position)` + ); + await db.execute(`CREATE TEMP TABLE tagsToUpload( id INTEGER REFERENCES itemsToUpload(id) ON DELETE CASCADE, @@ -2006,9 +2030,10 @@ async function withTiming(name, func, recordTiming) { * the merge. */ class BookmarkObserverRecorder { - constructor(db, { maxFrecenciesToRecalculate, signal }) { + constructor(db, { maxFrecenciesToRecalculate, notifyInStableOrder, signal }) { this.db = db; this.maxFrecenciesToRecalculate = maxFrecenciesToRecalculate; + this.notifyInStableOrder = notifyInStableOrder; this.signal = signal; this.placesEvents = []; this.itemRemovedNotifications = []; @@ -2037,6 +2062,12 @@ class BookmarkObserverRecorder { await updateFrecencies(this.db, this.maxFrecenciesToRecalculate); } + orderBy(level, parent, position) { + return `ORDER BY ${ + this.notifyInStableOrder ? `${level}, ${parent}, ${position}` : level + }`; + } + /** * Records Places observer notifications for removed, added, moved, and * changed items. @@ -2044,15 +2075,13 @@ class BookmarkObserverRecorder { async noteAllChanges() { MirrorLog.trace("Recording observer notifications for removed items"); // `ORDER BY v.level DESC` sorts deleted children before parents, to ensure - // that we update caches in the correct order (bug 1297941). We also order - // by parent and position so that the notifications are well-ordered for - // tests. + // that we update caches in the correct order (bug 1297941). await this.db.execute( `SELECT v.itemId AS id, v.parentId, v.parentGuid, v.position, v.type, - h.url, v.guid, v.isUntagging + (SELECT h.url FROM moz_places h WHERE h.id = v.placeId) AS url, + v.guid, v.isUntagging FROM itemsRemoved v - LEFT JOIN moz_places h ON h.id = v.placeId - ORDER BY v.level DESC, v.parentId, v.position`, + ${this.orderBy("v.level", "v.parentId", "v.position")}`, null, (row, cancel) => { if (this.signal.aborted) { @@ -2085,7 +2114,7 @@ class BookmarkObserverRecorder { FROM guidsChanged c JOIN moz_bookmarks b ON b.id = c.itemId JOIN moz_bookmarks p ON p.id = b.parent - ORDER BY c.level, p.id, b.position`, + ${this.orderBy("c.level", "b.parent", "b.position")}`, null, (row, cancel) => { if (this.signal.aborted) { @@ -2112,14 +2141,14 @@ class BookmarkObserverRecorder { MirrorLog.trace("Recording observer notifications for new items"); await this.db.execute( - `SELECT b.id, p.id AS parentId, b.position, b.type, h.url, - IFNULL(b.title, '') AS title, b.dateAdded, b.guid, - p.guid AS parentGuid, n.isTagging, n.keywordChanged + `SELECT b.id, p.id AS parentId, b.position, b.type, + (SELECT h.url FROM moz_places h WHERE h.id = b.fk) AS url, + IFNULL(b.title, '') AS title, b.dateAdded, b.guid, + p.guid AS parentGuid, n.isTagging, n.keywordChanged FROM itemsAdded n JOIN moz_bookmarks b ON b.guid = n.guid JOIN moz_bookmarks p ON p.id = b.parent - LEFT JOIN moz_places h ON h.id = b.fk - ORDER BY n.level, p.id, b.position`, + ${this.orderBy("n.level", "b.parent", "b.position")}`, null, (row, cancel) => { if (this.signal.aborted) { @@ -2154,12 +2183,12 @@ class BookmarkObserverRecorder { await this.db.execute( `SELECT b.id, b.guid, b.type, p.id AS newParentId, c.oldParentId, p.guid AS newParentGuid, c.oldParentGuid, - b.position AS newPosition, c.oldPosition, h.url + b.position AS newPosition, c.oldPosition, + (SELECT h.url FROM moz_places h WHERE h.id = b.fk) AS url FROM itemsMoved c JOIN moz_bookmarks b ON b.id = c.itemId JOIN moz_bookmarks p ON p.id = b.parent - LEFT JOIN moz_places h ON h.id = b.fk - ORDER BY c.level, newParentId, newPosition`, + ${this.orderBy("c.level", "b.parent", "b.position")}`, null, (row, cancel) => { if (this.signal.aborted) { @@ -2192,15 +2221,16 @@ class BookmarkObserverRecorder { `SELECT b.id, b.guid, b.lastModified, b.type, IFNULL(b.title, '') AS newTitle, IFNULL(c.oldTitle, '') AS oldTitle, - h.url AS newURL, i.url AS oldURL, + (SELECT h.url FROM moz_places h + WHERE h.id = b.fk) AS newURL, + (SELECT h.url FROM moz_places h + WHERE h.id = c.oldPlaceId) AS oldURL, p.id AS parentId, p.guid AS parentGuid, c.keywordChanged FROM itemsChanged c JOIN moz_bookmarks b ON b.id = c.itemId JOIN moz_bookmarks p ON p.id = b.parent - LEFT JOIN moz_places h ON h.id = b.fk - LEFT JOIN moz_places i ON i.id = c.oldPlaceId - ORDER BY c.level, p.id, b.position`, + ${this.orderBy("c.level", "b.parent", "b.position")}`, null, (row, cancel) => { if (this.signal.aborted) { diff --git a/toolkit/components/places/tests/sync/test_bookmark_observer_recorder.js b/toolkit/components/places/tests/sync/test_bookmark_observer_recorder.js index be98c076ce49..7cbe81b5fb64 100644 --- a/toolkit/components/places/tests/sync/test_bookmark_observer_recorder.js +++ b/toolkit/components/places/tests/sync/test_bookmark_observer_recorder.js @@ -411,6 +411,7 @@ add_task(async function test_apply_then_revert() { let secondTimeRecords = await buf.apply({ localTimeSeconds, remoteTimeSeconds: now, + notifyInStableOrder: true, }); deepEqual( await buf.fetchUnmergedGuids(), diff --git a/toolkit/components/places/tests/sync/test_bookmark_structure_changes.js b/toolkit/components/places/tests/sync/test_bookmark_structure_changes.js index 603a4d979d2f..c857b7fadead 100644 --- a/toolkit/components/places/tests/sync/test_bookmark_structure_changes.js +++ b/toolkit/components/places/tests/sync/test_bookmark_structure_changes.js @@ -123,6 +123,7 @@ add_task(async function test_value_structure_conflict() { let observer = expectBookmarkChangeNotifications(); let changesToUpload = await buf.apply({ remoteTimeSeconds: Date.now() / 1000, + notifyInStableOrder: true, }); deepEqual( await buf.fetchUnmergedGuids(), @@ -375,7 +376,9 @@ add_task(async function test_move() { info("Apply remote"); let observer = expectBookmarkChangeNotifications(); - let changesToUpload = await buf.apply(); + let changesToUpload = await buf.apply({ + notifyInStableOrder: true, + }); deepEqual(await buf.fetchUnmergedGuids(), [], "Should merge all items"); let idsToUpload = inspectChangeRecords(changesToUpload); @@ -695,7 +698,9 @@ add_task(async function test_move_into_parent_sibling() { info("Apply remote"); let observer = expectBookmarkChangeNotifications(); - let changesToUpload = await buf.apply(); + let changesToUpload = await buf.apply({ + notifyInStableOrder: true, + }); deepEqual(await buf.fetchUnmergedGuids(), [], "Should merge all items"); let idsToUpload = inspectChangeRecords(changesToUpload); @@ -903,7 +908,9 @@ add_task(async function test_complex_move_with_additions() { info("Apply remote"); let observer = expectBookmarkChangeNotifications(); - let changesToUpload = await buf.apply(); + let changesToUpload = await buf.apply({ + notifyInStableOrder: true, + }); deepEqual( await buf.fetchUnmergedGuids(), ["folderAAAAAA"], diff --git a/toolkit/components/places/tests/sync/test_bookmark_value_changes.js b/toolkit/components/places/tests/sync/test_bookmark_value_changes.js index d688fa91a4da..e58954af37c3 100644 --- a/toolkit/components/places/tests/sync/test_bookmark_value_changes.js +++ b/toolkit/components/places/tests/sync/test_bookmark_value_changes.js @@ -105,7 +105,9 @@ add_task(async function test_value_combo() { info("Apply remote"); let observer = expectBookmarkChangeNotifications({ skipTags: true }); - let changesToUpload = await buf.apply(); + let changesToUpload = await buf.apply({ + notifyInStableOrder: true, + }); deepEqual( await buf.fetchUnmergedGuids(), [PlacesUtils.bookmarks.toolbarGuid], @@ -1197,7 +1199,9 @@ add_task(async function test_keywords_complex() { info("Apply remote"); let observer = expectBookmarkChangeNotifications(); - let changesToUpload = await buf.apply(); + let changesToUpload = await buf.apply({ + notifyInStableOrder: true, + }); deepEqual( await buf.fetchUnmergedGuids(), ["bookmarkAAA1", "bookmarkAAAA", "bookmarkBBB1"], @@ -1901,7 +1905,9 @@ add_task(async function test_date_added() { info("Apply remote"); let observer = expectBookmarkChangeNotifications(); - let changesToUpload = await buf.apply(); + let changesToUpload = await buf.apply({ + notifyInStableOrder: true, + }); deepEqual(await buf.fetchUnmergedGuids(), [], "Should merge all items"); let idsToUpload = inspectChangeRecords(changesToUpload); @@ -2091,7 +2097,9 @@ add_task(async function test_duplicate_url_rows() { info("Apply mirror"); let observer = expectBookmarkChangeNotifications(); - let changesToUpload = await buf.apply(); + let changesToUpload = await buf.apply({ + notifyInStableOrder: true, + }); deepEqual( await buf.fetchUnmergedGuids(), [ From 5c982b86d763983e0d6b7dcafd6dab72f963a848 Mon Sep 17 00:00:00 2001 From: Lina Cambridge Date: Tue, 27 Aug 2019 20:24:21 +0000 Subject: [PATCH 03/41] Bug 1575757 - Don't use `WITH RECURSIVE` to build the local tree. r=tcsc This commit replaces the `localItems` CTE with a single table scan that fetches item and structure info in one pass. It also adds a `structurePositions` index, so that fetching the remote structure can use a covering index and avoid an extra sorting step. Differential Revision: https://phabricator.services.mozilla.com/D43577 --HG-- extra : moz-landing-system : lando --- Cargo.lock | 6 +- third_party/rust/dogear/.cargo-checksum.json | 2 +- third_party/rust/dogear/Cargo.toml | 2 +- third_party/rust/dogear/src/merge.rs | 150 ++++++++++-------- third_party/rust/dogear/src/tests.rs | 78 ++++++++- third_party/rust/dogear/src/tree.rs | 98 ++++++------ .../places/SyncedBookmarksMirror.jsm | 10 +- .../places/bookmark_sync/Cargo.toml | 2 +- .../places/bookmark_sync/src/store.rs | 84 +++++----- .../sync/test_bookmark_mirror_migration.js | 33 +++- 10 files changed, 307 insertions(+), 158 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 12b9b9dcaae5..40dec37080cf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -385,7 +385,7 @@ version = "0.1.0" dependencies = [ "atomic_refcell 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "cstr 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)", - "dogear 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)", + "dogear 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.60 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", "moz_task 0.1.0", @@ -956,7 +956,7 @@ dependencies = [ [[package]] name = "dogear" -version = "0.3.1" +version = "0.3.3" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", @@ -3947,7 +3947,7 @@ dependencies = [ "checksum devd-rs 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "0d009f166c0d9e9f9909dc751630b3a6411ab7f85a153d32d01deb364ffe52a7" "checksum digest 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)" = "05f47366984d3ad862010e22c7ce81a7dbcaebbdfb37241a620f8b6596ee135c" "checksum dirs 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)" = "88972de891f6118092b643d85a0b28e0678e0f948d7f879aa32f2d5aafe97d2a" -"checksum dogear 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)" = "251a15c9a597d70eb53cbb0c5473d8d8c6241aef615c092030ebab27fb5b26ef" +"checksum dogear 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)" = "57cd6ee785daa898686f3e2fb4a2b1ce490fcd6d69665c857d16fb61b48f4aae" "checksum dtoa 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)" = "09c3753c3db574d215cba4ea76018483895d7bff25a31b49ba45db21c48e50ab" "checksum dtoa-short 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)" = "068d4026697c1a18f0b0bb8cfcad1b0c151b90d8edb9bf4c235ad68128920d1d" "checksum dwrote 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)" = "0bd1369e02db5e9b842a9b67bce8a2fcc043beafb2ae8a799dd482d46ea1ff0d" diff --git a/third_party/rust/dogear/.cargo-checksum.json b/third_party/rust/dogear/.cargo-checksum.json index 7c83ae4fea0d..61399f5009c2 100644 --- a/third_party/rust/dogear/.cargo-checksum.json +++ b/third_party/rust/dogear/.cargo-checksum.json @@ -1 +1 @@ -{"files":{"CODE_OF_CONDUCT.md":"e85149c44f478f164f7d5f55f6e66c9b5ae236d4a11107d5e2a93fe71dd874b9","Cargo.toml":"cfc0416b0a1c11ade3b5c11ec579c8e26ed3fc3b5871dd99d4fbcc12bd2614d0","LICENSE":"c71d239df91726fc519c6eb72d318ec65820627232b2f796219e87dcf35d0ab4","README.md":"303ea5ec53d4e86f2c321056e8158e31aa061353a99e52de3d76859d40919efc","src/driver.rs":"913de08a578b38902f4eb0d78147289897af5d3b1ecdad818c4ea6f83da6c714","src/error.rs":"d4ef0cba5c7fc54959ed62da166f10435548d705e0a817eed449fb001fe4e21d","src/guid.rs":"c82af64fba3ad87948a9b599241e48753d17587e8c642f610949163be3d499bf","src/lib.rs":"0606e69b235650bf404ae0b03a1e85c2063bb4b7147fa4d5e8ff2c128a757453","src/merge.rs":"e74727171831585d304eee3a3c7ba24fec52a5e7c6999ed4e647fefa14b8be99","src/store.rs":"629410e44485c0473617911be0e06dc5ce504c7427782be02f05a2f4096b5351","src/tests.rs":"51bc77eb989974f7594dfa0cdf7741f8e26f2956eebc34d1f71cbfd137512940","src/tree.rs":"fcdc9a5ae0e3f1b0d62d01bf1024db2a0aa8660e6839ada607a6a20e0fe2ad83"},"package":"251a15c9a597d70eb53cbb0c5473d8d8c6241aef615c092030ebab27fb5b26ef"} \ No newline at end of file +{"files":{"CODE_OF_CONDUCT.md":"e85149c44f478f164f7d5f55f6e66c9b5ae236d4a11107d5e2a93fe71dd874b9","Cargo.toml":"9c145c9ecdad0143f95fb00d5646fb3f46203779a201f2f2389961563ca37397","LICENSE":"c71d239df91726fc519c6eb72d318ec65820627232b2f796219e87dcf35d0ab4","README.md":"303ea5ec53d4e86f2c321056e8158e31aa061353a99e52de3d76859d40919efc","src/driver.rs":"913de08a578b38902f4eb0d78147289897af5d3b1ecdad818c4ea6f83da6c714","src/error.rs":"d4ef0cba5c7fc54959ed62da166f10435548d705e0a817eed449fb001fe4e21d","src/guid.rs":"c82af64fba3ad87948a9b599241e48753d17587e8c642f610949163be3d499bf","src/lib.rs":"0606e69b235650bf404ae0b03a1e85c2063bb4b7147fa4d5e8ff2c128a757453","src/merge.rs":"bfc83aa6f42c196a5c6c75e095d1e44470198a38f3a62a6537cd5854d51208cd","src/store.rs":"629410e44485c0473617911be0e06dc5ce504c7427782be02f05a2f4096b5351","src/tests.rs":"fd2f03fd47e02459b7a3c7f799896be71536054184a2eca7e30cdbe817da0a97","src/tree.rs":"30c7d0c80f180936d1f99d2a3fbe9add36f3fe36a64998e3176d0242f425c488"},"package":"57cd6ee785daa898686f3e2fb4a2b1ce490fcd6d69665c857d16fb61b48f4aae"} \ No newline at end of file diff --git a/third_party/rust/dogear/Cargo.toml b/third_party/rust/dogear/Cargo.toml index 047a33b54179..c66194fe10f5 100644 --- a/third_party/rust/dogear/Cargo.toml +++ b/third_party/rust/dogear/Cargo.toml @@ -13,7 +13,7 @@ [package] edition = "2018" name = "dogear" -version = "0.3.1" +version = "0.3.3" authors = ["Lina Cambridge "] exclude = ["/.travis/**", ".travis.yml", "/docs/**", "book.toml"] description = "A library for merging bookmark trees." diff --git a/third_party/rust/dogear/src/merge.rs b/third_party/rust/dogear/src/merge.rs index 86705107dbb6..ec86bb82e486 100644 --- a/third_party/rust/dogear/src/merge.rs +++ b/third_party/rust/dogear/src/merge.rs @@ -1528,47 +1528,44 @@ impl<'t, D: Driver, A: AbortSignal> Merger<'t, D, A> { for (local_position, local_child_node) in local_parent_node.children().enumerate() { self.signal.err_if_aborted()?; if local_child_node.is_built_in_root() { - continue; - } - if let Some(local_child_content) = local_child_node.content() { - if let Some(remote_child_node) = - self.remote_tree.node_for_guid(&local_child_node.guid) - { - trace!( - self.driver, - "Not deduping local child {}; already exists remotely as {}", - local_child_node, - remote_child_node - ); - continue; - } - if self.remote_tree.is_deleted(&local_child_node.guid) { - trace!( - self.driver, - "Not deduping local child {}; deleted remotely", - local_child_node - ); - continue; - } - // Store matching local children in an array, in case multiple children - // have the same dupe key (for example, a toolbar containing multiple - // empty folders, as in bug 1213369). - let dupe_key = match local_child_content { - Content::Bookmark { .. } | Content::Folder { .. } => { - DupeKey::WithoutPosition(local_child_content) - } - Content::Separator => { - DupeKey::WithPosition(local_child_content, local_position) - } - }; - let local_nodes_for_key = dupe_key_to_local_nodes.entry(dupe_key).or_default(); - local_nodes_for_key.push_back(local_child_node); - } else { trace!( self.driver, - "Not deduping local child {}; already uploaded", + "Not deduping local built-in root {}", local_child_node ); + continue; + } + if self.remote_tree.mentions(&local_child_node.guid) { + trace!( + self.driver, + "Not deduping local child {}; already deleted or exists remotely", + local_child_node + ); + continue; + } + match local_child_node.content() { + Some(local_child_content) => { + // Store matching local children in an array, in case multiple children + // have the same dupe key (for example, a toolbar containing multiple + // empty folders, as in bug 1213369). + let dupe_key = match local_child_content { + Content::Bookmark { .. } | Content::Folder { .. } => { + DupeKey::WithoutPosition(local_child_content) + } + Content::Separator => { + DupeKey::WithPosition(local_child_content, local_position) + } + }; + let local_nodes_for_key = dupe_key_to_local_nodes.entry(dupe_key).or_default(); + local_nodes_for_key.push_back(local_child_node); + } + None => { + trace!( + self.driver, + "Not deduping local child {} without content info", + local_child_node + ); + } } } @@ -1577,6 +1574,22 @@ impl<'t, D: Driver, A: AbortSignal> Merger<'t, D, A> { for (remote_position, remote_child_node) in remote_parent_node.children().enumerate() { self.signal.err_if_aborted()?; + if remote_child_node.is_built_in_root() { + trace!( + self.driver, + "Not deduping remote built-in root {}", + remote_child_node + ); + continue; + } + if self.local_tree.mentions(&remote_child_node.guid) { + trace!( + self.driver, + "Not deduping remote child {}; already deleted or exists locally", + remote_child_node + ); + continue; + } if remote_to_local.contains_key(&remote_child_node.guid) { trace!( self.driver, @@ -1588,47 +1601,52 @@ impl<'t, D: Driver, A: AbortSignal> Merger<'t, D, A> { // Note that we don't need to check if the remote node is deleted // locally, because it wouldn't have local content entries if it // were. - if let Some(remote_child_content) = remote_child_node.content() { - let dupe_key = match remote_child_content { - Content::Bookmark { .. } | Content::Folder { .. } => { - DupeKey::WithoutPosition(remote_child_content) - } - Content::Separator => { - DupeKey::WithPosition(remote_child_content, remote_position) - } - }; - if let Some(local_nodes_for_key) = dupe_key_to_local_nodes.get_mut(&dupe_key) { - if let Some(local_child_node) = local_nodes_for_key.pop_front() { - trace!( - self.driver, - "Deduping local child {} to remote child {}", - local_child_node, - remote_child_node - ); - local_to_remote.insert(local_child_node.guid.clone(), remote_child_node); - remote_to_local.insert(remote_child_node.guid.clone(), local_child_node); + match remote_child_node.content() { + Some(remote_child_content) => { + let dupe_key = match remote_child_content { + Content::Bookmark { .. } | Content::Folder { .. } => { + DupeKey::WithoutPosition(remote_child_content) + } + Content::Separator => { + DupeKey::WithPosition(remote_child_content, remote_position) + } + }; + if let Some(local_nodes_for_key) = dupe_key_to_local_nodes.get_mut(&dupe_key) { + if let Some(local_child_node) = local_nodes_for_key.pop_front() { + trace!( + self.driver, + "Deduping local child {} to remote child {}", + local_child_node, + remote_child_node + ); + local_to_remote + .insert(local_child_node.guid.clone(), remote_child_node); + remote_to_local + .insert(remote_child_node.guid.clone(), local_child_node); + } else { + trace!( + self.driver, + "Not deduping remote child {}; no remaining local content matches", + remote_child_node + ); + continue; + } } else { trace!( self.driver, - "Not deduping remote child {}; no remaining local content matches", + "Not deduping remote child {}; no local content matches", remote_child_node ); continue; } - } else { + } + None => { trace!( self.driver, - "Not deduping remote child {}; no local content matches", + "Not deduping remote child {} without content info", remote_child_node ); - continue; } - } else { - trace!( - self.driver, - "Not deduping remote child {}; already merged", - remote_child_node - ); } } diff --git a/third_party/rust/dogear/src/tests.rs b/third_party/rust/dogear/src/tests.rs index 55dceeb197b2..86189e452c0f 100644 --- a/third_party/rust/dogear/src/tests.rs +++ b/third_party/rust/dogear/src/tests.rs @@ -60,7 +60,7 @@ impl TryFrom for Builder { _ => return Err(err), } } - b.mutate(&guid).by_structure(&parent_guid)?; + b.parent_for(&guid).by_structure(&parent_guid)?; for child in node.children { inflate(b, &guid, child)?; } @@ -1323,6 +1323,82 @@ fn newer_move_to_deleted() { assert_eq!(merged_root.counts(), &expected_telem); } +#[test] +fn deduping_multiple_candidates() { + before_each(); + + let mut local_tree_builder = Builder::try_from(nodes!({ + ("menu________", Folder[needs_merge = true, age = 5], { + ("folderAAAAA1", Folder[needs_merge = true, age = 5]), + ("folderAAAAA2", Folder[needs_merge = true, age = 5]) + }), + ("toolbar_____", Folder[needs_merge = true], { + ("folderBBBBB1", Folder[needs_merge = true]) + }) + })) + .unwrap(); + local_tree_builder + .mutate(&"folderAAAAA1".into()) + .content(Content::Folder { title: "A".into() }); + local_tree_builder + .mutate(&"folderAAAAA2".into()) + .content(Content::Folder { title: "A".into() }); + local_tree_builder + .mutate(&"folderBBBBB1".into()) + .content(Content::Folder { title: "B".into() }); + let local_tree = local_tree_builder.into_tree().unwrap(); + + let mut remote_tree_builder = Builder::try_from(nodes!({ + ("menu________", Folder[needs_merge = true], { + ("folderAAAAA1", Folder[needs_merge = true]) + }), + ("toolbar_____", Folder[needs_merge = true, age = 5], { + ("folderBBBBB1", Folder[needs_merge = true, age = 5]), + ("folderBBBBB2", Folder[needs_merge = true, age = 5]) + }) + })) + .unwrap(); + remote_tree_builder + .mutate(&"folderAAAAA1".into()) + .content(Content::Folder { title: "A".into() }); + remote_tree_builder + .mutate(&"folderBBBBB1".into()) + .content(Content::Folder { title: "B".into() }); + remote_tree_builder + .mutate(&"folderBBBBB2".into()) + .content(Content::Folder { title: "B".into() }); + let remote_tree = remote_tree_builder.into_tree().unwrap(); + + let merger = Merger::new(&local_tree, &remote_tree); + let merged_root = merger.merge().unwrap(); + + let expected_tree = merged_nodes!({ + ("menu________", LocalWithNewLocalStructure, { + ("folderAAAAA1", Remote), + ("folderAAAAA2", Local) + }), + ("toolbar_____", LocalWithNewLocalStructure, { + ("folderBBBBB1", Local), + ("folderBBBBB2", Remote) + }) + }); + let expected_telem = StructureCounts { + remote_revives: 0, + local_deletes: 0, + local_revives: 0, + remote_deletes: 0, + dupes: 0, + merged_nodes: 6, + merged_deletions: 0, + }; + + assert_eq!(&expected_tree, merged_root.node()); + + assert_eq!(merged_root.deletions().count(), 0); + + assert_eq!(merged_root.counts(), &expected_telem); +} + #[test] fn deduping_local_newer() { before_each(); diff --git a/third_party/rust/dogear/src/tree.rs b/third_party/rust/dogear/src/tree.rs index b0df51593964..cc8513571364 100644 --- a/third_party/rust/dogear/src/tree.rs +++ b/third_party/rust/dogear/src/tree.rs @@ -499,52 +499,10 @@ impl<'b> ItemBuilder<'b> { b.by_parent_guid(parent_guid) } - /// Records a `parent_guid` from a valid tree structure. This is for - /// callers who already know their structure is consistent, like - /// `Store::fetch_local_tree()` on Desktop, and - /// `std::convert::TryInto` in the tests. - /// - /// Both the item and `parent_guid` must exist, and the `parent_guid` must - /// refer to a folder. - /// - /// `by_structure(parent_guid)` is logically the same as: - /// - /// ```no_run - /// # use dogear::{Item, Kind, Result, ROOT_GUID, Tree}; - /// # fn main() -> Result<()> { - /// # let mut builder = Tree::with_root(Item::new(ROOT_GUID, Kind::Folder)); - /// # let child_guid = "bookmarkAAAA".into(); - /// # let parent_guid = "folderAAAAAA".into(); - /// builder.parent_for(&child_guid) - /// .by_children(&parent_guid)? - /// .parent_for(&child_guid) - /// .by_parent_guid(parent_guid)?; - /// # Ok(()) - /// # } - /// ``` - /// - /// ...But more convenient. It's also more efficient, because it avoids - /// multiple lookups for the item and parent, as well as an extra heap - /// allocation to store the parents. + #[inline] pub fn by_structure(self, parent_guid: &Guid) -> Result<&'b mut Builder> { - let parent_index = match self.0.entry_index_by_guid.get(parent_guid) { - Some(&parent_index) if self.0.entries[parent_index].item.is_folder() => parent_index, - _ => { - return Err(ErrorKind::InvalidParent( - self.0.entries[self.1].item.guid.clone(), - parent_guid.clone(), - ) - .into()); - } - }; - self.0.entries[self.1].parents_by(&[ - BuilderParentBy::Children(parent_index), - BuilderParentBy::KnownItem(parent_index), - ])?; - self.0.entries[parent_index] - .children - .push(BuilderEntryChild::Exists(self.1)); - Ok(self.0) + let b = ParentBuilder(self.0, BuilderEntryChild::Exists(self.1)); + b.by_structure(parent_guid) } } @@ -593,6 +551,56 @@ impl<'b> ParentBuilder<'b> { } Ok(self.0) } + + /// Records a `parent_guid` from a valid tree structure. This is for + /// callers who already know their structure is consistent, like + /// `Store::fetch_local_tree()` on Desktop, and + /// `std::convert::TryInto` in the tests. + /// + /// Both the item and `parent_guid` must exist, and the `parent_guid` must + /// refer to a folder. + /// + /// `by_structure(parent_guid)` is logically the same as: + /// + /// ```no_run + /// # use dogear::{Item, Kind, Result, ROOT_GUID, Tree}; + /// # fn main() -> Result<()> { + /// # let mut builder = Tree::with_root(Item::new(ROOT_GUID, Kind::Folder)); + /// # let child_guid = "bookmarkAAAA".into(); + /// # let parent_guid = "folderAAAAAA".into(); + /// builder.parent_for(&child_guid) + /// .by_children(&parent_guid)? + /// .parent_for(&child_guid) + /// .by_parent_guid(parent_guid)?; + /// # Ok(()) + /// # } + /// ``` + /// + /// ...But more convenient. It's also more efficient, because it avoids + /// multiple lookups for the item and parent, as well as an extra heap + /// allocation to store the parents. + pub fn by_structure(self, parent_guid: &Guid) -> Result<&'b mut Builder> { + let parent_index = match self.0.entry_index_by_guid.get(parent_guid) { + Some(&parent_index) if self.0.entries[parent_index].item.is_folder() => parent_index, + _ => { + let child_guid = match &self.1 { + BuilderEntryChild::Exists(index) => &self.0.entries[*index].item.guid, + BuilderEntryChild::Missing(guid) => guid, + }; + return Err( + ErrorKind::InvalidParent(child_guid.clone(), parent_guid.clone()).into(), + ); + } + }; + if let BuilderEntryChild::Exists(child_index) = &self.1 { + self.0.entries[*child_index].parents_by(&[ + BuilderParentBy::Children(parent_index), + BuilderParentBy::KnownItem(parent_index), + ])?; + } + self.0.entries[parent_index].children.push(self.1); + Ok(self.0) + } } /// An entry wraps a tree item with references to its parents and children, diff --git a/toolkit/components/places/SyncedBookmarksMirror.jsm b/toolkit/components/places/SyncedBookmarksMirror.jsm index 0266510019a9..b6baf0eb9710 100644 --- a/toolkit/components/places/SyncedBookmarksMirror.jsm +++ b/toolkit/components/places/SyncedBookmarksMirror.jsm @@ -84,7 +84,7 @@ const SQLITE_MAX_VARIABLE_NUMBER = 999; // The current mirror database schema version. Bump for migrations, then add // migration code to `migrateMirrorSchema`. -const MIRROR_SCHEMA_VERSION = 6; +const MIRROR_SCHEMA_VERSION = 7; const DEFAULT_MAX_FRECENCIES_TO_RECALCULATE = 400; @@ -1481,6 +1481,10 @@ async function migrateMirrorSchema(db, currentSchemaVersion) { await db.execute(`CREATE INDEX mirror.itemKeywords ON items(keyword) WHERE keyword NOT NULL`); } + if (currentSchemaVersion < 7) { + await db.execute(`CREATE INDEX mirror.structurePositions ON + structure(parentGuid, position)`); + } } /** @@ -1548,6 +1552,10 @@ async function initializeMirrorDatabase(db) { tag TEXT NOT NULL )`); + await db.execute( + `CREATE INDEX mirror.structurePositions ON structure(parentGuid, position)` + ); + await db.execute(`CREATE INDEX mirror.urlHashes ON urls(hash)`); await db.execute(`CREATE INDEX mirror.itemURLs ON items(urlId)`); diff --git a/toolkit/components/places/bookmark_sync/Cargo.toml b/toolkit/components/places/bookmark_sync/Cargo.toml index f63a5a126054..11ccec906a6e 100644 --- a/toolkit/components/places/bookmark_sync/Cargo.toml +++ b/toolkit/components/places/bookmark_sync/Cargo.toml @@ -6,7 +6,7 @@ edition = "2018" [dependencies] atomic_refcell = "0.1" -dogear = "0.3.1" +dogear = "0.3.3" libc = "0.2" log = "0.4" cstr = "0.1" diff --git a/toolkit/components/places/bookmark_sync/src/store.rs b/toolkit/components/places/bookmark_sync/src/store.rs index 35a91956bb94..048399bfd974 100644 --- a/toolkit/components/places/bookmark_sync/src/store.rs +++ b/toolkit/components/places/bookmark_sync/src/store.rs @@ -2,7 +2,7 @@ * 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/. */ -use std::{convert::TryFrom, fmt, time::SystemTime}; +use std::{collections::HashMap, convert::TryFrom, fmt, time::SystemTime}; use dogear::{ debug, AbortSignal, CompletionOps, Content, Deletion, Guid, Item, Kind, MergedRoot, Tree, @@ -259,59 +259,69 @@ impl<'s> dogear::Store for Store<'s> { /// Builds a fully rooted, consistent tree from the items and tombstones in /// Places. fn fetch_local_tree(&self) -> Result { - let mut items_statement = self.db.prepare(format!( - "WITH RECURSIVE - localItems(id, guid, parentId, parentGuid, position, type, title, - parentTitle, placeId, dateAdded, lastModified, - syncChangeCounter, syncStatus, level) - AS ( - SELECT b.id, b.guid, 0, NULL, b.position, b.type, b.title, - NULL, b.fk, b.dateAdded, b.lastModified, - b.syncChangeCounter, b.syncStatus, 0 - FROM moz_bookmarks b - WHERE b.guid = 'root________' - UNION ALL - SELECT b.id, b.guid, s.id, s.guid, b.position, b.type, b.title, - s.title, b.fk, b.dateAdded, b.lastModified, - b.syncChangeCounter, b.syncStatus, s.level + 1 - FROM moz_bookmarks b - JOIN localItems s ON s.id = b.parent - ) - SELECT s.guid, s.parentGuid, s.type, s.syncChangeCounter, - s.syncStatus, s.lastModified / 1000 AS localModified, - IFNULL(s.title, '') AS title, s.position, - (SELECT h.url FROM moz_places h - WHERE h.id = s.placeId) AS url, - EXISTS(SELECT 1 FROM moz_items_annos a - JOIN moz_anno_attributes n ON n.id = a.anno_attribute_id - WHERE a.item_id = s.id AND - n.name = '{}') AS isLivemark - FROM localItems s - ORDER BY s.level, s.parentId, s.position", - LMANNO_FEEDURI, + let mut root_statement = self.db.prepare(format!( + "SELECT guid, type, syncChangeCounter, syncStatus, + lastModified / 1000 AS localModified, + NULL AS url, 0 AS isLivemark + FROM moz_bookmarks + WHERE guid = '{}'", + dogear::ROOT_GUID ))?; - let mut builder = match items_statement.step()? { - // The first row is always the root. + let mut builder = match root_statement.step()? { Some(step) => { let (item, _) = self.local_row_to_item(&step)?; Tree::with_root(item) } None => return Err(Error::InvalidLocalRoots), }; + + // Add items and contents to the builder, keeping track of their + // structure in a separate map. We can't call `p.by_structure(...)` + // after adding the item, because this query might return rows for + // children before their parents. This approach also lets us scan + // `moz_bookmarks` once, using the index on `(b.parent, b.position)` + // to avoid a temp B-tree for the `ORDER BY`. + let mut child_guids_by_parent_guid: HashMap> = HashMap::new(); + let mut items_statement = self.db.prepare(format!( + "SELECT b.guid, p.guid AS parentGuid, b.type, b.syncChangeCounter, + b.syncStatus, b.lastModified / 1000 AS localModified, + IFNULL(b.title, '') AS title, + (SELECT h.url FROM moz_places h WHERE h.id = b.fk) AS url, + EXISTS(SELECT 1 FROM moz_items_annos a + JOIN moz_anno_attributes n ON n.id = a.anno_attribute_id + WHERE a.item_id = b.id AND + n.name = '{}') AS isLivemark + FROM moz_bookmarks b + JOIN moz_bookmarks p ON p.id = b.parent + WHERE b.guid <> '{}' + ORDER BY b.parent, b.position", + LMANNO_FEEDURI, + dogear::ROOT_GUID, + ))?; while let Some(step) = items_statement.step()? { - // All subsequent rows are descendants. self.controller.err_if_aborted()?; + let (item, content) = self.local_row_to_item(&step)?; + let raw_parent_guid: nsString = step.get_by_name("parentGuid")?; let parent_guid = Guid::from_utf16(&*raw_parent_guid)?; + child_guids_by_parent_guid + .entry(parent_guid) + .or_default() + .push(item.guid.clone()); - let (item, content) = self.local_row_to_item(&step)?; let mut p = builder.item(item)?; - if let Some(content) = content { p.content(content); } + } - p.by_structure(&parent_guid)?; + // At this point, we've added entries for all items to the tree, so + // we can add their structure info. + for (parent_guid, child_guids) in &child_guids_by_parent_guid { + for child_guid in child_guids { + self.controller.err_if_aborted()?; + builder.parent_for(child_guid).by_structure(parent_guid)?; + } } let mut deletions_statement = self.db.prepare("SELECT guid FROM moz_bookmarks_deleted")?; diff --git a/toolkit/components/places/tests/sync/test_bookmark_mirror_migration.js b/toolkit/components/places/tests/sync/test_bookmark_mirror_migration.js index 85449682dcab..695567bf981f 100644 --- a/toolkit/components/places/tests/sync/test_bookmark_mirror_migration.js +++ b/toolkit/components/places/tests/sync/test_bookmark_mirror_migration.js @@ -2,9 +2,24 @@ * http://creativecommons.org/publicdomain/zero/1.0/ */ // Keep in sync with `SyncedBookmarksMirror.jsm`. -const CURRENT_MIRROR_SCHEMA_VERSION = 6; +const CURRENT_MIRROR_SCHEMA_VERSION = 7; -// Migrations between 5 and 6 add two indexes. +async function getIndexNames(db, table, schema = "mirror") { + let rows = await db.execute(`PRAGMA ${schema}.index_list(${table})`); + let names = []; + for (let row of rows) { + // Column 4 is `c` if the index was created via `CREATE INDEX`, `u` if + // via `UNIQUE`, and `pk` if via `PRIMARY KEY`. + let wasCreated = row.getResultByIndex(3) == "c"; + if (wasCreated) { + // Column 2 is the name of the index. + names.push(row.getResultByIndex(1)); + } + } + return names.sort(); +} + +// Migrations between 5 and 7 add three indexes. add_task(async function test_migrate_from_5_to_current() { await PlacesTestUtils.markBookmarksAsSynced(); @@ -23,6 +38,20 @@ add_task(async function test_migrate_from_5_to_current() { "Should upgrade mirror schema to current version" ); + let itemsIndexNames = await getIndexNames(buf.db, "items"); + deepEqual( + itemsIndexNames, + ["itemKeywords", "itemURLs"], + "Should add two indexes on items" + ); + + let structureIndexNames = await getIndexNames(buf.db, "structure"); + deepEqual( + structureIndexNames, + ["structurePositions"], + "Should add an index on structure" + ); + let changesToUpload = await buf.apply(); deepEqual(changesToUpload, {}, "Shouldn't flag any items for reupload"); From c50d51f19d25685d11276b5c07d7f8bb5531c606 Mon Sep 17 00:00:00 2001 From: Tomislav Jovanovic Date: Tue, 27 Aug 2019 20:12:22 +0000 Subject: [PATCH 04/41] Bug 1573045 - Use {restrictedSchemes: false} for tabs.query MatchPattern r=mixedpuppy Differential Revision: https://phabricator.services.mozilla.com/D43602 --HG-- extra : moz-landing-system : lando --- .../components/extensions/parent/ext-tabs.js | 8 ++- .../test/browser/browser_ext_tabs_query.js | 66 +++++++------------ .../android/components/extensions/ext-tabs.js | 4 +- 3 files changed, 33 insertions(+), 45 deletions(-) diff --git a/browser/components/extensions/parent/ext-tabs.js b/browser/components/extensions/parent/ext-tabs.js index df8d706c4705..68bfc7bc0a29 100644 --- a/browser/components/extensions/parent/ext-tabs.js +++ b/browser/components/extensions/parent/ext-tabs.js @@ -205,7 +205,9 @@ class TabsUpdateFilterEventManager extends EventManager { let register = (fire, filterProps) => { let filter = { ...filterProps }; if (filter.urls) { - filter.urls = new MatchPatternSet(filter.urls); + filter.urls = new MatchPatternSet(filter.urls, { + restrictSchemes: false, + }); } let needsModified = true; if (filter.properties) { @@ -944,7 +946,9 @@ this.tabs = class extends ExtensionAPI { queryInfo = Object.assign({}, queryInfo); if (queryInfo.url !== null) { - queryInfo.url = new MatchPatternSet([].concat(queryInfo.url)); + queryInfo.url = new MatchPatternSet([].concat(queryInfo.url), { + restrictSchemes: false, + }); } if (queryInfo.title !== null) { queryInfo.title = new MatchGlob(queryInfo.title); diff --git a/browser/components/extensions/test/browser/browser_ext_tabs_query.js b/browser/components/extensions/test/browser/browser_ext_tabs_query.js index cac9a3eef98e..ea724b33723b 100644 --- a/browser/components/extensions/test/browser/browser_ext_tabs_query.js +++ b/browser/components/extensions/test/browser/browser_ext_tabs_query.js @@ -21,56 +21,38 @@ add_task(async function() { permissions: ["tabs"], }, - background: function() { - browser.tabs.query( - { - lastFocusedWindow: true, - }, - function(tabs) { - browser.test.assertEq(tabs.length, 3, "should have three tabs"); + async background() { + let tabs = await browser.tabs.query({ lastFocusedWindow: true }); + browser.test.assertEq(tabs.length, 3, "should have three tabs"); - tabs.sort((tab1, tab2) => tab1.index - tab2.index); + tabs.sort((tab1, tab2) => tab1.index - tab2.index); - browser.test.assertEq(tabs[0].url, "about:blank", "first tab blank"); - tabs.shift(); + browser.test.assertEq(tabs[0].url, "about:blank", "first tab blank"); + tabs.shift(); - browser.test.assertTrue(tabs[0].active, "tab 0 active"); - browser.test.assertFalse(tabs[1].active, "tab 1 inactive"); + browser.test.assertTrue(tabs[0].active, "tab 0 active"); + browser.test.assertFalse(tabs[1].active, "tab 1 inactive"); - browser.test.assertFalse(tabs[0].pinned, "tab 0 unpinned"); - browser.test.assertFalse(tabs[1].pinned, "tab 1 unpinned"); + browser.test.assertFalse(tabs[0].pinned, "tab 0 unpinned"); + browser.test.assertFalse(tabs[1].pinned, "tab 1 unpinned"); - browser.test.assertEq( - tabs[0].url, - "about:robots", - "tab 0 url correct" - ); - browser.test.assertEq( - tabs[1].url, - "about:config", - "tab 1 url correct" - ); + browser.test.assertEq(tabs[0].url, "about:robots", "tab 0 url correct"); + browser.test.assertEq(tabs[1].url, "about:config", "tab 1 url correct"); - browser.test.assertEq( - tabs[0].status, - "complete", - "tab 0 status correct" - ); - browser.test.assertEq( - tabs[1].status, - "complete", - "tab 1 status correct" - ); + browser.test.assertEq(tabs[0].status, "complete", "tab 0 status correct"); + browser.test.assertEq(tabs[1].status, "complete", "tab 1 status correct"); - browser.test.assertEq( - tabs[0].title, - "Gort! Klaatu barada nikto!", - "tab 0 title correct" - ); - - browser.test.notifyPass("tabs.query"); - } + browser.test.assertEq( + tabs[0].title, + "Gort! Klaatu barada nikto!", + "tab 0 title correct" ); + + tabs = await browser.tabs.query({ url: "about:blank" }); + browser.test.assertEq(tabs.length, 1, "about:blank query finds one tab"); + browser.test.assertEq(tabs[0].url, "about:blank", "with the correct url"); + + browser.test.notifyPass("tabs.query"); }, }); diff --git a/mobile/android/components/extensions/ext-tabs.js b/mobile/android/components/extensions/ext-tabs.js index 924d17d3f0cb..74c1d0abadbf 100644 --- a/mobile/android/components/extensions/ext-tabs.js +++ b/mobile/android/components/extensions/ext-tabs.js @@ -456,7 +456,9 @@ this.tabs = class extends ExtensionAPI { queryInfo = Object.assign({}, queryInfo); if (queryInfo.url !== null) { - queryInfo.url = new MatchPatternSet([].concat(queryInfo.url)); + queryInfo.url = new MatchPatternSet([].concat(queryInfo.url), { + restrictSchemes: false, + }); } if (queryInfo.title !== null) { queryInfo.title = new MatchGlob(queryInfo.title); From 4b6fb0cabcd632a08c0a314fefab7c73e7386e1d Mon Sep 17 00:00:00 2001 From: Miko Mynttinen Date: Tue, 27 Aug 2019 20:00:26 +0000 Subject: [PATCH 05/41] Bug 1576985 - Use RefPtr for hit test info AGR and ASR r=mattwoodrow Differential Revision: https://phabricator.services.mozilla.com/D43661 --HG-- extra : moz-landing-system : lando --- layout/painting/FrameLayerBuilder.cpp | 3 +-- layout/painting/nsDisplayList.h | 9 ++++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/layout/painting/FrameLayerBuilder.cpp b/layout/painting/FrameLayerBuilder.cpp index a9f392a4bfcc..31ac1b4bd5e3 100644 --- a/layout/painting/FrameLayerBuilder.cpp +++ b/layout/painting/FrameLayerBuilder.cpp @@ -4012,9 +4012,7 @@ void PaintedLayerData::AccumulateHitTestItem(ContainerState* aState, nsDisplayItem* aItem, const DisplayItemClip& aClip, TransformClipNode* aTransform) { - MOZ_ASSERT(aItem->HasHitTestInfo()); auto* item = static_cast(aItem); - const HitTestInfo& info = item->GetHitTestInfo(); nsRect area = info.mArea; @@ -4556,6 +4554,7 @@ void ContainerState::ProcessDisplayItems(nsDisplayList* aList) { nsRect itemContent; if (marker == DisplayItemEntryType::HitTestInfo) { + MOZ_ASSERT(item->IsHitTestItem()); const auto& hitTestInfo = static_cast(item)->GetHitTestInfo(); diff --git a/layout/painting/nsDisplayList.h b/layout/painting/nsDisplayList.h index 2f8ad94ab722..accd4f1257a4 100644 --- a/layout/painting/nsDisplayList.h +++ b/layout/painting/nsDisplayList.h @@ -3874,8 +3874,8 @@ struct HitTestInfo { nsRect mArea; mozilla::gfx::CompositorHitTestInfo mFlags; - AnimatedGeometryRoot* mAGR; - const mozilla::ActiveScrolledRoot* mASR; + RefPtr mAGR; + RefPtr mASR; RefPtr mClipChain; const mozilla::DisplayItemClip* mClip; }; @@ -3893,7 +3893,10 @@ class nsDisplayHitTestInfoItem : public nsPaintedDisplayItem { const nsDisplayHitTestInfoItem& aOther) : nsPaintedDisplayItem(aBuilder, aOther) {} - const HitTestInfo& GetHitTestInfo() const { return *mHitTestInfo; } + const HitTestInfo& GetHitTestInfo() const { + MOZ_ASSERT(HasHitTestInfo()); + return *mHitTestInfo; + } void SetActiveScrolledRoot( const ActiveScrolledRoot* aActiveScrolledRoot) override { From 6626259eb9a784f50a2c9c5f5f076e0da789d185 Mon Sep 17 00:00:00 2001 From: Nihanth Subramanya Date: Tue, 27 Aug 2019 17:17:48 +0000 Subject: [PATCH 06/41] Bug 1576929 - [PanelMultiView] Ensure multi-line subview headers have enough padding. r=pbz Differential Revision: https://phabricator.services.mozilla.com/D43620 --HG-- extra : moz-landing-system : lando --- browser/themes/shared/customizableui/panelUI.inc.css | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/browser/themes/shared/customizableui/panelUI.inc.css b/browser/themes/shared/customizableui/panelUI.inc.css index 4fc223cf707c..31782dfb8be3 100644 --- a/browser/themes/shared/customizableui/panelUI.inc.css +++ b/browser/themes/shared/customizableui/panelUI.inc.css @@ -1699,14 +1699,13 @@ toolbarpaletteitem[place="menu-panel"] > .subviewbutton-nav::after { border-bottom: 1px solid var(--panel-separator-color); display: flex; flex: 1 auto; - height: 40px; /* fixed item height to prevent flex sizing; height + 2*4px padding */ padding: 4px; } .panel-header > label { flex: auto; font-weight: 600; - margin: 0; + margin: 4px 0; text-align: center; } From 40d4617494a17871fd083b8c2c1366fcb6e9780b Mon Sep 17 00:00:00 2001 From: longsonr Date: Tue, 27 Aug 2019 20:33:46 +0000 Subject: [PATCH 07/41] Bug 1570799 - pass the original element into nsXMLContentSerializer::CheckElementEnd so that we can determine whether it has children properly r=hsivonen Differential Revision: https://phabricator.services.mozilla.com/D43450 --HG-- extra : moz-landing-system : lando --- dom/base/nsDocumentEncoder.cpp | 33 ++++++++++++++++----------- dom/base/nsHTMLContentSerializer.cpp | 4 +++- dom/base/nsHTMLContentSerializer.h | 1 + dom/base/nsIContentSerializer.h | 1 + dom/base/nsPlainTextSerializer.cpp | 4 +++- dom/base/nsPlainTextSerializer.h | 1 + dom/base/nsXHTMLContentSerializer.cpp | 4 +++- dom/base/nsXHTMLContentSerializer.h | 1 + dom/base/nsXMLContentSerializer.cpp | 14 ++++++------ dom/base/nsXMLContentSerializer.h | 2 ++ 10 files changed, 42 insertions(+), 23 deletions(-) diff --git a/dom/base/nsDocumentEncoder.cpp b/dom/base/nsDocumentEncoder.cpp index 1b3675ebb3e5..f7d0f759e68b 100644 --- a/dom/base/nsDocumentEncoder.cpp +++ b/dom/base/nsDocumentEncoder.cpp @@ -239,7 +239,8 @@ class nsDocumentEncoder : public nsIDocumentEncoder { nsresult SerializeToStringRecursive(nsINode* aNode, nsAString& aStr, bool aDontSerializeRoot, uint32_t aMaxLength = 0); - nsresult SerializeNodeEnd(nsINode& aNode, nsAString& aStr); + nsresult SerializeNodeEnd(nsINode& aOriginalNode, nsAString& aStr, + nsINode* aFixupNode = nullptr); // This serializes the content of aNode. nsresult SerializeToStringIterative(nsINode* aNode, nsAString& aStr); nsresult SerializeRangeToString(nsRange* aRange, nsAString& aOutputString); @@ -660,9 +661,8 @@ nsresult nsDocumentEncoder::SerializeNodeStart(nsINode& aOriginalNode, nsLayoutUtils::IsInvisibleBreak(node)) { return rv; } - Element* originalElement = aOriginalNode.AsElement(); - rv = mSerializer->AppendElementStart(node->AsElement(), originalElement, - aStr); + rv = mSerializer->AppendElementStart(node->AsElement(), + aOriginalNode.AsElement(), aStr); return rv; } @@ -697,26 +697,33 @@ nsresult nsDocumentEncoder::SerializeNodeStart(nsINode& aOriginalNode, return rv; } -nsresult nsDocumentEncoder::SerializeNodeEnd(nsINode& aNode, nsAString& aStr) { +nsresult nsDocumentEncoder::SerializeNodeEnd(nsINode& aOriginalNode, + nsAString& aStr, + nsINode* aFixupNode) { if (mNeedsPreformatScanning) { - if (aNode.IsElement()) { - mSerializer->ForgetElementForPreformat(aNode.AsElement()); - } else if (aNode.IsText()) { - const nsCOMPtr parent = aNode.GetParent(); + if (aOriginalNode.IsElement()) { + mSerializer->ForgetElementForPreformat(aOriginalNode.AsElement()); + } else if (aOriginalNode.IsText()) { + const nsCOMPtr parent = aOriginalNode.GetParent(); if (parent && parent->IsElement()) { mSerializer->ForgetElementForPreformat(parent->AsElement()); } } } - if (IsInvisibleNodeAndShouldBeSkipped(aNode)) { + if (IsInvisibleNodeAndShouldBeSkipped(aOriginalNode)) { return NS_OK; } nsresult rv = NS_OK; - if (aNode.IsElement()) { - rv = mSerializer->AppendElementEnd(aNode.AsElement(), aStr); + FixupNodeDeterminer fixupNodeDeterminer{mNodeFixup, aFixupNode, + aOriginalNode}; + nsINode* node = &fixupNodeDeterminer.GetFixupNodeFallBackToOriginalNode(); + + if (node->IsElement()) { + rv = mSerializer->AppendElementEnd(node->AsElement(), + aOriginalNode.AsElement(), aStr); } return rv; @@ -773,7 +780,7 @@ nsresult nsDocumentEncoder::SerializeToStringRecursive(nsINode* aNode, } if (!aDontSerializeRoot) { - rv = SerializeNodeEnd(*maybeFixedNode, aStr); + rv = SerializeNodeEnd(*aNode, aStr, maybeFixedNode); NS_ENSURE_SUCCESS(rv, rv); } diff --git a/dom/base/nsHTMLContentSerializer.cpp b/dom/base/nsHTMLContentSerializer.cpp index 46e45b63e159..4a9f43ad1332 100644 --- a/dom/base/nsHTMLContentSerializer.cpp +++ b/dom/base/nsHTMLContentSerializer.cpp @@ -256,7 +256,9 @@ nsHTMLContentSerializer::AppendElementStart(Element* aElement, } NS_IMETHODIMP -nsHTMLContentSerializer::AppendElementEnd(Element* aElement, nsAString& aStr) { +nsHTMLContentSerializer::AppendElementEnd(Element* aElement, + Element* aOriginalElement, + nsAString& aStr) { NS_ENSURE_ARG(aElement); nsAtom* name = aElement->NodeInfo()->NameAtom(); diff --git a/dom/base/nsHTMLContentSerializer.h b/dom/base/nsHTMLContentSerializer.h index f0ea07f46b57..756435058d0c 100644 --- a/dom/base/nsHTMLContentSerializer.h +++ b/dom/base/nsHTMLContentSerializer.h @@ -29,6 +29,7 @@ class nsHTMLContentSerializer final : public nsXHTMLContentSerializer { nsAString& aStr) override; NS_IMETHOD AppendElementEnd(mozilla::dom::Element* aElement, + mozilla::dom::Element* aOriginalElement, nsAString& aStr) override; NS_IMETHOD AppendDocumentStart(mozilla::dom::Document* aDocument, diff --git a/dom/base/nsIContentSerializer.h b/dom/base/nsIContentSerializer.h index 5c889e423a3b..75ec274e94d9 100644 --- a/dom/base/nsIContentSerializer.h +++ b/dom/base/nsIContentSerializer.h @@ -60,6 +60,7 @@ class nsIContentSerializer : public nsISupports { nsAString& aStr) = 0; NS_IMETHOD AppendElementEnd(mozilla::dom::Element* aElement, + mozilla::dom::Element* aOriginalElement, nsAString& aStr) = 0; NS_IMETHOD Flush(nsAString& aStr) = 0; diff --git a/dom/base/nsPlainTextSerializer.cpp b/dom/base/nsPlainTextSerializer.cpp index 7b67ba2524b0..afb34ca55e27 100644 --- a/dom/base/nsPlainTextSerializer.cpp +++ b/dom/base/nsPlainTextSerializer.cpp @@ -419,7 +419,9 @@ nsPlainTextSerializer::AppendElementStart(Element* aElement, } NS_IMETHODIMP -nsPlainTextSerializer::AppendElementEnd(Element* aElement, nsAString& aStr) { +nsPlainTextSerializer::AppendElementEnd(Element* aElement, + Element* aOriginalElement, + nsAString& aStr) { NS_ENSURE_ARG(aElement); mElement = aElement; diff --git a/dom/base/nsPlainTextSerializer.h b/dom/base/nsPlainTextSerializer.h index 1baf617126b4..ad560b29a2f6 100644 --- a/dom/base/nsPlainTextSerializer.h +++ b/dom/base/nsPlainTextSerializer.h @@ -69,6 +69,7 @@ class nsPlainTextSerializer final : public nsIContentSerializer { mozilla::dom::Element* aOriginalElement, nsAString& aStr) override; NS_IMETHOD AppendElementEnd(mozilla::dom::Element* aElement, + mozilla::dom::Element* aOriginalElement, nsAString& aStr) override; NS_IMETHOD Flush(nsAString& aStr) override; diff --git a/dom/base/nsXHTMLContentSerializer.cpp b/dom/base/nsXHTMLContentSerializer.cpp index a2161560812b..50a391a47912 100644 --- a/dom/base/nsXHTMLContentSerializer.cpp +++ b/dom/base/nsXHTMLContentSerializer.cpp @@ -410,6 +410,7 @@ bool nsXHTMLContentSerializer::CheckElementStart(Element* aElement, } bool nsXHTMLContentSerializer::CheckElementEnd(Element* aElement, + Element* aOriginalElement, bool& aForceFormat, nsAString& aStr) { NS_ASSERTION(!mIsHTMLSerializer, @@ -428,7 +429,8 @@ bool nsXHTMLContentSerializer::CheckElementEnd(Element* aElement, } bool dummyFormat; - return nsXMLContentSerializer::CheckElementEnd(aElement, dummyFormat, aStr); + return nsXMLContentSerializer::CheckElementEnd(aElement, aOriginalElement, + dummyFormat, aStr); } bool nsXHTMLContentSerializer::AppendAndTranslateEntities( diff --git a/dom/base/nsXHTMLContentSerializer.h b/dom/base/nsXHTMLContentSerializer.h index 38bde6f77129..c6f43ddc5f32 100644 --- a/dom/base/nsXHTMLContentSerializer.h +++ b/dom/base/nsXHTMLContentSerializer.h @@ -52,6 +52,7 @@ class nsXHTMLContentSerializer : public nsXMLContentSerializer { nsAString& aStr) override; virtual bool CheckElementEnd(mozilla::dom::Element* aContent, + mozilla::dom::Element* aOriginalElement, bool& aForceFormat, nsAString& aStr) override; virtual void AfterElementEnd(nsIContent* aContent, nsAString& aStr) override; diff --git a/dom/base/nsXMLContentSerializer.cpp b/dom/base/nsXMLContentSerializer.cpp index 48d12a784d24..42746703aea3 100644 --- a/dom/base/nsXMLContentSerializer.cpp +++ b/dom/base/nsXMLContentSerializer.cpp @@ -955,13 +955,16 @@ bool nsXMLContentSerializer::AppendEndOfElementStart(Element* aElement, } NS_IMETHODIMP -nsXMLContentSerializer::AppendElementEnd(Element* aElement, nsAString& aStr) { +nsXMLContentSerializer::AppendElementEnd(Element* aElement, + Element* aOriginalElement, + nsAString& aStr) { NS_ENSURE_ARG(aElement); nsIContent* content = aElement; bool forceFormat = false, outputElementEnd; - outputElementEnd = CheckElementEnd(aElement, forceFormat, aStr); + outputElementEnd = + CheckElementEnd(aElement, aOriginalElement, forceFormat, aStr); nsAtom* name = content->NodeInfo()->NameAtom(); @@ -1083,15 +1086,12 @@ bool nsXMLContentSerializer::CheckElementStart(Element*, bool& aForceFormat, } bool nsXMLContentSerializer::CheckElementEnd(Element* aElement, + Element* aOriginalElement, bool& aForceFormat, nsAString& aStr) { // We don't output a separate end tag for empty element aForceFormat = false; - - // XXXbz this is a bit messed up, but by now we don't have our fixed-up - // version of aElement anymore. Let's hope fixup never changes the localName - // or namespace... - return ElementNeedsSeparateEndTag(aElement, aElement); + return ElementNeedsSeparateEndTag(aElement, aOriginalElement); } bool nsXMLContentSerializer::AppendToString(const char16_t aChar, diff --git a/dom/base/nsXMLContentSerializer.h b/dom/base/nsXMLContentSerializer.h index 31b04326927f..0e7199097332 100644 --- a/dom/base/nsXMLContentSerializer.h +++ b/dom/base/nsXMLContentSerializer.h @@ -63,6 +63,7 @@ class nsXMLContentSerializer : public nsIContentSerializer { nsAString& aStr) override; NS_IMETHOD AppendElementEnd(mozilla::dom::Element* aElement, + mozilla::dom::Element* aOriginalElement, nsAString& aStr) override; NS_IMETHOD Flush(nsAString& aStr) override { return NS_OK; } @@ -305,6 +306,7 @@ class nsXMLContentSerializer : public nsIContentSerializer { * @return boolean true if the element can be output */ virtual bool CheckElementEnd(mozilla::dom::Element* aElement, + mozilla::dom::Element* aOriginalElement, bool& aForceFormat, nsAString& aStr); /** From 7e54f29018dd73a998fcd8067301d74f83da04f9 Mon Sep 17 00:00:00 2001 From: Michael Kaply Date: Tue, 27 Aug 2019 18:20:59 +0000 Subject: [PATCH 08/41] Bug 1573967 - Add policy to set the default rememberSignons policy. r=MattN,fluent-reviewers,flod Differential Revision: https://phabricator.services.mozilla.com/D42715 --HG-- extra : moz-landing-system : lando --- browser/components/enterprisepolicies/Policies.jsm | 6 ++++++ .../enterprisepolicies/schemas/policies-schema.json | 4 ++++ .../tests/xpcshell/test_simple_pref_policies.js | 10 ++++++++++ .../en-US/browser/policies/policies-descriptions.ftl | 2 ++ 4 files changed, 22 insertions(+) diff --git a/browser/components/enterprisepolicies/Policies.jsm b/browser/components/enterprisepolicies/Policies.jsm index dd1daaf69639..9892a2242542 100644 --- a/browser/components/enterprisepolicies/Policies.jsm +++ b/browser/components/enterprisepolicies/Policies.jsm @@ -978,6 +978,12 @@ var Policies = { }, }, + OfferToSaveLoginsDefault: { + onBeforeUIStartup(manager, param) { + setDefaultPref("signon.rememberSignons", param); + }, + }, + OverrideFirstRunPage: { onProfileAfterChange(manager, param) { let url = param ? param.href : ""; diff --git a/browser/components/enterprisepolicies/schemas/policies-schema.json b/browser/components/enterprisepolicies/schemas/policies-schema.json index 49b62af8ddee..1575b678294d 100644 --- a/browser/components/enterprisepolicies/schemas/policies-schema.json +++ b/browser/components/enterprisepolicies/schemas/policies-schema.json @@ -497,6 +497,10 @@ "type": "boolean" }, + "OfferToSaveLoginsDefault": { + "type": "boolean" + }, + "OverrideFirstRunPage": { "type": "URLorEmpty" }, diff --git a/browser/components/enterprisepolicies/tests/xpcshell/test_simple_pref_policies.js b/browser/components/enterprisepolicies/tests/xpcshell/test_simple_pref_policies.js index f258533de26d..96b4037f0574 100644 --- a/browser/components/enterprisepolicies/tests/xpcshell/test_simple_pref_policies.js +++ b/browser/components/enterprisepolicies/tests/xpcshell/test_simple_pref_policies.js @@ -456,6 +456,16 @@ const POLICIES_TESTS = [ "browser.newtabpage.activity-stream.feeds.section.topstories": false, }, }, + + // POLICY: OfferToSaveLoginsDefault + { + policies: { + OfferToSaveLoginsDefault: false, + }, + unlockedPrefs: { + "signon.rememberSignons": false, + }, + }, ]; add_task(async function test_policy_simple_prefs() { diff --git a/browser/locales/en-US/browser/policies/policies-descriptions.ftl b/browser/locales/en-US/browser/policies/policies-descriptions.ftl index b649c64b328d..9f449cf0a318 100644 --- a/browser/locales/en-US/browser/policies/policies-descriptions.ftl +++ b/browser/locales/en-US/browser/policies/policies-descriptions.ftl @@ -115,6 +115,8 @@ policy-NoDefaultBookmarks = Disable creation of the default bookmarks bundled wi policy-OfferToSaveLogins = Enforce the setting to allow { -brand-short-name } to offer to remember saved logins and passwords. Both true and false values are accepted. +policy-OfferToSaveLoginsDefault = Set the default value for allowing { -brand-short-name } to offer to remember saved logins and passwords. Both true and false values are accepted. + policy-OverrideFirstRunPage = Override the first run page. Set this policy to blank if you want to disable the first run page. policy-OverridePostUpdatePage = Override the post-update “What’s New” page. Set this policy to blank if you want to disable the post-update page. From 37be9d3076388e2af92e203bf77928c8f14bbbfa Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Tue, 27 Aug 2019 16:08:40 +0000 Subject: [PATCH 09/41] Bug 1575198 - Reenable cranelift on central. r=nalexander Bug 1555894 disabled it for gecko builds, but since then bug 1576003 and https://github.com/CraneStation/cranelift/pull/924 landed, which should make its cost to the build less problematic. Differential Revision: https://phabricator.services.mozilla.com/D43581 --HG-- extra : moz-landing-system : lando --- js/moz.configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/moz.configure b/js/moz.configure index 9a1c146a575e..e33e8c8407c5 100644 --- a/js/moz.configure +++ b/js/moz.configure @@ -439,7 +439,7 @@ set_define('JS_BUILD_BINAST', depends_if('--enable-binast')(lambda x: True)) # ============================================================== js_option('--enable-cranelift', - default=milestone.is_nightly & js_standalone, + default=milestone.is_nightly, help='{Enable|Disable} Cranelift code generator for wasm') set_config('ENABLE_WASM_CRANELIFT', depends_if('--enable-cranelift')(lambda x: True)) From fdbeea9b5f02fff2b5ab33274310462260f20b01 Mon Sep 17 00:00:00 2001 From: David Walsh Date: Tue, 27 Aug 2019 20:51:10 +0000 Subject: [PATCH 10/41] Bug 1555333 - Toggle skipPausing when adding or enabling a breakpoint via reducers r=jlast Differential Revision: https://phabricator.services.mozilla.com/D43512 --HG-- extra : moz-landing-system : lando --- .../debugger/src/actions/pause/index.js | 2 +- .../debugger/src/actions/pause/skipPausing.js | 16 +++++++++++++ .../SecondaryPanes/DOMMutationBreakpoints.js | 17 ++++++++++++-- .../client/debugger/src/reducers/pause.js | 14 +++++++++++ .../browser_dbg-breakpoint-skipping.js | 23 +++++++++++++++++++ 5 files changed, 69 insertions(+), 3 deletions(-) diff --git a/devtools/client/debugger/src/actions/pause/index.js b/devtools/client/debugger/src/actions/pause/index.js index a617e6a14c39..ad7bfffc7015 100644 --- a/devtools/client/debugger/src/actions/pause/index.js +++ b/devtools/client/debugger/src/actions/pause/index.js @@ -26,7 +26,7 @@ export { breakOnNext } from "./breakOnNext"; export { mapFrames } from "./mapFrames"; export { pauseOnExceptions } from "./pauseOnExceptions"; export { selectFrame } from "./selectFrame"; -export { toggleSkipPausing } from "./skipPausing"; +export { toggleSkipPausing, setSkipPausing } from "./skipPausing"; export { toggleMapScopes } from "./mapScopes"; export { setExpandedScope } from "./expandScopes"; export { generateInlinePreview } from "./inlinePreview"; diff --git a/devtools/client/debugger/src/actions/pause/skipPausing.js b/devtools/client/debugger/src/actions/pause/skipPausing.js index bccedd573ca4..e5d3a9ef403b 100644 --- a/devtools/client/debugger/src/actions/pause/skipPausing.js +++ b/devtools/client/debugger/src/actions/pause/skipPausing.js @@ -18,3 +18,19 @@ export function toggleSkipPausing() { dispatch({ type: "TOGGLE_SKIP_PAUSING", skipPausing }); }; } + +/** + * @memberof actions/pause + * @static + */ +export function setSkipPausing(skipPausing: boolean) { + return async ({ dispatch, client, getState, sourceMaps }: ThunkArgs) => { + const currentlySkipping = getSkipPausing(getState()); + if (currentlySkipping === skipPausing) { + return; + } + + await client.setSkipPausing(skipPausing); + dispatch({ type: "TOGGLE_SKIP_PAUSING", skipPausing }); + }; +} diff --git a/devtools/client/debugger/src/components/SecondaryPanes/DOMMutationBreakpoints.js b/devtools/client/debugger/src/components/SecondaryPanes/DOMMutationBreakpoints.js index 643ac90dd455..3b5bef23a89f 100644 --- a/devtools/client/debugger/src/components/SecondaryPanes/DOMMutationBreakpoints.js +++ b/devtools/client/debugger/src/components/SecondaryPanes/DOMMutationBreakpoints.js @@ -31,6 +31,7 @@ type Props = { unHighlightDomElement: typeof actions.unHighlightDomElement, deleteBreakpoint: typeof deleteDOMMutationBreakpoint, toggleBreakpoint: typeof toggleDOMMutationBreakpointState, + setSkipPausing: typeof actions.setSkipPausing, }; const localizationTerms = { @@ -40,12 +41,22 @@ const localizationTerms = { }; class DOMMutationBreakpointsContents extends Component { + handleBreakpoint(breakpointId, shouldEnable) { + const { toggleBreakpoint, setSkipPausing } = this.props; + + // The user has enabled a mutation breakpoint so we should no + // longer skip pausing + if (shouldEnable) { + setSkipPausing(false); + } + toggleBreakpoint(breakpointId, shouldEnable); + } + renderItem(breakpoint: DOMMutationBreakpoint) { const { openElementInInspector, highlightDomElement, unHighlightDomElement, - toggleBreakpoint, deleteBreakpoint, } = this.props; const { enabled, id: breakpointId, nodeFront, mutationType } = breakpoint; @@ -55,7 +66,7 @@ class DOMMutationBreakpointsContents extends Component { toggleBreakpoint(breakpointId, !enabled)} + onChange={() => this.handleBreakpoint(breakpointId, !enabled)} />
@@ -123,6 +134,7 @@ class DomMutationBreakpoints extends Component { openElementInInspector={this.props.openElementInInspector} highlightDomElement={this.props.highlightDomElement} unHighlightDomElement={this.props.unHighlightDomElement} + setSkipPausing={this.props.setSkipPausing} /> ); } @@ -136,5 +148,6 @@ export default connect( openElementInInspector: actions.openElementInInspectorCommand, highlightDomElement: actions.highlightDomElement, unHighlightDomElement: actions.unHighlightDomElement, + setSkipPausing: actions.setSkipPausing, } )(DomMutationBreakpoints); diff --git a/devtools/client/debugger/src/reducers/pause.js b/devtools/client/debugger/src/reducers/pause.js index aa54fcbe947c..fe9cfa30e07c 100644 --- a/devtools/client/debugger/src/reducers/pause.js +++ b/devtools/client/debugger/src/reducers/pause.js @@ -343,6 +343,20 @@ function update( }; } + // Disable skipPausing if a breakpoint is enabled or added + case "SET_BREAKPOINT": { + return action.breakpoint.disabled + ? state + : { ...state, skipPausing: false }; + } + + case "UPDATE_EVENT_LISTENERS": + case "REMOVE_BREAKPOINT": + case "SET_XHR_BREAKPOINT": + case "ENABLE_XHR_BREAKPOINT": { + return { ...state, skipPausing: false }; + } + case "TOGGLE_SKIP_PAUSING": { const { skipPausing } = action; prefs.skipPausing = skipPausing; diff --git a/devtools/client/debugger/test/mochitest/browser_dbg-breakpoint-skipping.js b/devtools/client/debugger/test/mochitest/browser_dbg-breakpoint-skipping.js index 233cd4c9e25b..9cca5d916194 100644 --- a/devtools/client/debugger/test/mochitest/browser_dbg-breakpoint-skipping.js +++ b/devtools/client/debugger/test/mochitest/browser_dbg-breakpoint-skipping.js @@ -7,6 +7,19 @@ function skipPausing(dbg) { return waitForState(dbg, state => dbg.selectors.getSkipPausing()); } +function toggleBreakpoint(dbg, index) { + const breakpoints = findAllElements(dbg, "breakpointItems"); + const bp = breakpoints[index]; + const input = bp.querySelector("input"); + input.click(); +} + +async function disableBreakpoint(dbg, index) { + const disabled = waitForDispatch(dbg, "SET_BREAKPOINT"); + toggleBreakpoint(dbg, index); + await disabled; +} + /* * Tests toggling the skip pausing button and * invoking functions without pausing. @@ -25,4 +38,14 @@ add_task(async function() { await reload(dbg, "simple3"); res = await invokeInTab("simple"); is(res, 3, "simple() successfully completed"); + + info("Adding a breakpoint disables skipPausing"); + await addBreakpoint(dbg, "simple3", 3); + await waitForState(dbg, state => !state.skipPausing); + + info("Enabling a breakpoint disables skipPausing"); + await skipPausing(dbg); + await disableBreakpoint(dbg, 0); + await toggleBreakpoint(dbg, 0); + await waitForState(dbg, state => !state.skipPausing); }); From dd6d6fa34ed57e8c875c037a2e022a1c5b34d934 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Tue, 27 Aug 2019 21:07:08 +0000 Subject: [PATCH 11/41] Bug 1575249 - Add a lint to check for source file permissions r=ahal Differential Revision: https://phabricator.services.mozilla.com/D42672 --HG-- extra : moz-landing-system : lando --- tools/lint/file-perm.yml | 24 +++++++++++++++++++++ tools/lint/file-perm/__init__.py | 37 ++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) create mode 100644 tools/lint/file-perm.yml create mode 100644 tools/lint/file-perm/__init__.py diff --git a/tools/lint/file-perm.yml b/tools/lint/file-perm.yml new file mode 100644 index 000000000000..a1408266d76a --- /dev/null +++ b/tools/lint/file-perm.yml @@ -0,0 +1,24 @@ +--- +file-perm: + description: File permission check + include: + - . + extensions: + - .c + - .cc + - .cpp + - .h + - .html + - .js + - .jsm + - .jsx + - .m + - .mm + - .rs + - .xhtml + - .xml + - .xul + support-files: + - 'tools/lint/file-perm/**' + type: external + payload: file-perm:lint diff --git a/tools/lint/file-perm/__init__.py b/tools/lint/file-perm/__init__.py new file mode 100644 index 000000000000..db0238e6d87e --- /dev/null +++ b/tools/lint/file-perm/__init__.py @@ -0,0 +1,37 @@ +# 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/. + +from __future__ import absolute_import + +import os + +from mozlint import result +from mozlint.pathutils import expand_exclusions + +results = [] + + +def lint(paths, config, fix=None, **lintargs): + files = list(expand_exclusions(paths, config, lintargs['root'])) + for f in files: + if os.access(f, os.X_OK): + with open(f, 'r+') as content: + # Some source files have +x permissions + line = content.readline() + if line.startswith("#!"): + # Check if the file doesn't start with a shebang + # if it does, not a warning + continue + + if fix: + # We want to fix it, do it and leave + os.chmod(f, 0o644) + continue + + res = {'path': f, + 'message': "Execution permissions on a source file", + 'level': 'error' + } + results.append(result.from_config(config, **res)) + return results From e41cf7ee420ece258b611b7e543d2634125ec3af Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Tue, 27 Aug 2019 21:10:58 +0000 Subject: [PATCH 12/41] Bug 1575249 - Run the check for incorrect file permissions in the CI r=ahal Differential Revision: https://phabricator.services.mozilla.com/D43362 --HG-- extra : moz-landing-system : lando --- taskcluster/ci/source-test/mozlint.yml | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/taskcluster/ci/source-test/mozlint.yml b/taskcluster/ci/source-test/mozlint.yml index bb43dd3f3018..aebfed63ae89 100644 --- a/taskcluster/ci/source-test/mozlint.yml +++ b/taskcluster/ci/source-test/mozlint.yml @@ -221,3 +221,28 @@ rustfmt: when: files-changed: - '**/*.rs' + + +file-perm: + description: Check for incorrect permissions on source files + platform: lint/opt + treeherder: + symbol: file-perm + run: + mach: lint -l file-perm -f treeherder -f json:/builds/worker/mozlint.json + when: + files-changed: + - '**/*.c' + - '**/*.cc' + - '**/*.cpp' + - '**/*.h' + - '**/*.html' + - '**/*.js' + - '**/*.jsm' + - '**/*.jsx' + - '**/*.m' + - '**/*.mm' + - '**/*.rs' + - '**/*.xhtml' + - '**/*.xml' + - '**/*.xul' From 71f8303be7406d1681f558e4dfc8b8c659d7e680 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Tue, 27 Aug 2019 21:42:18 +0000 Subject: [PATCH 13/41] Bug 1575250 - Add a lint to check for trailing white spaces and windows line return r=ahal Differential Revision: https://phabricator.services.mozilla.com/D42675 --HG-- extra : moz-landing-system : lando --- tools/lint/file-whitespace.yml | 95 ++++++++++++++++++++++++++ tools/lint/file-whitespace/__init__.py | 61 +++++++++++++++++ 2 files changed, 156 insertions(+) create mode 100644 tools/lint/file-whitespace.yml create mode 100644 tools/lint/file-whitespace/__init__.py diff --git a/tools/lint/file-whitespace.yml b/tools/lint/file-whitespace.yml new file mode 100644 index 000000000000..00a701fc84b7 --- /dev/null +++ b/tools/lint/file-whitespace.yml @@ -0,0 +1,95 @@ +--- +file-whitespace: + description: File content sanity check + include: + - . + exclude: + - build/pymake/pymake/command.py + - build/pymake/pymake/data.py + - build/pymake/pymake/functions.py + - build/pymake/pymake/implicit.py + - build/pymake/pymake/parser.py + - build/pymake/pymake/process.py + - build/pymake/pymake/util.py + - build/pymake/tests/runtests.py + - dom/bindings/Codegen.py + - dom/bindings/Configuration.py + - dom/bindings/parser/WebIDL.py + - dom/bindings/parser/tests/test_attributes_on_types.py + - dom/bindings/parser/tests/test_extended_attributes.py + - dom/bindings/parser/tests/test_implements.py + - dom/bindings/parser/tests/test_interface.py + - dom/bindings/parser/tests/test_record.py + - dom/bindings/parser/tests/test_securecontext_extended_attribute.py + - dom/bindings/parser/tests/test_special_methods.py + - dom/bindings/parser/tests/test_toJSON.py + - dom/bindings/parser/tests/test_typedef.py + - dom/encoding/encodings2arrays.py + - dom/media/gtest/AudioGenerator.cpp + - dom/media/gtest/AudioGenerator.h + - dom/security/test/csp/file_websocket_self_wsh.py + - dom/webauthn/winwebauthn/webauthn.h + - dom/websocket/tests/file_websocket_wsh.py + - gfx/vr/nsFxrCommandLineHandler.cpp + - gfx/vr/vrhost/vrhostapi.cpp + - js/src/frontend/BytecodeEmitter.cpp + - js/src/frontend/SharedContext.h + - layout/reftests/fonts/gsubtest/makegsubfonts.py + - layout/reftests/fonts/mark-generate.py + - media/mtransport/nricectx.cpp + - netwerk/dns/prepare_tlds.py + - python/devtools/migrate-l10n/migrate/main.py + - python/l10n/convert_xul_to_fluent/convert.py + - python/l10n/convert_xul_to_fluent/lib/__init__.py + - python/l10n/convert_xul_to_fluent/lib/dtd.py + - python/l10n/convert_xul_to_fluent/lib/fluent.py + - python/l10n/convert_xul_to_fluent/lib/migration.py + - python/l10n/convert_xul_to_fluent/lib/utils.py + - python/l10n/convert_xul_to_fluent/lib/xul.py + - testing/mochitest/bisection.py + - testing/mozharness/configs/raptor/linux64_config_taskcluster.py + - testing/mozharness/configs/talos/linux64_config_taskcluster.py + - testing/mozharness/configs/web_platform_tests/test_config_windows.py + - testing/mozharness/external_tools/virtualenv/virtualenv_embedded/distutils-init.py + - testing/talos/talos/cmanager_base.py + - testing/talos/talos/profiler/profiling.py + - testing/talos/talos/unittests/conftest.py + - testing/talos/talos/unittests/test_ffsetup.py + - testing/talos/talos/unittests/test_test.py + - testing/talos/talos/unittests/test_xtalos/test_etlparser.py + - testing/web-platform/tests/content-security-policy/embedded-enforcement/support/echo-allow-csp-from.py + - testing/web-platform/tests/content-security-policy/embedded-enforcement/support/echo-policy-multiple.py + - testing/web-platform/tests/css/tools/apiclient/apiclient/__init__.py + - testing/web-platform/tests/css/tools/apiclient/apiclient/apiclient.py + - testing/web-platform/tests/css/tools/apiclient/apiclient/uritemplate.py + - testing/web-platform/tests/css/tools/apiclient/setup.py + - testing/web-platform/tests/css/tools/apiclient/test.py + - testing/web-platform/tests/css/tools/w3ctestlib/Groups.py + - testing/web-platform/tests/css/tools/w3ctestlib/HTMLSerializer.py + - testing/web-platform/tests/css/tools/w3ctestlib/Indexer.py + - testing/web-platform/tests/css/tools/w3ctestlib/OutputFormats.py + - testing/web-platform/tests/css/tools/w3ctestlib/Suite.py + - testing/web-platform/tests/css/tools/w3ctestlib/Utils.py + - testing/web-platform/tests/css/tools/w3ctestlib/__init__.py + - testing/web-platform/tests/tools/webdriver/webdriver/transport.py + - testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/edgechromium.py + - testing/web-platform/tests/tools/wptrunner/wptrunner/executors/executoredgechromium.py + - testing/web-platform/tests/tools/wptrunner/wptrunner/manifestupdate.py + - testing/web-platform/tests/tools/wptrunner/wptrunner/metadata.py + - testing/web-platform/tests/tools/wptrunner/wptrunner/tests/test_update.py + - toolkit/components/telemetry/build_scripts/setup.py + - toolkit/components/telemetry/tests/marionette/mach_commands.py + - toolkit/mozapps/installer/windows/nsis/preprocess-locale.py + - widget/nsFilePickerProxy.cpp + - widget/windows/tests/TestUrisToValidate.h + extensions: + - .c + - .cc + - .cpp + - .h + - .py + - .rs + support-files: + - 'tools/lint/file-whitespace/**' + type: external + payload: file-whitespace:lint diff --git a/tools/lint/file-whitespace/__init__.py b/tools/lint/file-whitespace/__init__.py new file mode 100644 index 000000000000..ed1734997f93 --- /dev/null +++ b/tools/lint/file-whitespace/__init__.py @@ -0,0 +1,61 @@ +# 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/. + +from __future__ import absolute_import + +from mozlint import result +from mozlint.pathutils import expand_exclusions + +results = [] + + +def lint(paths, config, fix=None, **lintargs): + files = list(expand_exclusions(paths, config, lintargs['root'])) + + for f in files: + with open(f, 'rb') as open_file: + hasFix = False + content_to_write = [] + for i, line in enumerate(open_file): + if line.endswith(" \n"): + # We found a trailing whitespace + if fix: + # We want to fix it, strip the trailing spaces + content_to_write.append(line.rstrip() + "\n") + hasFix = True + else: + res = {'path': f, + 'message': "Trailing whitespace", + 'level': 'error' + } + results.append(result.from_config(config, **res)) + else: + if fix: + content_to_write.append(line) + if hasFix: + # Only update the file when we found a change to make + with open(f, 'wb') as open_file_to_write: + open_file_to_write.write("".join(content_to_write)) + + # We are still using the same fp, let's return to the first + # line + open_file.seek(0) + # Open it as once as we just need to know if there is + # at least one \r\n + content = open_file.read() + + if "\r\n" in content: + if fix: + # replace \r\n by \n + content = content.replace(b'\r\n', b'\n') + with open(f, 'wb') as open_file_to_write: + open_file_to_write.write(content) + else: + res = {'path': f, + 'message': "Windows line return", + 'level': 'error' + } + results.append(result.from_config(config, **res)) + + return results From cf254e9cad1cee0ba2a9de96325cc3ed97a7fc4c Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Tue, 27 Aug 2019 21:46:55 +0000 Subject: [PATCH 14/41] Bug 1575250 - Run the check for trailing whitespaces and Windows CR in the CI r=ahal Differential Revision: https://phabricator.services.mozilla.com/D43360 --HG-- extra : moz-landing-system : lando --- taskcluster/ci/source-test/mozlint.yml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/taskcluster/ci/source-test/mozlint.yml b/taskcluster/ci/source-test/mozlint.yml index aebfed63ae89..c3313ad20549 100644 --- a/taskcluster/ci/source-test/mozlint.yml +++ b/taskcluster/ci/source-test/mozlint.yml @@ -223,6 +223,23 @@ rustfmt: - '**/*.rs' +file-whitespace: + description: Check for trailing whitespaces and Windows CR + platform: lint/opt + treeherder: + symbol: file-whitespace + run: + mach: lint -l file-whitespace -f treeherder -f json:/builds/worker/mozlint.json + when: + files-changed: + - '**/*.c' + - '**/*.cc' + - '**/*.cpp' + - '**/*.h' + - '**/*.py' + - '**/*.rs' + + file-perm: description: Check for incorrect permissions on source files platform: lint/opt From 2bbe6fef7a2fd0f0aa25fa28f3cf3bbad0b96c0d Mon Sep 17 00:00:00 2001 From: David Walsh Date: Tue, 27 Aug 2019 21:48:49 +0000 Subject: [PATCH 15/41] Bug 1576679 - Turn on inline preview by default r=jlast Differential Revision: https://phabricator.services.mozilla.com/D43490 --HG-- extra : moz-landing-system : lando --- .../src/actions/tests/pending-breakpoints.spec.js | 3 +++ .../client/debugger/src/actions/types/UIAction.js | 4 ++++ devtools/client/debugger/src/actions/ui.js | 9 +++++++++ .../client/debugger/src/components/Editor/index.js | 6 +++++- .../debugger/src/components/Editor/menus/editor.js | 13 +++++++++++++ devtools/client/debugger/src/reducers/ui.js | 13 ++++++++++++- devtools/client/locales/en-US/debugger.properties | 8 ++++++++ devtools/client/preferences/debugger.js | 2 +- ...rowser_webconsole_eval_in_debugger_stackframe.js | 4 ++++ ...owser_webconsole_eval_in_debugger_stackframe2.js | 4 ++++ ...ject_inspector_while_debugging_and_inspecting.js | 4 ++++ 11 files changed, 67 insertions(+), 3 deletions(-) diff --git a/devtools/client/debugger/src/actions/tests/pending-breakpoints.spec.js b/devtools/client/debugger/src/actions/tests/pending-breakpoints.spec.js index cf1f61955ec2..31b42d45da8e 100644 --- a/devtools/client/debugger/src/actions/tests/pending-breakpoints.spec.js +++ b/devtools/client/debugger/src/actions/tests/pending-breakpoints.spec.js @@ -30,6 +30,9 @@ jest.mock("../../utils/prefs", () => ({ pendingBreakpoints: {}, }, clear: jest.fn(), + features: { + inlinePreview: true, + }, })); import { diff --git a/devtools/client/debugger/src/actions/types/UIAction.js b/devtools/client/debugger/src/actions/types/UIAction.js index 39ff41e6ccd3..e9888b88ae75 100644 --- a/devtools/client/debugger/src/actions/types/UIAction.js +++ b/devtools/client/debugger/src/actions/types/UIAction.js @@ -30,6 +30,10 @@ export type UIAction = +type: "TOGGLE_FRAMEWORK_GROUPING", +value: boolean, |} + | {| + +type: "TOGGLE_INLINE_PREVIEW", + +value: boolean, + |} | {| +type: "SHOW_SOURCE", +source: Source, diff --git a/devtools/client/debugger/src/actions/ui.js b/devtools/client/debugger/src/actions/ui.js index 48673b26098c..7aeed2fd55f1 100644 --- a/devtools/client/debugger/src/actions/ui.js +++ b/devtools/client/debugger/src/actions/ui.js @@ -77,6 +77,15 @@ export function toggleFrameworkGrouping(toggleValue: boolean) { }; } +export function toggleInlinePreview(toggleValue: boolean) { + return ({ dispatch, getState }: ThunkArgs) => { + dispatch({ + type: "TOGGLE_INLINE_PREVIEW", + value: toggleValue, + }); + }; +} + export function showSource(cx: Context, sourceId: string) { return ({ dispatch, getState }: ThunkArgs) => { const source = getSource(getState(), sourceId); diff --git a/devtools/client/debugger/src/components/Editor/index.js b/devtools/client/debugger/src/components/Editor/index.js index 3e48897ba0b4..e622c13f0fe4 100644 --- a/devtools/client/debugger/src/components/Editor/index.js +++ b/devtools/client/debugger/src/components/Editor/index.js @@ -37,6 +37,7 @@ import { getCurrentThread, getThreadContext, getSkipPausing, + getInlinePreview, } from "../../selectors"; // Redux actions @@ -103,6 +104,7 @@ export type Props = { symbols: SymbolDeclarations, isPaused: boolean, skipPausing: boolean, + inlinePreviewEnabled: boolean, // Actions openConditionalPanel: typeof actions.openConditionalPanel, @@ -584,6 +586,7 @@ class Editor extends PureComponent { selectedSource, conditionalPanelLocation, isPaused, + inlinePreviewEnabled, } = this.props; const { editor, contextMenu } = this.state; @@ -611,7 +614,7 @@ class Editor extends PureComponent { {features.columnBreakpoints ? ( ) : null} - {isPaused && features.inlinePreview ? ( + {isPaused && inlinePreviewEnabled ? ( ) : null}
@@ -665,6 +668,7 @@ const mapStateToProps = state => { symbols: getSymbols(state, selectedSource), isPaused: getIsPaused(state, getCurrentThread(state)), skipPausing: getSkipPausing(state), + inlinePreviewEnabled: getInlinePreview(state), }; }; diff --git a/devtools/client/debugger/src/components/Editor/menus/editor.js b/devtools/client/debugger/src/components/Editor/menus/editor.js index ae7034eda58e..5d2c0d226b74 100644 --- a/devtools/client/debugger/src/components/Editor/menus/editor.js +++ b/devtools/client/debugger/src/components/Editor/menus/editor.js @@ -15,6 +15,7 @@ import { } from "../../../utils/source"; import { downloadFile } from "../../../utils/utils"; +import { features } from "../../../utils/prefs"; import { isFulfilled } from "../../../utils/async-value"; import actions from "../../../actions"; @@ -159,6 +160,14 @@ const downloadFileItem = ( click: () => downloadFile(selectedContent, getFilename(selectedSource)), }); +const inlinePreviewItem = (editorActions: EditorItemActions) => ({ + id: "node-menu-inline-preview", + label: features.inlinePreview + ? L10N.getStr("inlinePreview.disable.label") + : L10N.getStr("inlinePreview.enable.label"), + click: () => editorActions.toggleInlinePreview(!features.inlinePreview), +}); + export function editorMenuItems({ cx, editorActions, @@ -218,6 +227,8 @@ export function editorMenuItems({ ); } + items.push({ type: "separator" }, inlinePreviewItem(editorActions)); + return items; } @@ -229,6 +240,7 @@ export type EditorItemActions = { jumpToMappedLocation: typeof actions.jumpToMappedLocation, showSource: typeof actions.showSource, toggleBlackBox: typeof actions.toggleBlackBox, + toggleInlinePreview: typeof actions.toggleInlinePreview, }; export function editorItemActions(dispatch: Function) { @@ -241,6 +253,7 @@ export function editorItemActions(dispatch: Function) { jumpToMappedLocation: actions.jumpToMappedLocation, showSource: actions.showSource, toggleBlackBox: actions.toggleBlackBox, + toggleInlinePreview: actions.toggleInlinePreview, }, dispatch ); diff --git a/devtools/client/debugger/src/reducers/ui.js b/devtools/client/debugger/src/reducers/ui.js index 0efb92d43818..720fd18677c8 100644 --- a/devtools/client/debugger/src/reducers/ui.js +++ b/devtools/client/debugger/src/reducers/ui.js @@ -9,7 +9,7 @@ * @module reducers/ui */ -import { prefs } from "../utils/prefs"; +import { prefs, features } from "../utils/prefs"; import type { Source, Range, SourceLocation } from "../types"; @@ -37,6 +37,7 @@ export type UIState = { }, conditionalPanelLocation: null | SourceLocation, isLogPoint: boolean, + inlinePreviewEnabled: boolean, }; export const createUIState = (): UIState => ({ @@ -51,6 +52,7 @@ export const createUIState = (): UIState => ({ isLogPoint: false, orientation: "horizontal", viewport: null, + inlinePreviewEnabled: features.inlinePreview, }); function update(state: UIState = createUIState(), action: Action): UIState { @@ -64,6 +66,11 @@ function update(state: UIState = createUIState(), action: Action): UIState { return { ...state, frameworkGroupingOn: action.value }; } + case "TOGGLE_INLINE_PREVIEW": { + features.inlinePreview = action.value; + return { ...state, inlinePreviewEnabled: action.value }; + } + case "SET_ORIENTATION": { return { ...state, orientation: action.orientation }; } @@ -185,4 +192,8 @@ export function getViewport(state: OuterState) { return state.ui.viewport; } +export function getInlinePreview(state: OuterState) { + return state.ui.inlinePreviewEnabled; +} + export default update; diff --git a/devtools/client/locales/en-US/debugger.properties b/devtools/client/locales/en-US/debugger.properties index f42e1eecd72e..18066f5012c5 100644 --- a/devtools/client/locales/en-US/debugger.properties +++ b/devtools/client/locales/en-US/debugger.properties @@ -430,6 +430,14 @@ editor.jumpToMappedLocation1.accesskey=m downloadFile.label=Download file downloadFile.accesskey=d +# LOCALIZATION NOTE (inlinePreview.enable.label): Context menu item +# for enabling the inline preview feature +inlinePreview.enable.label=Enable inline preview + +# LOCALIZATION NOTE (inlinePreview.disable.label): Context menu item +# for disabling the inline preview feature +inlinePreview.disable.label=Disable inline preview + # LOCALIZATION NOTE (framework.disableGrouping): This is the text that appears in the # context menu to disable framework grouping. framework.disableGrouping=Disable framework grouping diff --git a/devtools/client/preferences/debugger.js b/devtools/client/preferences/debugger.js index a21278e7cd4f..080870ff33ba 100644 --- a/devtools/client/preferences/debugger.js +++ b/devtools/client/preferences/debugger.js @@ -83,4 +83,4 @@ pref("devtools.debugger.features.event-listeners-breakpoints", true); pref("devtools.debugger.features.dom-mutation-breakpoints", true); pref("devtools.debugger.features.log-points", true); pref("devtools.debugger.features.overlay-step-buttons", false); -pref("devtools.debugger.features.inline-preview", false); +pref("devtools.debugger.features.inline-preview", true); diff --git a/devtools/client/webconsole/test/browser/browser_webconsole_eval_in_debugger_stackframe.js b/devtools/client/webconsole/test/browser/browser_webconsole_eval_in_debugger_stackframe.js index 93b0202a89b5..a52d61208d72 100644 --- a/devtools/client/webconsole/test/browser/browser_webconsole_eval_in_debugger_stackframe.js +++ b/devtools/client/webconsole/test/browser/browser_webconsole_eval_in_debugger_stackframe.js @@ -12,6 +12,10 @@ const TEST_URI = "test/browser/test-eval-in-stackframe.html"; add_task(async function() { + // TODO: Remove this pref change when middleware for terminating requests + // when closing a panel is implemented + await pushPref("devtools.debugger.features.inline-preview", false); + info("open the console"); const hud = await openNewTabAndConsole(TEST_URI); diff --git a/devtools/client/webconsole/test/browser/browser_webconsole_eval_in_debugger_stackframe2.js b/devtools/client/webconsole/test/browser/browser_webconsole_eval_in_debugger_stackframe2.js index 359575224929..76d9d91cb8b1 100644 --- a/devtools/client/webconsole/test/browser/browser_webconsole_eval_in_debugger_stackframe2.js +++ b/devtools/client/webconsole/test/browser/browser_webconsole_eval_in_debugger_stackframe2.js @@ -12,6 +12,10 @@ const TEST_URI = "test/browser/test-eval-in-stackframe.html"; add_task(async function() { + // TODO: Remove this pref change when middleware for terminating requests + // when closing a panel is implemented + await pushPref("devtools.debugger.features.inline-preview", false); + info("open the console"); const hud = await openNewTabAndConsole(TEST_URI); diff --git a/devtools/client/webconsole/test/browser/browser_webconsole_object_inspector_while_debugging_and_inspecting.js b/devtools/client/webconsole/test/browser/browser_webconsole_object_inspector_while_debugging_and_inspecting.js index 63a2f5769993..cba19fa1616a 100644 --- a/devtools/client/webconsole/test/browser/browser_webconsole_object_inspector_while_debugging_and_inspecting.js +++ b/devtools/client/webconsole/test/browser/browser_webconsole_object_inspector_while_debugging_and_inspecting.js @@ -11,6 +11,10 @@ const TEST_URI = "test/browser/test-eval-in-stackframe.html"; add_task(async function() { + // TODO: Remove this pref change when middleware for terminating requests + // when closing a panel is implemented + await pushPref("devtools.debugger.features.inline-preview", false); + const hud = await openNewTabAndConsole(TEST_URI); info("Switch to the debugger"); From e008eddf527ac93d771785f32631cf9559823872 Mon Sep 17 00:00:00 2001 From: Tom Ritter Date: Tue, 27 Aug 2019 22:02:23 +0000 Subject: [PATCH 16/41] Bug 1575975 - Enable the maintenance service for MinGW r=firefox-build-system-reviewers,mshal We don't need to disable it. The problem with this commit is that we want to keep the --disable-maintenance-service flag working, so we might want a separate build for that (and other unusual --disable flags) - but such a build can be clearly labeled so people understand why things broke. Differential Revision: https://phabricator.services.mozilla.com/D43145 --HG-- extra : moz-landing-system : lando --- browser/config/mozconfigs/win32/mingwclang | 1 - browser/config/mozconfigs/win64/mingwclang | 1 - toolkit/components/maintenanceservice/moz.build | 7 +++++++ 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/browser/config/mozconfigs/win32/mingwclang b/browser/config/mozconfigs/win32/mingwclang index d7656e18b9fc..477b1435ffb9 100644 --- a/browser/config/mozconfigs/win32/mingwclang +++ b/browser/config/mozconfigs/win32/mingwclang @@ -41,7 +41,6 @@ ac_add_options --disable-jemalloc ac_add_options --enable-proxy-bypass-protection # These aren't supported on mingw at this time -ac_add_options --disable-maintenance-service ac_add_options --disable-webrtc # Bug 1393901 ac_add_options --disable-geckodriver # Bug 1489320 diff --git a/browser/config/mozconfigs/win64/mingwclang b/browser/config/mozconfigs/win64/mingwclang index 8170d393b368..bd25a57acc67 100755 --- a/browser/config/mozconfigs/win64/mingwclang +++ b/browser/config/mozconfigs/win64/mingwclang @@ -41,7 +41,6 @@ ac_add_options --disable-jemalloc ac_add_options --enable-proxy-bypass-protection # These aren't supported on mingw at this time -ac_add_options --disable-maintenance-service ac_add_options --disable-webrtc # Bug 1393901 ac_add_options --disable-geckodriver # Bug 1489320 diff --git a/toolkit/components/maintenanceservice/moz.build b/toolkit/components/maintenanceservice/moz.build index 856b9278632c..0d9f7d2e5769 100644 --- a/toolkit/components/maintenanceservice/moz.build +++ b/toolkit/components/maintenanceservice/moz.build @@ -35,6 +35,12 @@ USE_STATIC_LIBS = True if CONFIG['CC_TYPE'] == 'clang-cl': WIN32_EXE_LDFLAGS += ['-ENTRY:wmainCRTStartup'] +if CONFIG['OS_TARGET'] == 'WINNT' and CONFIG['CC_TYPE'] in ('gcc', 'clang'): + # This allows us to use wmain as the entry point on mingw + LDFLAGS += [ + '-municode', + ] + RCINCLUDE = 'maintenanceservice.rc' DisableStlWrapping() @@ -43,6 +49,7 @@ OS_LIBS += [ 'comctl32', 'ws2_32', 'shell32', + 'shlwapi', ] with Files('**'): From 3efe2b6ed0dfabceea22227842efa131e9da741c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 27 Aug 2019 22:10:46 +0000 Subject: [PATCH 17/41] Bug 1554498 - Don't use nsIMutationObserver for ShadowRoot. r=smaug This penalizes a bit non-shadow-DOM content, in exchange of making Shadow DOM slightly faster as well. The biggest advantage of this is that all ContentRemoved notifications would see the flattened tree before the changes, which is something a11y needs to be correct. XBL would still not be handled right by a11y, but that's not new and content cannot do random stuff with XBL so it's not too bad. Differential Revision: https://phabricator.services.mozilla.com/D32639 --HG-- extra : moz-landing-system : lando --- dom/base/CharacterData.cpp | 7 ++ dom/base/Element.cpp | 29 ++++--- dom/base/ShadowRoot.cpp | 149 ++++++++++------------------------- dom/base/ShadowRoot.h | 23 +++--- dom/base/nsIContent.h | 16 ++++ dom/base/nsIContentInlines.h | 47 +++++++++++ dom/html/HTMLSlotElement.cpp | 25 +++--- dom/html/HTMLSlotElement.h | 6 +- 8 files changed, 157 insertions(+), 145 deletions(-) diff --git a/dom/base/CharacterData.cpp b/dom/base/CharacterData.cpp index 9a656d509fca..f1837e722f3f 100644 --- a/dom/base/CharacterData.cpp +++ b/dom/base/CharacterData.cpp @@ -489,6 +489,11 @@ nsresult CharacterData::BindToTree(BindContext& aContext, nsINode& aParent) { UpdateEditableState(false); + // Ensure we only do these once, in the case we move the shadow host around. + if (aContext.SubtreeRootChanges()) { + HandleShadowDOMRelatedInsertionSteps(hadParent); + } + MOZ_ASSERT(OwnerDoc() == aParent.OwnerDoc(), "Bound to wrong document"); MOZ_ASSERT(IsInComposedDoc() == aContext.InComposedDoc()); MOZ_ASSERT(IsInUncomposedDoc() == aContext.InUncomposedDoc()); @@ -506,6 +511,8 @@ void CharacterData::UnbindFromTree(bool aNullParent) { // Unset frame flags; if we need them again later, they'll get set again. UnsetFlags(NS_CREATE_FRAME_IF_NON_WHITESPACE | NS_REFRAME_IF_WHITESPACE); + HandleShadowDOMRelatedRemovalSteps(aNullParent); + Document* document = GetComposedDoc(); if (aNullParent) { diff --git a/dom/base/Element.cpp b/dom/base/Element.cpp index 6f96f139ad9d..895ddafaa410 100644 --- a/dom/base/Element.cpp +++ b/dom/base/Element.cpp @@ -1721,6 +1721,7 @@ nsresult Element::BindToTree(BindContext& aContext, nsINode& aParent) { if (HasID()) { AddToIdTable(DoGetID()); } + HandleShadowDOMRelatedInsertionSteps(hadParent); } if (MayHaveStyle() && !IsXULElement()) { @@ -1801,6 +1802,8 @@ bool WillDetachFromShadowOnUnbind(const Element& aElement, bool aNullParent) { } void Element::UnbindFromTree(bool aNullParent) { + HandleShadowDOMRelatedRemovalSteps(aNullParent); + const bool detachingFromShadow = WillDetachFromShadowOnUnbind(*this, aNullParent); // Make sure to only remove from the ID table if our subtree root is actually @@ -2629,19 +2632,25 @@ nsresult Element::AfterSetAttr(int32_t aNamespaceID, nsAtom* aName, const nsAttrValue* aOldValue, nsIPrincipal* aMaybeScriptedPrincipal, bool aNotify) { - if (aNamespaceID == kNameSpaceID_None && aName == nsGkAtoms::part) { - bool isPart = !!aValue; - if (HasPartAttribute() != isPart) { - SetHasPartAttribute(isPart); - if (ShadowRoot* shadow = GetContainingShadow()) { - if (isPart) { - shadow->PartAdded(*this); - } else { - shadow->PartRemoved(*this); + if (aNamespaceID == kNameSpaceID_None) { + if (aName == nsGkAtoms::part) { + bool isPart = !!aValue; + if (HasPartAttribute() != isPart) { + SetHasPartAttribute(isPart); + if (ShadowRoot* shadow = GetContainingShadow()) { + if (isPart) { + shadow->PartAdded(*this); + } else { + shadow->PartRemoved(*this); + } } } + MOZ_ASSERT(HasPartAttribute() == isPart); + } else if (aName == nsGkAtoms::slot && GetParent()) { + if (ShadowRoot* shadow = GetParent()->GetShadowRoot()) { + shadow->MaybeReassignElement(*this); + } } - MOZ_ASSERT(HasPartAttribute() == isPart); } return NS_OK; } diff --git a/dom/base/ShadowRoot.cpp b/dom/base/ShadowRoot.cpp index d4367d4dd150..98aaaa58b437 100644 --- a/dom/base/ShadowRoot.cpp +++ b/dom/base/ShadowRoot.cpp @@ -36,15 +36,11 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(ShadowRoot, DocumentFragment) NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(ShadowRoot) - if (tmp->GetHost()) { - tmp->GetHost()->RemoveMutationObserver(tmp); - } DocumentOrShadowRoot::Unlink(tmp); NS_IMPL_CYCLE_COLLECTION_UNLINK_END_INHERITED(DocumentFragment) NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(ShadowRoot) NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIContent) - NS_INTERFACE_MAP_ENTRY(nsIMutationObserver) NS_INTERFACE_MAP_ENTRY(nsIRadioGroupContainer) NS_INTERFACE_MAP_END_INHERITING(DocumentFragment) @@ -69,19 +65,9 @@ ShadowRoot::ShadowRoot(Element* aElement, ShadowRootMode aMode, ExtendedDOMSlots()->mBindingParent = aElement; ExtendedDOMSlots()->mContainingShadow = this; - - // Add the ShadowRoot as a mutation observer on the host to watch - // for mutations because the insertion points in this ShadowRoot - // may need to be updated when the host children are modified. - GetHost()->AddMutationObserver(this); } ShadowRoot::~ShadowRoot() { - if (auto* host = GetHost()) { - // mHost may have been unlinked. - host->RemoveMutationObserver(this); - } - if (IsInComposedDoc()) { OwnerDoc()->RemoveComposedDocShadowRoot(*this); } @@ -158,13 +144,11 @@ void ShadowRoot::Unattach() { MOZ_ASSERT(!HasSlots(), "Won't work!"); if (!GetHost()) { // It is possible that we've been unlinked already. In such case host - // should have called Unbind and ShadowRoot's own unlink - // RemoveMutationObserver. + // should have called Unbind and ShadowRoot's own unlink. return; } Unbind(); - GetHost()->RemoveMutationObserver(this); SetHost(nullptr); } @@ -220,8 +204,8 @@ void ShadowRoot::AddSlot(HTMLSlotElement* aSlot) { while (assignedNodes.Length() > 0) { nsINode* assignedNode = assignedNodes[0]; - oldSlot->RemoveAssignedNode(assignedNode); - aSlot->AppendAssignedNode(assignedNode); + oldSlot->RemoveAssignedNode(*assignedNode->AsContent()); + aSlot->AppendAssignedNode(*assignedNode->AsContent()); doEnqueueSlotChange = true; } @@ -245,7 +229,7 @@ void ShadowRoot::AddSlot(HTMLSlotElement* aSlot) { continue; } doEnqueueSlotChange = true; - aSlot->AppendAssignedNode(child); + aSlot->AppendAssignedNode(*child); } if (doEnqueueSlotChange) { @@ -299,8 +283,8 @@ void ShadowRoot::RemoveSlot(HTMLSlotElement* aSlot) { while (!assignedNodes.IsEmpty()) { nsINode* assignedNode = assignedNodes[0]; - aSlot->RemoveAssignedNode(assignedNode); - replacementSlot->AppendAssignedNode(assignedNode); + aSlot->RemoveAssignedNode(*assignedNode->AsContent()); + replacementSlot->AppendAssignedNode(*assignedNode->AsContent()); } aSlot->EnqueueSlotChangeEvent(); @@ -488,7 +472,7 @@ void ShadowRoot::GetEventTargetParent(EventChainPreVisitor& aVisitor) { } } -ShadowRoot::SlotAssignment ShadowRoot::SlotAssignmentFor(nsIContent* aContent) { +ShadowRoot::SlotAssignment ShadowRoot::SlotAssignmentFor(nsIContent& aContent) { nsAutoString slotName; // Note that if slot attribute is missing, assign it to the first default // slot, if exists. @@ -513,7 +497,7 @@ ShadowRoot::SlotAssignment ShadowRoot::SlotAssignmentFor(nsIContent* aContent) { // Seek through the host's explicit children until the // assigned content is found. while (currentContent && currentContent != assignedNodes[i]) { - if (currentContent == aContent) { + if (currentContent == &aContent) { insertionIndex.emplace(i); break; } @@ -529,9 +513,9 @@ ShadowRoot::SlotAssignment ShadowRoot::SlotAssignmentFor(nsIContent* aContent) { return {slot, insertionIndex}; } -void ShadowRoot::MaybeReassignElement(Element* aElement) { - MOZ_ASSERT(aElement->GetParent() == GetHost()); - HTMLSlotElement* oldSlot = aElement->GetAssignedSlot(); +void ShadowRoot::MaybeReassignElement(Element& aElement) { + MOZ_ASSERT(aElement.GetParent() == GetHost()); + HTMLSlotElement* oldSlot = aElement.GetAssignedSlot(); SlotAssignment assignment = SlotAssignmentFor(aElement); if (assignment.mSlot == oldSlot) { @@ -541,7 +525,7 @@ void ShadowRoot::MaybeReassignElement(Element* aElement) { if (Document* doc = GetComposedDoc()) { if (RefPtr presShell = doc->GetPresShell()) { - presShell->SlotAssignmentWillChange(*aElement, oldSlot, assignment.mSlot); + presShell->SlotAssignmentWillChange(aElement, oldSlot, assignment.mSlot); } } @@ -615,103 +599,56 @@ nsINode* ShadowRoot::CreateElementAndAppendChildAt(nsINode& aParentNode, return aParentNode.AppendChild(*node, rv); } -void ShadowRoot::AttributeChanged(Element* aElement, int32_t aNameSpaceID, - nsAtom* aAttribute, int32_t aModType, - const nsAttrValue* aOldValue) { - if (aNameSpaceID != kNameSpaceID_None || aAttribute != nsGkAtoms::slot) { +void ShadowRoot::MaybeUnslotHostChild(nsIContent& aChild) { + // Need to null-check the host because we may be unlinked already. + MOZ_ASSERT(!GetHost() || aChild.GetParent() == GetHost()); + + HTMLSlotElement* slot = aChild.GetAssignedSlot(); + if (!slot) { return; } - if (aElement->GetParent() != GetHost()) { - return; + MOZ_DIAGNOSTIC_ASSERT(!aChild.IsRootOfAnonymousSubtree(), + "How did aChild end up assigned to a slot?"); + // If the slot is going to start showing fallback content, we need to tell + // layout about it. + if (slot->AssignedNodes().Length() == 1) { + InvalidateStyleAndLayoutOnSubtree(slot); } - MaybeReassignElement(aElement); + slot->RemoveAssignedNode(aChild); + slot->EnqueueSlotChangeEvent(); } -void ShadowRoot::ContentAppended(nsIContent* aFirstNewContent) { - for (nsIContent* content = aFirstNewContent; content; - content = content->GetNextSibling()) { - ContentInserted(content); - } -} - -void ShadowRoot::ContentInserted(nsIContent* aChild) { - // Check to ensure that the child not an anonymous subtree root because - // even though its parent could be the host it may not be in the host's child - // list. - if (aChild->IsRootOfAnonymousSubtree()) { +void ShadowRoot::MaybeSlotHostChild(nsIContent& aChild) { + MOZ_ASSERT(aChild.GetParent() == GetHost()); + // Check to ensure that the child not an anonymous subtree root because even + // though its parent could be the host it may not be in the host's child list. + if (aChild.IsRootOfAnonymousSubtree()) { return; } - if (!aChild->IsSlotable()) { + if (!aChild.IsSlotable()) { return; } - if (aChild->GetParent() == GetHost()) { - SlotAssignment assignment = SlotAssignmentFor(aChild); - if (!assignment.mSlot) { - return; - } - - // Fallback content will go away, let layout know. - if (assignment.mSlot->AssignedNodes().IsEmpty()) { - InvalidateStyleAndLayoutOnSubtree(assignment.mSlot); - } - - if (assignment.mIndex) { - assignment.mSlot->InsertAssignedNode(*assignment.mIndex, aChild); - } else { - assignment.mSlot->AppendAssignedNode(aChild); - } - assignment.mSlot->EnqueueSlotChangeEvent(); - - SlotStateChanged(assignment.mSlot); + SlotAssignment assignment = SlotAssignmentFor(aChild); + if (!assignment.mSlot) { return; } - // If parent's root is a shadow root, and parent is a slot whose assigned - // nodes is the empty list, then run signal a slot change for parent. - HTMLSlotElement* slot = HTMLSlotElement::FromNodeOrNull(aChild->GetParent()); - if (slot && slot->GetContainingShadow() == this && - slot->AssignedNodes().IsEmpty()) { - slot->EnqueueSlotChangeEvent(); - } -} - -void ShadowRoot::ContentRemoved(nsIContent* aChild, - nsIContent* aPreviousSibling) { - // Check to ensure that the child not an anonymous subtree root because - // even though its parent could be the host it may not be in the host's child - // list. - if (aChild->IsRootOfAnonymousSubtree()) { - return; + // Fallback content will go away, let layout know. + if (assignment.mSlot->AssignedNodes().IsEmpty()) { + InvalidateStyleAndLayoutOnSubtree(assignment.mSlot); } - if (!aChild->IsSlotable()) { - return; - } - - if (aChild->GetParent() == GetHost()) { - if (HTMLSlotElement* slot = aChild->GetAssignedSlot()) { - // If the slot is going to start showing fallback content, we need to tell - // layout about it. - if (slot->AssignedNodes().Length() == 1) { - InvalidateStyleAndLayoutOnSubtree(slot); - } - slot->RemoveAssignedNode(aChild); - slot->EnqueueSlotChangeEvent(); - } - return; - } - - // If parent's root is a shadow root, and parent is a slot whose assigned - // nodes is the empty list, then run signal a slot change for parent. - HTMLSlotElement* slot = HTMLSlotElement::FromNodeOrNull(aChild->GetParent()); - if (slot && slot->GetContainingShadow() == this && - slot->AssignedNodes().IsEmpty()) { - slot->EnqueueSlotChangeEvent(); + if (assignment.mIndex) { + assignment.mSlot->InsertAssignedNode(*assignment.mIndex, aChild); + } else { + assignment.mSlot->AppendAssignedNode(aChild); } + assignment.mSlot->EnqueueSlotChangeEvent(); + SlotStateChanged(assignment.mSlot); } ServoStyleRuleMap& ShadowRoot::ServoStyleRuleMap() { diff --git a/dom/base/ShadowRoot.h b/dom/base/ShadowRoot.h index 4de3c5440e34..362d5465d2d6 100644 --- a/dom/base/ShadowRoot.h +++ b/dom/base/ShadowRoot.h @@ -39,7 +39,6 @@ class HTMLInputElement; class ShadowRoot final : public DocumentFragment, public DocumentOrShadowRoot, - public nsStubMutationObserver, public nsIRadioGroupContainer { public: NS_IMPL_FROMNODE_HELPER(ShadowRoot, IsShadowRoot()); @@ -47,16 +46,20 @@ class ShadowRoot final : public DocumentFragment, NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(ShadowRoot, DocumentFragment) NS_DECL_ISUPPORTS_INHERITED - NS_DECL_NSIMUTATIONOBSERVER_ATTRIBUTECHANGED - NS_DECL_NSIMUTATIONOBSERVER_CONTENTAPPENDED - NS_DECL_NSIMUTATIONOBSERVER_CONTENTINSERTED - NS_DECL_NSIMUTATIONOBSERVER_CONTENTREMOVED - ShadowRoot(Element* aElement, ShadowRootMode aMode, already_AddRefed&& aNodeInfo); void AddSizeOfExcludingThis(nsWindowSizes&, size_t* aNodeSize) const final; + // Try to reassign an element to a slot. + void MaybeReassignElement(Element&); + // Called when an element is inserted as a direct child of our host. Tries to + // slot the child in one of our slots. + void MaybeSlotHostChild(nsIContent&); + // Called when a direct child of our host is removed. Tries to un-slot the + // child from the currently-assigned slot, if any. + void MaybeUnslotHostChild(nsIContent&); + // Shadow DOM v1 Element* Host() const { MOZ_ASSERT(GetHost(), @@ -106,12 +109,6 @@ class ShadowRoot final : public DocumentFragment, InsertSheetAt(SheetCount(), aSheet); } - /** - * Try to reassign an element to a slot and returns whether the assignment - * changed. - */ - void MaybeReassignElement(Element* aElement); - /** * Represents the insertion point in a slot for a given node. */ @@ -131,7 +128,7 @@ class ShadowRoot final : public DocumentFragment, * It's the caller's responsibility to actually call InsertAssignedNode / * AppendAssignedNode in the slot as needed. */ - SlotAssignment SlotAssignmentFor(nsIContent* aContent); + SlotAssignment SlotAssignmentFor(nsIContent&); /** * Explicitly invalidates the style and layout of the flattened-tree subtree diff --git a/dom/base/nsIContent.h b/dom/base/nsIContent.h index 620458fac1f6..3a1c75f47674 100644 --- a/dom/base/nsIContent.h +++ b/dom/base/nsIContent.h @@ -492,6 +492,22 @@ class nsIContent : public nsINode { */ inline nsIContent* GetFlattenedTreeParent() const; + protected: + // Handles getting inserted or removed directly under a element. + // This is meant to only be called from the two functions below. + inline void HandleInsertionToOrRemovalFromSlot(); + + // Handles Shadow-DOM-related state tracking. Meant to be called near the + // end of BindToTree(), only if the tree we're in actually changed, that is, + // after the subtree has been bound to the new parent. + inline void HandleShadowDOMRelatedInsertionSteps(bool aHadParent); + + // Handles Shadow-DOM related state tracking. Meant to be called near the + // beginning of UnbindFromTree(), before the node has lost the reference to + // its parent. + inline void HandleShadowDOMRelatedRemovalSteps(bool aNullParent); + + public: /** * API to check if this is a link that's traversed in response to user input * (e.g. a click event). Specializations for HTML/SVG/generic XML allow for diff --git a/dom/base/nsIContentInlines.h b/dom/base/nsIContentInlines.h index a0fc003d8d9c..8149eae92098 100644 --- a/dom/base/nsIContentInlines.h +++ b/dom/base/nsIContentInlines.h @@ -221,4 +221,51 @@ inline bool nsIContent::IsInAnonymousSubtree() const { return !bindingParent->GetShadowRoot(); } +inline void nsIContent::HandleInsertionToOrRemovalFromSlot() { + using mozilla::dom::HTMLSlotElement; + + MOZ_ASSERT(GetParentElement()); + if (!IsInShadowTree() || IsRootOfAnonymousSubtree()) { + return; + } + HTMLSlotElement* slot = HTMLSlotElement::FromNode(mParent); + if (!slot) { + return; + } + // If parent's root is a shadow root, and parent is a slot whose + // assigned nodes is the empty list, then run signal a slot change for + // parent. + if (slot->AssignedNodes().IsEmpty()) { + slot->EnqueueSlotChangeEvent(); + } +} + +inline void nsIContent::HandleShadowDOMRelatedInsertionSteps(bool aHadParent) { + using mozilla::dom::Element; + using mozilla::dom::ShadowRoot; + + if (!aHadParent) { + if (Element* parentElement = Element::FromNode(mParent)) { + if (ShadowRoot* shadow = parentElement->GetShadowRoot()) { + shadow->MaybeSlotHostChild(*this); + } + HandleInsertionToOrRemovalFromSlot(); + } + } +} + +inline void nsIContent::HandleShadowDOMRelatedRemovalSteps(bool aNullParent) { + using mozilla::dom::Element; + using mozilla::dom::ShadowRoot; + + if (aNullParent) { + if (Element* parentElement = Element::FromNode(mParent)) { + if (ShadowRoot* shadow = parentElement->GetShadowRoot()) { + shadow->MaybeUnslotHostChild(*this); + } + HandleInsertionToOrRemovalFromSlot(); + } + } +} + #endif // nsIContentInlines_h diff --git a/dom/html/HTMLSlotElement.cpp b/dom/html/HTMLSlotElement.cpp index 35d31332feb4..c1070344a09b 100644 --- a/dom/html/HTMLSlotElement.cpp +++ b/dom/html/HTMLSlotElement.cpp @@ -154,26 +154,25 @@ const nsTArray>& HTMLSlotElement::AssignedNodes() const { return mAssignedNodes; } -void HTMLSlotElement::InsertAssignedNode(uint32_t aIndex, nsINode* aNode) { - MOZ_ASSERT(!aNode->AsContent()->GetAssignedSlot(), "Losing track of a slot"); - mAssignedNodes.InsertElementAt(aIndex, aNode); - aNode->AsContent()->SetAssignedSlot(this); +void HTMLSlotElement::InsertAssignedNode(uint32_t aIndex, nsIContent& aNode) { + MOZ_ASSERT(!aNode.GetAssignedSlot(), "Losing track of a slot"); + mAssignedNodes.InsertElementAt(aIndex, &aNode); + aNode.SetAssignedSlot(this); } -void HTMLSlotElement::AppendAssignedNode(nsINode* aNode) { - MOZ_ASSERT(!aNode->AsContent()->GetAssignedSlot(), "Losing track of a slot"); - mAssignedNodes.AppendElement(aNode); - aNode->AsContent()->SetAssignedSlot(this); +void HTMLSlotElement::AppendAssignedNode(nsIContent& aNode) { + MOZ_ASSERT(!aNode.GetAssignedSlot(), "Losing track of a slot"); + mAssignedNodes.AppendElement(&aNode); + aNode.SetAssignedSlot(this); } -void HTMLSlotElement::RemoveAssignedNode(nsINode* aNode) { +void HTMLSlotElement::RemoveAssignedNode(nsIContent& aNode) { // This one runs from unlinking, so we can't guarantee that the slot pointer // hasn't been cleared. - MOZ_ASSERT(!aNode->AsContent()->GetAssignedSlot() || - aNode->AsContent()->GetAssignedSlot() == this, + MOZ_ASSERT(!aNode.GetAssignedSlot() || aNode.GetAssignedSlot() == this, "How exactly?"); - mAssignedNodes.RemoveElement(aNode); - aNode->AsContent()->SetAssignedSlot(nullptr); + mAssignedNodes.RemoveElement(&aNode); + aNode.SetAssignedSlot(nullptr); } void HTMLSlotElement::ClearAssignedNodes() { diff --git a/dom/html/HTMLSlotElement.h b/dom/html/HTMLSlotElement.h index 8c2f46f3544b..136fa267af30 100644 --- a/dom/html/HTMLSlotElement.h +++ b/dom/html/HTMLSlotElement.h @@ -54,9 +54,9 @@ class HTMLSlotElement final : public nsGenericHTMLElement { // Helper methods const nsTArray>& AssignedNodes() const; - void InsertAssignedNode(uint32_t aIndex, nsINode* aNode); - void AppendAssignedNode(nsINode* aNode); - void RemoveAssignedNode(nsINode* aNode); + void InsertAssignedNode(uint32_t aIndex, nsIContent&); + void AppendAssignedNode(nsIContent&); + void RemoveAssignedNode(nsIContent&); void ClearAssignedNodes(); void EnqueueSlotChangeEvent(); From 0176e7d22e127e2c2e8e1abb0e59c590091a5703 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 27 Aug 2019 22:09:26 +0000 Subject: [PATCH 18/41] Bug 1554498 - Fix an assertion that starts firing with the previous patch after addressing review feedback. r=smaug After addressing review comments in the other patch of this bug, debug builds assert all the time here when called from UnbindFromTree() on an already-unbound subtree. We clear the binding parent on unbind, so this is only guaranteed to match for connected nodes, as far as I can tell. Differential Revision: https://phabricator.services.mozilla.com/D43641 --HG-- extra : moz-landing-system : lando --- dom/base/Element.cpp | 1 - dom/base/FragmentOrElement.cpp | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/dom/base/Element.cpp b/dom/base/Element.cpp index 895ddafaa410..b67d20da9a41 100644 --- a/dom/base/Element.cpp +++ b/dom/base/Element.cpp @@ -1925,7 +1925,6 @@ void Element::UnbindFromTree(bool aNullParent) { #ifdef MOZ_XUL if (nsXULElement* xulElem = nsXULElement::FromNode(this)) { - ; xulElem->SetXULBindingParent(nullptr); clearBindingParent = false; } diff --git a/dom/base/FragmentOrElement.cpp b/dom/base/FragmentOrElement.cpp index a9f8fc4e89e4..10023755b730 100644 --- a/dom/base/FragmentOrElement.cpp +++ b/dom/base/FragmentOrElement.cpp @@ -1118,7 +1118,7 @@ void nsIContent::AssertAnonymousSubtreeRelatedInvariants() const { (GetParent() && GetBindingParent() == GetParent()), "root of native anonymous subtree must have parent equal " "to binding parent"); - NS_ASSERTION(!GetParent() || + NS_ASSERTION(!GetParent() || !IsInComposedDoc() || ((GetBindingParent() == GetParent()) == HasFlag(NODE_IS_ANONYMOUS_ROOT)) || // Unfortunately default content for XBL insertion points @@ -1129,7 +1129,7 @@ void nsIContent::AssertAnonymousSubtreeRelatedInvariants() const { (GetBindingParent() && (GetBindingParent() == GetParent()->GetBindingParent()) == HasFlag(NODE_IS_ANONYMOUS_ROOT)), - "For nodes with parent, flag and GetBindingParent() check " + "For connected nodes, flag and GetBindingParent() check " "should match"); } #endif From 37a0bb26bb20017a9255d563eba395725f1d0155 Mon Sep 17 00:00:00 2001 From: Chris Manchester Date: Tue, 27 Aug 2019 22:10:54 +0000 Subject: [PATCH 19/41] Bug 1576737 - Add sccache-dist in offices docs to the tree. r=firefox-build-system-reviewers,mshal Differential Revision: https://phabricator.services.mozilla.com/D43531 --HG-- extra : moz-landing-system : lando --- build/docs/index.rst | 1 + build/docs/sccache-dist.rst | 117 ++++++++++++++++++++++++++++++++++++ 2 files changed, 118 insertions(+) create mode 100644 build/docs/sccache-dist.rst diff --git a/build/docs/index.rst b/build/docs/index.rst index 14172ed9a850..c6d5ea283b59 100644 --- a/build/docs/index.rst +++ b/build/docs/index.rst @@ -32,6 +32,7 @@ Important Concepts sparse Support for projects building with GN telemetry + sccache-dist test_certificates integrated development environment (IDE) diff --git a/build/docs/sccache-dist.rst b/build/docs/sccache-dist.rst new file mode 100644 index 000000000000..e36bffcd6ff3 --- /dev/null +++ b/build/docs/sccache-dist.rst @@ -0,0 +1,117 @@ +.. _sccache_dist: + +================================== +Distributed sccache (sccache-dist) +================================== + +`sccache `_ is a ccache-like tool written in +rust. + +Distributed sccache (also referred to as sccache-dist) is being rolled out to +Mozilla offices as a replacement for icecc. The steps for setting up your +machine as an sccache-dist server as well as distributing your build to servers +in your office are detailed below. + +In addition to improved security properties, distributed sccache offers +distribution and caching of rust compilation, so it should be an improvement +above and beyond what we see with icecc. Build servers run on linux and +distributing builds is currently supported from macOS and linux machines. +Distribution from Windows is supported in principle but hasn't seen sufficient +testing. + +Steps for distributing a build as an sccache-dist client +======================================================== + +Start by following the instructions at https://github.com/mozilla/sccache/blob/master/docs/DistributedQuickstart.md#configure-a-client +to configure your sccache distributed client. Ignore the note about custom +toolchains if you're distributing compilation from linux. +sccache 0.2.10 or above is recommended, and the auth section of your config +must read:: + + [dist.auth] + type = "mozilla" + +* The scheduler url to use is: ``https://sccache1.corpdmz..mozilla.com``, + where is, for instance, sfo1. A complete list of office short names + to be used can be found `here `_ + +* If you're compiling from a macOS client, there are a handful of additional + considerations detailed here: + https://github.com/mozilla/sccache/blob/master/docs/DistributedQuickstart.md#considerations-when-distributing-from-macos. + In particular, custom toolchains will need to be specified. + Run ``./mach bootstrap`` to download prebuilt toolchains and place them in + ``~/.mozbuild/clang-dist-toolchain.tar.xz`` and + ``~/.mozbuild/rustc-dist-toolchain.tar.xz``. + +* Add the following to your mozconfig:: + + ac_add_options CCACHE=/path/to/sccache + +* When attempting to get your client running, the output of ``sccache -s`` should + be consulted to confirm compilations are being distributed. To receive helpful + logging from the local daemon in case they aren't, run + ``SCCACHE_NO_DAEMON=1 RUST_LOG=sccache=trace path/to/sccache --start-server`` + in a terminal window separate from your build prior to building. + +* Run ``./mach build -j`` with an appropriately large ````. + ``sccache --dist-status`` should provide the number of cores available to you + (or a message if you're not connected). In the future this will be integrated + with the build system to automatically select an appropriate value. + +This should be enough to distribute your build and replace your use of icecc. +Bear in mind there may be a few speedbumps, and please ensure your version of +sccache is current before investigating further. Please see the common questions +section below and ask for help if anything is preventing you from using it over +email (dev-builds), on slack in #sccache, or in #build on irc. + +Steps for setting up a server +============================= + +Build servers must run linux and use bubblewrap 3.0+ for sandboxing of compile +processes. This requires a kernel 4.6 or greater, so Ubuntu 18+, RHEL 8, or +similar. + +* Acquire a recent build of sccache. ``./mach bootstrap`` or + ``./mach artifact toolchain --from-build linux64-sccache`` will provide this. + ``cargo install`` may also be used, but ensure the version for the ``sccache`` + and ``sccache-dist`` binaries you end up using is 0.2.10 or above. + Alternatively, the source is available at https://github.com/mozilla/sccache + and should be built with the ``dist-server`` feature selected if building from + there. + +* Collect the IP of your builder and request assignment of a static IP in a bug + filed in + `NetOps :: Other `_ + cc sccache-admins@mozilla.com on this bug. + +* File a bug in + `Infrastructure :: Other `_ + asking for an sccache builder auth token to be generated for your builder. + The bug should include your name, office, where in the office the builder is + located (desk, closet, etc - so IT can physically find it if needed), the IP + address, and a secure method to reach you (gpg, keybase.io, etc). An auth + token will be generated and delivered to you. + +* The instructions at https://github.com/mozilla/sccache/blob/master/docs/DistributedQuickstart.md#configure-a-build-server + should contain everything else required to configure and run the server. + + +Common questions/considerations +=============================== + +* My build is still slow: scache-dist can only do so much with parts of the + build that aren't able to be parallelized. To start debugging a slow build, + ensure the "Successful distributed compilations" line in the output of + ``sccache -s`` dominates other counts. For a full build, at least a 2-3x + improvement should be observed. + +* My build output is incomprehensible due to a flood of warnings: clang will + treat some warnings differently when its fed preprocessed code in a separate + invocation (preprocessing occurs locally with sccache-dist). See the + following note about disabling problematic warnings: + https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Using_Icecream#I_get_build_failures_due_to_-Werror_when_building_remotely_but_not_when_building_locally + +* My build fails with a message about incompatible versions of rustc between + dependent crates: if you're using a custom toolchain check that the version + of rustc in your ``rustc-dist-toolchain.tar.xz`` is the same as the version + you're running locally. From 5361128d9549521f7b1aac69d6c8e44a525ed4cd Mon Sep 17 00:00:00 2001 From: Toshihito Kikuchi Date: Tue, 27 Aug 2019 22:19:38 +0000 Subject: [PATCH 20/41] Bug 1575352 - Remove dynamic linking of urlmon for MinGW. r=aklotz 4eca0f08c43b73dc1dd908fad58bdfd7f6973119 introduced dynamic linking of urlmon for MinGW as a workaround. Now that MinGW was updated, we can remove that workaround. Differential Revision: https://phabricator.services.mozilla.com/D43281 --HG-- extra : moz-landing-system : lando --- widget/windows/UrlmonHeaderOnlyUtils.h | 17 +---------------- widget/windows/tests/TestUriValidation.cpp | 17 +---------------- 2 files changed, 2 insertions(+), 32 deletions(-) diff --git a/widget/windows/UrlmonHeaderOnlyUtils.h b/widget/windows/UrlmonHeaderOnlyUtils.h index d348641e9a49..fe3db5ee8e64 100644 --- a/widget/windows/UrlmonHeaderOnlyUtils.h +++ b/widget/windows/UrlmonHeaderOnlyUtils.h @@ -39,21 +39,6 @@ inline LauncherResult<_bstr_t> UrlmonValidateUri(const wchar_t* aUri) { return LAUNCHER_ERROR_FROM_RESULT(pidlResult); } -#ifndef __MINGW32__ - const auto createUri = CreateUri; -#else - HMODULE urlmonDll = GetModuleHandleW(L"urlmon"); - if (!urlmonDll) { - return LAUNCHER_ERROR_FROM_LAST(); - } - - const auto createUri = reinterpret_cast( - GetProcAddress(urlmonDll, "CreateUri")); - if (!createUri) { - return LAUNCHER_ERROR_FROM_LAST(); - } -#endif - // The value of |flags| is the same value as used in ieframe!_EnsureIUri in // Win7, which is called behind SHParseDisplayName. In Win10, on the other // hand, an flag 0x03000000 is also passed to CreateUri, but we don't @@ -63,7 +48,7 @@ inline LauncherResult<_bstr_t> UrlmonValidateUri(const wchar_t* aUri) { Uri_CREATE_CRACK_UNKNOWN_SCHEMES | Uri_CREATE_PRE_PROCESS_HTML_URI | Uri_CREATE_IE_SETTINGS; RefPtr uri; - HRESULT hr = createUri(aUri, flags, 0, getter_AddRefs(uri)); + HRESULT hr = CreateUri(aUri, flags, 0, getter_AddRefs(uri)); if (FAILED(hr)) { return LAUNCHER_ERROR_FROM_HRESULT(hr); } diff --git a/widget/windows/tests/TestUriValidation.cpp b/widget/windows/tests/TestUriValidation.cpp index 11e5a3eb7c35..dacdab15eaf2 100644 --- a/widget/windows/tests/TestUriValidation.cpp +++ b/widget/windows/tests/TestUriValidation.cpp @@ -50,27 +50,12 @@ static LauncherResult<_bstr_t> ShellValidateUri(const wchar_t* aUri) { } static LauncherResult<_bstr_t> GetFragment(const wchar_t* aUri) { -#ifndef __MINGW32__ - const auto createUri = CreateUri; -#else - HMODULE urlmonDll = GetModuleHandleW(L"urlmon"); - if (!urlmonDll) { - return LAUNCHER_ERROR_FROM_LAST(); - } - - const auto createUri = reinterpret_cast( - GetProcAddress(urlmonDll, "CreateUri")); - if (!createUri) { - return LAUNCHER_ERROR_FROM_LAST(); - } -#endif - constexpr DWORD flags = Uri_CREATE_NO_DECODE_EXTRA_INFO | Uri_CREATE_CANONICALIZE | Uri_CREATE_CRACK_UNKNOWN_SCHEMES | Uri_CREATE_PRE_PROCESS_HTML_URI | Uri_CREATE_IE_SETTINGS; RefPtr uri; - HRESULT hr = createUri(aUri, flags, 0, getter_AddRefs(uri)); + HRESULT hr = CreateUri(aUri, flags, 0, getter_AddRefs(uri)); if (FAILED(hr)) { return LAUNCHER_ERROR_FROM_HRESULT(hr); } From 97a1fcf5a7e2f65980463e11c065015f4004b6a8 Mon Sep 17 00:00:00 2001 From: Nick Alexander Date: Tue, 27 Aug 2019 22:21:42 +0000 Subject: [PATCH 21/41] Bug 1576707 - Allow "absolute" fetch artifacts to ignore `artifact_prefix`. r=aki The way that `artifact-prefix` works is that it applies to the _consuming_ task's fetches... meaning that changing the prefix requires working out the new paths for every fetch in the consuming task. This allows to "opt-out" of the artifact prefix mechanism for individual fetches. Differential Revision: https://phabricator.services.mozilla.com/D43535 --HG-- extra : moz-landing-system : lando --- taskcluster/taskgraph/transforms/job/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/taskcluster/taskgraph/transforms/job/__init__.py b/taskcluster/taskgraph/transforms/job/__init__.py index 64b8e63b3918..088ec7f1eb7c 100644 --- a/taskcluster/taskgraph/transforms/job/__init__.py +++ b/taskcluster/taskgraph/transforms/job/__init__.py @@ -238,7 +238,8 @@ def use_fetches(config, jobs): extract = artifact.get('extract', True) fetch = { - 'artifact': '{prefix}/{path}'.format(prefix=prefix, path=path), + 'artifact': '{prefix}/{path}'.format(prefix=prefix, path=path) + if not path.startswith('/') else path[1:], 'task': '<{dep}>'.format(dep=kind), 'extract': extract, } From 211b3751d7f64aece6ec5ae6913b331ccf9d7d7f Mon Sep 17 00:00:00 2001 From: Makoto Kato Date: Tue, 27 Aug 2019 13:00:38 +0000 Subject: [PATCH 22/41] Bug 1511154 - Add junit test for location.hash setter. r=geckoview-reviewers,droeh By bug 1563538, we don't call `onLoadRequest` when setting location.hash. So this issue is fixed by it. So we should add geckoview-junit test for this. Differential Revision: https://phabricator.services.mozilla.com/D43574 --HG-- extra : moz-landing-system : lando --- .../geckoview/test/NavigationDelegateTest.kt | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/NavigationDelegateTest.kt b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/NavigationDelegateTest.kt index 8caaf8f7443f..d1abe12ff2be 100644 --- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/NavigationDelegateTest.kt +++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/NavigationDelegateTest.kt @@ -1240,4 +1240,41 @@ class NavigationDelegateTest : BaseSessionTest() { mainSession.loadTestPath(HELLO_HTML_PATH) sessionRule.waitForPageStop() } + + @Test fun setLocationHash() { + sessionRule.session.loadUri("$TEST_ENDPOINT$HELLO_HTML_PATH") + sessionRule.waitForPageStop() + + sessionRule.session.evaluateJS("location.hash = 'test1';") + + sessionRule.session.waitUntilCalled(object : Callbacks.NavigationDelegate { + @AssertCalled(count = 0) + override fun onLoadRequest(session: GeckoSession, + request: LoadRequest): + GeckoResult? { + return null + } + + @AssertCalled(count = 1) + override fun onLocationChange(session: GeckoSession, url: String?) { + assertThat("URI should match", url, endsWith("#test1")) + } + }) + + sessionRule.session.evaluateJS("location.hash = 'test2';") + + sessionRule.session.waitUntilCalled(object : Callbacks.NavigationDelegate { + @AssertCalled(count = 0) + override fun onLoadRequest(session: GeckoSession, + request: LoadRequest): + GeckoResult? { + return null + } + + @AssertCalled(count = 1) + override fun onLocationChange(session: GeckoSession, url: String?) { + assertThat("URI should match", url, endsWith("#test2")) + } + }) + } } From c0d2f698811644efa529bbc41fe8c3210c4932cb Mon Sep 17 00:00:00 2001 From: Olli Pettay Date: Tue, 27 Aug 2019 20:40:33 +0000 Subject: [PATCH 23/41] Bug 1576783, make browser_doorhanger_bc_downloadAutoFailures_bgWin.js more resilient to when popupshown event fires, r=rstrong popupshown may get fired while waiting for closeWindow or promiseFocus to be executed. Other uses of runDoorhangerUpdateTest seem to be simpler and should be fine without popupShown parameter. Differential Revision: https://phabricator.services.mozilla.com/D43672 --HG-- extra : moz-landing-system : lando --- ...ser_doorhanger_bc_downloadAutoFailures_bgWin.js | 14 +++++++++++++- toolkit/mozapps/update/tests/browser/head.js | 10 ++++++---- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/toolkit/mozapps/update/tests/browser/browser_doorhanger_bc_downloadAutoFailures_bgWin.js b/toolkit/mozapps/update/tests/browser/browser_doorhanger_bc_downloadAutoFailures_bgWin.js index 865e56c6228f..d8cec82ae58e 100644 --- a/toolkit/mozapps/update/tests/browser/browser_doorhanger_bc_downloadAutoFailures_bgWin.js +++ b/toolkit/mozapps/update/tests/browser/browser_doorhanger_bc_downloadAutoFailures_bgWin.js @@ -43,8 +43,20 @@ add_task(async function doorhanger_bc_downloadAutoFailures_bgWin() { buttonEl.click(); if (destroyWindow) { + // The next popup may be shown during closeWindow or promiseFocus + // calls. + let waitForPopupShown = new Promise(resolve => { + window.addEventListener( + "popupshown", + () => { + executeSoon(resolve); + }, + { once: true } + ); + }); await BrowserTestUtils.closeWindow(extraWindow); await SimpleTest.promiseFocus(window); + await waitForPopupShown; } }; } @@ -57,7 +69,7 @@ add_task(async function doorhanger_bc_downloadAutoFailures_bgWin() { let extraWindow = await BrowserTestUtils.openNewBrowserWindow(); await SimpleTest.promiseFocus(extraWindow); - let params = { checkAttempts: 1, queryString: "&badURL=1" }; + let params = { checkAttempts: 1, queryString: "&badURL=1", popupShown: true }; await runDoorhangerUpdateTest(params, [ getBackgroundWindowHandler(false), getBackgroundWindowHandler(true), diff --git a/toolkit/mozapps/update/tests/browser/head.js b/toolkit/mozapps/update/tests/browser/head.js index a6262e448c9c..cf761584d154 100644 --- a/toolkit/mozapps/update/tests/browser/head.js +++ b/toolkit/mozapps/update/tests/browser/head.js @@ -569,10 +569,12 @@ function runDoorhangerUpdateTest(params, steps) { const { notificationId, button, checkActiveUpdate, pageURLs } = step; return (async function() { - await BrowserTestUtils.waitForEvent( - PanelUI.notificationPanel, - "popupshown" - ); + if (!params.popupShown) { + await BrowserTestUtils.waitForEvent( + PanelUI.notificationPanel, + "popupshown" + ); + } const shownNotificationId = AppMenuNotifications.activeNotification.id; is( shownNotificationId, From f3a7c205527625449746a0a14130f4f1c855d7d5 Mon Sep 17 00:00:00 2001 From: lesleynorton Date: Mon, 26 Aug 2019 16:47:54 +0000 Subject: [PATCH 24/41] Bug 1550122: Show errors in aboutLogins UI. r=MattN,fluent-reviewers,ntim,flod Differential Revision: https://phabricator.services.mozilla.com/D42705 --HG-- extra : moz-landing-system : lando --- browser/components/BrowserGlue.jsm | 1 + .../aboutlogins/AboutLoginsChild.jsm | 3 + .../aboutlogins/AboutLoginsParent.jsm | 22 ++- .../aboutlogins/content/aboutLogins.html | 3 + .../aboutlogins/content/aboutLogins.js | 4 + .../content/components/login-item.css | 20 +++ .../content/components/login-item.js | 29 +++- .../aboutlogins/tests/browser/browser.ini | 1 + .../tests/browser/browser_loginItemErrors.js | 126 ++++++++++++++++++ browser/locales/en-US/browser/aboutLogins.ftl | 11 ++ 10 files changed, 216 insertions(+), 4 deletions(-) create mode 100644 browser/components/aboutlogins/tests/browser/browser_loginItemErrors.js diff --git a/browser/components/BrowserGlue.jsm b/browser/components/BrowserGlue.jsm index 190231293335..76c9cbcda4d7 100644 --- a/browser/components/BrowserGlue.jsm +++ b/browser/components/BrowserGlue.jsm @@ -162,6 +162,7 @@ let LEGACY_ACTORS = { "AboutLogins:LoginRemoved", "AboutLogins:MasterPasswordResponse", "AboutLogins:SendFavicons", + "AboutLogins:ShowLoginItemError", "AboutLogins:SyncState", "AboutLogins:UpdateBreaches", ], diff --git a/browser/components/aboutlogins/AboutLoginsChild.jsm b/browser/components/aboutlogins/AboutLoginsChild.jsm index ffe5937785f7..d2937daa27da 100644 --- a/browser/components/aboutlogins/AboutLoginsChild.jsm +++ b/browser/components/aboutlogins/AboutLoginsChild.jsm @@ -191,6 +191,9 @@ class AboutLoginsChild extends ActorChild { case "AboutLogins:SendFavicons": this.sendToContent("SendFavicons", message.data); break; + case "AboutLogins:ShowLoginItemError": + this.sendToContent("ShowLoginItemError", message.data); + break; case "AboutLogins:SyncState": this.sendToContent("SyncState", message.data); break; diff --git a/browser/components/aboutlogins/AboutLoginsParent.jsm b/browser/components/aboutlogins/AboutLoginsParent.jsm index 97ee2f7fc522..e437de88cb9e 100644 --- a/browser/components/aboutlogins/AboutLoginsParent.jsm +++ b/browser/components/aboutlogins/AboutLoginsParent.jsm @@ -121,7 +121,11 @@ var AboutLoginsParent = { usernameField: "", passwordField: "", }); - Services.logins.addLogin(LoginHelper.vanillaObjectToLogin(newLogin)); + try { + Services.logins.addLogin(LoginHelper.vanillaObjectToLogin(newLogin)); + } catch (error) { + this.handleLoginStorageErrors(newLogin, error, message); + } break; } case "AboutLogins:DeleteLogin": { @@ -489,13 +493,25 @@ var AboutLoginsParent = { if (loginUpdates.hasOwnProperty("password")) { modifiedLogin.password = loginUpdates.password; } - - Services.logins.modifyLogin(logins[0], modifiedLogin); + try { + Services.logins.modifyLogin(logins[0], modifiedLogin); + } catch (error) { + this.handleLoginStorageErrors(modifiedLogin, error, message); + } break; } } }, + handleLoginStorageErrors(login, error, message) { + const messageManager = message.target.messageManager; + const errorMessage = error.message; + messageManager.sendAsyncMessage("AboutLogins:ShowLoginItemError", { + login: augmentVanillaLoginObject(login), + errorMessage, + }); + }, + async observe(subject, topic, type) { if (!ChromeUtils.nondeterministicGetWeakSetKeys(this._subscribers).length) { Services.obs.removeObserver(this, "passwordmgr-crypto-login"); diff --git a/browser/components/aboutlogins/content/aboutLogins.html b/browser/components/aboutlogins/content/aboutLogins.html index 407881c191b6..82ed4e07bd03 100644 --- a/browser/components/aboutlogins/content/aboutLogins.html +++ b/browser/components/aboutlogins/content/aboutLogins.html @@ -137,6 +137,9 @@ +
+ +
diff --git a/browser/components/aboutlogins/content/aboutLogins.js b/browser/components/aboutlogins/content/aboutLogins.js index 7e635f518d56..597de806a519 100644 --- a/browser/components/aboutlogins/content/aboutLogins.js +++ b/browser/components/aboutlogins/content/aboutLogins.js @@ -65,6 +65,10 @@ window.addEventListener("AboutLoginsChromeToContent", event => { gElements.loginList.addFavicons(event.detail.value); break; } + case "ShowLoginItemError": { + gElements.loginItem.showLoginItemError(event.detail.value); + break; + } case "SyncState": { gElements.fxAccountsButton.updateState(event.detail.value); gElements.loginFooter.hidden = event.detail.value.hideMobileFooter; diff --git a/browser/components/aboutlogins/content/components/login-item.css b/browser/components/aboutlogins/content/components/login-item.css index a7362a583e97..c11920e9be80 100644 --- a/browser/components/aboutlogins/content/components/login-item.css +++ b/browser/components/aboutlogins/content/components/login-item.css @@ -291,6 +291,26 @@ a.breach-alert-link { background-color: transparent; } +.error-message { + color: #fff; + background-color: var(--red-60); + border: 1px solid transparent; + padding-block: 6px; + display: inline-block; + padding-inline-start: 32px; + padding-inline-end: 16px; + background-image: url("chrome://global/skin/icons/warning.svg"); + background-repeat: no-repeat; + background-position: left 10px center; + -moz-context-properties: fill; + fill: currentColor; + margin-bottom: 38px; +} + +.error-message:dir(rtl) { + background-position-x: right 10px; +} + @supports -moz-bool-pref("browser.in-content.dark-mode") { @media (prefers-color-scheme: dark) { :host { diff --git a/browser/components/aboutlogins/content/components/login-item.js b/browser/components/aboutlogins/content/components/login-item.js index 4af595af1f0e..47146df33e15 100644 --- a/browser/components/aboutlogins/content/components/login-item.js +++ b/browser/components/aboutlogins/content/components/login-item.js @@ -41,6 +41,7 @@ export default class LoginItem extends HTMLElement { ); this._deleteButton = this.shadowRoot.querySelector(".delete-button"); this._editButton = this.shadowRoot.querySelector(".edit-button"); + this._errorMessage = this.shadowRoot.querySelector(".error-message"); this._form = this.shadowRoot.querySelector("form"); this._originInput = this.shadowRoot.querySelector("input[name='origin']"); this._usernameInput = this.shadowRoot.querySelector( @@ -87,7 +88,9 @@ export default class LoginItem extends HTMLElement { } async render() { - this._breachAlert.hidden = true; + [this._errorMessage, this._breachAlert].forEach(el => { + el.hidden = true; + }); if (this._breachesMap && this._breachesMap.has(this._login.guid)) { const breachDetails = this._breachesMap.get(this._login.guid); const breachAlertLink = this._breachAlert.querySelector( @@ -150,6 +153,30 @@ export default class LoginItem extends HTMLElement { ); } + showLoginItemError(error) { + const errorMessageText = this._errorMessage.querySelector( + ".error-message-text" + ); + if (!error.errorMessage) { + return; + } + if (error.errorMessage.includes("This login already exists")) { + document.l10n.setAttributes( + errorMessageText, + "about-logins-error-message-duplicate-login", + { + loginTitle: error.login.title, + } + ); + } else { + document.l10n.setAttributes( + errorMessageText, + "about-logins-error-message-default" + ); + } + this._errorMessage.hidden = false; + } + async handleEvent(event) { switch (event.type) { case "AboutLoginsInitialLoginSelected": { diff --git a/browser/components/aboutlogins/tests/browser/browser.ini b/browser/components/aboutlogins/tests/browser/browser.ini index 6237f04e4795..61f9c7d41d37 100644 --- a/browser/components/aboutlogins/tests/browser/browser.ini +++ b/browser/components/aboutlogins/tests/browser/browser.ini @@ -17,6 +17,7 @@ skip-if = asan || debug || verify # bug 1574023 [browser_deleteLogin.js] [browser_dismissFooter.js] [browser_fxAccounts.js] +[browser_loginItemErrors.js] [browser_loginListChanges.js] [browser_masterPassword.js] skip-if = (os == 'linux') # bug 1569789 diff --git a/browser/components/aboutlogins/tests/browser/browser_loginItemErrors.js b/browser/components/aboutlogins/tests/browser/browser_loginItemErrors.js new file mode 100644 index 000000000000..ed66eed663bb --- /dev/null +++ b/browser/components/aboutlogins/tests/browser/browser_loginItemErrors.js @@ -0,0 +1,126 @@ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ */ + +add_task(async function setup() { + await BrowserTestUtils.openNewForegroundTab({ + gBrowser, + url: "about:logins", + }); + registerCleanupFunction(() => { + BrowserTestUtils.removeTab(gBrowser.selectedTab); + Services.logins.removeAllLogins(); + }); +}); + +add_task(async function test_showLoginItemErrors() { + const browser = gBrowser.selectedBrowser; + let LOGIN_TO_UPDATE = new nsLoginInfo( + "https://example.com/", + "https://example.com/", + null, + "user2", + "pass2", + "username", + "password" + ); + LOGIN_TO_UPDATE = Services.logins.addLogin(LOGIN_TO_UPDATE); + + await ContentTask.spawn( + browser, + LoginHelper.loginToVanillaObject(LOGIN_TO_UPDATE), + async loginToUpdate => { + const loginItem = Cu.waiveXrays( + content.document.querySelector("login-item") + ); + const loginItemErrorMessage = Cu.waiveXrays( + loginItem.shadowRoot.querySelector(".error-message") + ); + const loginList = Cu.waiveXrays( + content.document.querySelector("login-list") + ); + + const createButton = loginList._createLoginButton; + createButton.click(); + + const loginUpdates = { + origin: "https://example.com/", + password: "my1GoodPassword", + username: "user1", + }; + + const event = Cu.cloneInto( + { + bubbles: true, + detail: loginUpdates, + }, + content + ); + + content.dispatchEvent( + // adds first lgoin + new content.CustomEvent("AboutLoginsCreateLogin", event) + ); + + ok( + loginItemErrorMessage.hidden, + "An error message should not be displayed after adding a new login." + ); + + content.dispatchEvent( + // adds a duplicate of the first login + new content.CustomEvent("AboutLoginsCreateLogin", event) + ); + + const loginItemErrorMessageVisible = await ContentTaskUtils.waitForCondition( + () => { + return !loginItemErrorMessage.hidden; + }, + "Waiting for error message to be shown after attempting to create a duplicate login." + ); + ok( + loginItemErrorMessageVisible, + "An error message should be shown after user attempts to add a login that already exists." + ); + + const loginItemErrorMessageText = loginItemErrorMessage.querySelector( + "span" + ); + ok( + loginItemErrorMessageText.dataset.l10nId === + "about-logins-error-message-duplicate-login", + "The correct error message is displayed." + ); + + let loginListItem = Cu.waiveXrays( + loginList.shadowRoot.querySelector( + `.login-list-item[data-guid='${loginToUpdate.guid}']` + ) + ); + loginListItem.click(); + + ok( + loginItemErrorMessage.hidden, + "The error message should no longer be visible." + ); + + const editButton = loginItem.shadowRoot.querySelector(".edit-button"); + editButton.click(); + + content.dispatchEvent( + // attempt to update LOGIN_TO_UPDATE to a username/origin combination that already exists. + new content.CustomEvent("AboutLoginsUpdateLogin", event) + ); + + const loginAlreadyExistsErrorShownAfterUpdate = ContentTaskUtils.waitForCondition( + () => { + return !loginItemErrorMessage.hidden; + }, + "Waiting for error message to show after updating login to existing login." + ); + ok( + loginAlreadyExistsErrorShownAfterUpdate, + "An error message should be shown after updating a login to a username/origin combination that already exists." + ); + } + ); +}); diff --git a/browser/locales/en-US/browser/aboutLogins.ftl b/browser/locales/en-US/browser/aboutLogins.ftl index ddb3b7997760..e1f08e35bba5 100644 --- a/browser/locales/en-US/browser/aboutLogins.ftl +++ b/browser/locales/en-US/browser/aboutLogins.ftl @@ -140,3 +140,14 @@ breach-alert-text = Passwords were leaked or stolen from this website since you breach-alert-link = Learn more about this breach. breach-alert-dismiss = .title = Close this alert + +## Error Messages + +# This is an error message that appears when a user attempts to save +# a new login that is identical to an existing saved login. +# Variables: +# $loginTitle (String) - The title of the website associated with the login. +about-logins-error-message-duplicate-login = An entry for { $loginTitle } with that username already exists. + +# This is a generic error message. +about-logins-error-message-default = An error occurred while trying to save this password. From 3afd91eb538b5c2b6d28e5299010acc3654a70ea Mon Sep 17 00:00:00 2001 From: Ting-Yu Lin Date: Tue, 27 Aug 2019 22:36:42 +0000 Subject: [PATCH 25/41] Bug 1573329 - Add web-platform reftests for multicol breaking. r=dbaron These tests are fixed by bug 1575106, Differential Revision: https://phabricator.services.mozilla.com/D43676 --HG-- extra : moz-landing-system : lando --- .../multicol-breaking-005.html.ini | 2 + ...ulticol-breaking-nobackground-005.html.ini | 2 + .../multicol-breaking-005-ref.html | 100 ++++++++++++++++++ .../css-multicol/multicol-breaking-005.html | 70 ++++++++++++ ...ulticol-breaking-nobackground-005-ref.html | 86 +++++++++++++++ .../multicol-breaking-nobackground-005.html | 68 ++++++++++++ 6 files changed, 328 insertions(+) create mode 100644 testing/web-platform/meta/css/css-multicol/multicol-breaking-005.html.ini create mode 100644 testing/web-platform/meta/css/css-multicol/multicol-breaking-nobackground-005.html.ini create mode 100644 testing/web-platform/tests/css/css-multicol/multicol-breaking-005-ref.html create mode 100644 testing/web-platform/tests/css/css-multicol/multicol-breaking-005.html create mode 100644 testing/web-platform/tests/css/css-multicol/multicol-breaking-nobackground-005-ref.html create mode 100644 testing/web-platform/tests/css/css-multicol/multicol-breaking-nobackground-005.html diff --git a/testing/web-platform/meta/css/css-multicol/multicol-breaking-005.html.ini b/testing/web-platform/meta/css/css-multicol/multicol-breaking-005.html.ini new file mode 100644 index 000000000000..ce943853aa78 --- /dev/null +++ b/testing/web-platform/meta/css/css-multicol/multicol-breaking-005.html.ini @@ -0,0 +1,2 @@ +[multicol-breaking-005.html] + prefs: [layout.css.column-span.enabled:true] diff --git a/testing/web-platform/meta/css/css-multicol/multicol-breaking-nobackground-005.html.ini b/testing/web-platform/meta/css/css-multicol/multicol-breaking-nobackground-005.html.ini new file mode 100644 index 000000000000..e6a8c5c70ea1 --- /dev/null +++ b/testing/web-platform/meta/css/css-multicol/multicol-breaking-nobackground-005.html.ini @@ -0,0 +1,2 @@ +[multicol-breaking-nobackground-005.html] + prefs: [layout.css.column-span.enabled:true] diff --git a/testing/web-platform/tests/css/css-multicol/multicol-breaking-005-ref.html b/testing/web-platform/tests/css/css-multicol/multicol-breaking-005-ref.html new file mode 100644 index 000000000000..a22fda57c0b2 --- /dev/null +++ b/testing/web-platform/tests/css/css-multicol/multicol-breaking-005-ref.html @@ -0,0 +1,100 @@ + +CSS Test Reference: breaking of a multicolumn + + + + + + +
+
+
+
+ AAAAA
+ BBBBB
+ CCCCC
+ DDDDD
+ EEEEE +
+
+ FFFFF
+ GGGGG
+ HHHHH
+ IIIII
+ JJJJJ +
+
+
+ KKKKK
+ LLLLL
+ MMMMM
+ NNNNN
+ OOOOO +
+
+ PPPPP
+ QQQQQ
+ RRRRR
+ SSSSS
+ TTTTT +
+
+
+ UUUUU
+ VVVVV
+ WWWWW
+ XXXXX
+ YYYYY +
+
+ ZZZZZ
+ aaaaa
+ bbbbb
+ ccccc
+ ddddd +
+
diff --git a/testing/web-platform/tests/css/css-multicol/multicol-breaking-005.html b/testing/web-platform/tests/css/css-multicol/multicol-breaking-005.html new file mode 100644 index 000000000000..18a1b78fc9a2 --- /dev/null +++ b/testing/web-platform/tests/css/css-multicol/multicol-breaking-005.html @@ -0,0 +1,70 @@ + +CSS Test: breaking of a multicolumn + + + + + + + + + + + +
+
+ AAAAA
+ BBBBB
+ CCCCC
+ DDDDD
+ EEEEE
+ FFFFF
+ GGGGG
+ HHHHH
+ IIIII
+ JJJJJ
+ KKKKK
+ LLLLL
+ MMMMM
+ NNNNN
+ OOOOO
+ PPPPP
+ QQQQQ
+ RRRRR
+ SSSSS
+ TTTTT
+ UUUUU
+ VVVVV
+ WWWWW
+ XXXXX
+ YYYYY
+ ZZZZZ
+ aaaaa
+ bbbbb
+ ccccc
+ ddddd +
+
diff --git a/testing/web-platform/tests/css/css-multicol/multicol-breaking-nobackground-005-ref.html b/testing/web-platform/tests/css/css-multicol/multicol-breaking-nobackground-005-ref.html new file mode 100644 index 000000000000..63296363bf99 --- /dev/null +++ b/testing/web-platform/tests/css/css-multicol/multicol-breaking-nobackground-005-ref.html @@ -0,0 +1,86 @@ + +CSS Test Reference: breaking of a multicolumn + + + + + + +
+
+ AAAAA
+ BBBBB
+ CCCCC
+ DDDDD
+ EEEEE +
+
+ FFFFF
+ GGGGG
+ HHHHH
+ IIIII
+ JJJJJ +
+
+ KKKKK
+ LLLLL
+ MMMMM
+ NNNNN
+ OOOOO +
+
+ PPPPP
+ QQQQQ
+ RRRRR
+ SSSSS
+ TTTTT +
+
+ UUUUU
+ VVVVV
+ WWWWW
+ XXXXX
+ YYYYY +
+
+ ZZZZZ
+ aaaaa
+ bbbbb
+ ccccc
+ ddddd +
+
diff --git a/testing/web-platform/tests/css/css-multicol/multicol-breaking-nobackground-005.html b/testing/web-platform/tests/css/css-multicol/multicol-breaking-nobackground-005.html new file mode 100644 index 000000000000..2065949cc92c --- /dev/null +++ b/testing/web-platform/tests/css/css-multicol/multicol-breaking-nobackground-005.html @@ -0,0 +1,68 @@ + +CSS Test: breaking of a multicolumn + + + + + + + + + + + +
+
+ AAAAA
+ BBBBB
+ CCCCC
+ DDDDD
+ EEEEE
+ FFFFF
+ GGGGG
+ HHHHH
+ IIIII
+ JJJJJ
+ KKKKK
+ LLLLL
+ MMMMM
+ NNNNN
+ OOOOO
+ PPPPP
+ QQQQQ
+ RRRRR
+ SSSSS
+ TTTTT
+ UUUUU
+ VVVVV
+ WWWWW
+ XXXXX
+ YYYYY
+ ZZZZZ
+ aaaaa
+ bbbbb
+ ccccc
+ ddddd +
+
From 71cfbd4b5a0b732f53059911a51d867455975344 Mon Sep 17 00:00:00 2001 From: Toshihito Kikuchi Date: Tue, 27 Aug 2019 22:51:32 +0000 Subject: [PATCH 26/41] Bug 1567219 - Add a metric to collect how many users launch a process with Admin but without UAC. r=aklotz This patch adds a new Scalar metric `os.environment.is_admin_without_uac` that indicates the process is lauched with Admin privileges when UAC is turned off. Differential Revision: https://phabricator.services.mozilla.com/D42047 --HG-- extra : moz-landing-system : lando --- browser/app/winlauncher/moz.build | 1 + toolkit/components/telemetry/Scalars.yaml | 19 ++++++ toolkit/xre/WinTokenUtils.cpp | 72 +++++++++++++++++++++++ toolkit/xre/WinTokenUtils.h | 18 ++++++ toolkit/xre/moz.build | 2 + toolkit/xre/nsAppRunner.cpp | 10 ++++ 6 files changed, 122 insertions(+) create mode 100644 toolkit/xre/WinTokenUtils.cpp create mode 100644 toolkit/xre/WinTokenUtils.h diff --git a/browser/app/winlauncher/moz.build b/browser/app/winlauncher/moz.build index f50cc8f1302c..be2be08e32a5 100644 --- a/browser/app/winlauncher/moz.build +++ b/browser/app/winlauncher/moz.build @@ -60,6 +60,7 @@ TEST_DIRS += [ if CONFIG['MOZ_LAUNCHER_PROCESS']: UNIFIED_SOURCES += [ '/toolkit/xre/LauncherRegistryInfo.cpp', + '/toolkit/xre/WinTokenUtils.cpp', ] for var in ('MOZ_APP_BASENAME', 'MOZ_APP_VENDOR'): DEFINES[var] = '"%s"' % CONFIG[var] diff --git a/toolkit/components/telemetry/Scalars.yaml b/toolkit/components/telemetry/Scalars.yaml index 4530dd594c6e..aae2012b4118 100644 --- a/toolkit/components/telemetry/Scalars.yaml +++ b/toolkit/components/telemetry/Scalars.yaml @@ -682,6 +682,25 @@ sandbox: operating_systems: - "windows" +os.environment: + is_admin_without_uac: + bug_numbers: + - 1567219 + description: > + Indicates that the process is lauched with Admin privileges but without + UAC. + expires: never + kind: boolean + notification_emails: + - tkikuchi@mozilla.com + release_channel_collection: opt-out + products: + - 'firefox' + record_in_processes: + - main + operating_systems: + - "windows" + pictureinpicture: opened_method: bug_numbers: diff --git a/toolkit/xre/WinTokenUtils.cpp b/toolkit/xre/WinTokenUtils.cpp new file mode 100644 index 000000000000..ed45e0c8fac8 --- /dev/null +++ b/toolkit/xre/WinTokenUtils.cpp @@ -0,0 +1,72 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim: set ts=8 sts=2 et sw=2 tw=80: */ +/* 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 https://mozilla.org/MPL/2.0/. */ + +#include "WinTokenUtils.h" +#include "nsWindowsHelpers.h" + +using namespace mozilla; + +// If |aToken| is nullptr, CheckTokenMembership uses the calling thread's +// primary token to check membership for. +static LauncherResult IsMemberOfAdministrators( + const nsAutoHandle& aToken) { + BYTE adminsGroupSid[SECURITY_MAX_SID_SIZE]; + DWORD adminsGroupSidSize = sizeof(adminsGroupSid); + if (!CreateWellKnownSid(WinBuiltinAdministratorsSid, nullptr, adminsGroupSid, + &adminsGroupSidSize)) { + return LAUNCHER_ERROR_FROM_LAST(); + } + + BOOL isMember; + if (!CheckTokenMembership(aToken, adminsGroupSid, &isMember)) { + return LAUNCHER_ERROR_FROM_LAST(); + } + return !!isMember; +} + +static LauncherResult IsUacEnabled() { + DWORD len = sizeof(DWORD); + DWORD value; + LSTATUS status = RegGetValueW( + HKEY_LOCAL_MACHINE, + L"SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Policies\\System", + L"EnableLUA", RRF_RT_DWORD, nullptr, &value, &len); + if (status != ERROR_SUCCESS) { + return LAUNCHER_ERROR_FROM_WIN32(status); + } + + // UAC is disabled only when EnableLUA is 0. + return (value != 0); +} + +namespace mozilla { + +LauncherResult IsAdminWithoutUac() { + // To check whether the process was launched with Administrator priviledges + // or not, we cannot simply check the integrity level of the current process + // because the launcher process spawns the browser process with the medium + // integrity level even though the launcher process is high integrity level. + // We check whether the thread's token contains Administratos SID or not + // instead. + LauncherResult containsAdminGroup = + IsMemberOfAdministrators(nsAutoHandle()); + if (containsAdminGroup.isErr()) { + return LAUNCHER_ERROR_FROM_RESULT(containsAdminGroup); + } + + if (!containsAdminGroup.unwrap()) { + return false; + } + + LauncherResult isUacEnabled = IsUacEnabled(); + if (isUacEnabled.isErr()) { + return LAUNCHER_ERROR_FROM_RESULT(isUacEnabled); + } + + return !isUacEnabled.unwrap(); +} + +} // namespace mozilla diff --git a/toolkit/xre/WinTokenUtils.h b/toolkit/xre/WinTokenUtils.h new file mode 100644 index 000000000000..f582bd0696b2 --- /dev/null +++ b/toolkit/xre/WinTokenUtils.h @@ -0,0 +1,18 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim: set ts=8 sts=2 et sw=2 tw=80: */ +/* 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 https://mozilla.org/MPL/2.0/. */ + +#ifndef mozilla_WinTokenUtils_h +#define mozilla_WinTokenUtils_h + +#include "mozilla/LauncherResult.h" + +namespace mozilla { + +LauncherResult IsAdminWithoutUac(); + +} // namespace mozilla + +#endif // mozilla_WinTokenUtils_h diff --git a/toolkit/xre/moz.build b/toolkit/xre/moz.build index 57ae50a68c0d..f53f7b38f4c0 100644 --- a/toolkit/xre/moz.build +++ b/toolkit/xre/moz.build @@ -50,6 +50,7 @@ if CONFIG['MOZ_WIDGET_TOOLKIT'] == 'windows': 'ModuleVersionInfo_windows.h', 'PolicyChecks.h', 'WinDllServices.h', + 'WinTokenUtils.h', ] UNIFIED_SOURCES += [ '/toolkit/mozapps/update/common/updateutils_win.cpp', @@ -57,6 +58,7 @@ if CONFIG['MOZ_WIDGET_TOOLKIT'] == 'windows': 'ModuleVersionInfo_windows.cpp', 'nsNativeAppSupportWin.cpp', 'WinDllServices.cpp', + 'WinTokenUtils.cpp', ] DEFINES['PROXY_PRINTING'] = 1 LOCAL_INCLUDES += [ diff --git a/toolkit/xre/nsAppRunner.cpp b/toolkit/xre/nsAppRunner.cpp index d7883d9f70a0..80b926d71367 100644 --- a/toolkit/xre/nsAppRunner.cpp +++ b/toolkit/xre/nsAppRunner.cpp @@ -113,6 +113,7 @@ # include "mozilla/WinHeaderOnlyUtils.h" # include "mozilla/mscom/ProcessRuntime.h" # include "mozilla/widget/AudioSession.h" +# include "WinTokenUtils.h" # if defined(MOZ_LAUNCHER_PROCESS) # include "mozilla/LauncherRegistryInfo.h" @@ -4563,6 +4564,15 @@ nsresult XREMain::XRE_mainRun() { CrashReporter::Annotation::ContentSandboxCapabilities, flagsString); #endif /* MOZ_SANDBOX && XP_LINUX */ +#if defined(XP_WIN) + LauncherResult isAdminWithoutUac = IsAdminWithoutUac(); + if (isAdminWithoutUac.isOk()) { + Telemetry::ScalarSet( + Telemetry::ScalarID::OS_ENVIRONMENT_IS_ADMIN_WITHOUT_UAC, + isAdminWithoutUac.unwrap()); + } +#endif /* XP_WIN */ + #if defined(MOZ_SANDBOX) AddSandboxAnnotations(); #endif /* MOZ_SANDBOX */ From 8de5cc2d2f59e90d7244b09f94f391e61afc421d Mon Sep 17 00:00:00 2001 From: Brindusan Cristian Date: Wed, 28 Aug 2019 02:15:58 +0300 Subject: [PATCH 27/41] Backed out changeset 8f134018ed03 (bug 1550122) for bc failures at browser_loginItemErrors.js. CLOSED TREE --- browser/components/BrowserGlue.jsm | 1 - .../aboutlogins/AboutLoginsChild.jsm | 3 - .../aboutlogins/AboutLoginsParent.jsm | 22 +-- .../aboutlogins/content/aboutLogins.html | 3 - .../aboutlogins/content/aboutLogins.js | 4 - .../content/components/login-item.css | 20 --- .../content/components/login-item.js | 29 +--- .../aboutlogins/tests/browser/browser.ini | 1 - .../tests/browser/browser_loginItemErrors.js | 126 ------------------ browser/locales/en-US/browser/aboutLogins.ftl | 11 -- 10 files changed, 4 insertions(+), 216 deletions(-) delete mode 100644 browser/components/aboutlogins/tests/browser/browser_loginItemErrors.js diff --git a/browser/components/BrowserGlue.jsm b/browser/components/BrowserGlue.jsm index 76c9cbcda4d7..190231293335 100644 --- a/browser/components/BrowserGlue.jsm +++ b/browser/components/BrowserGlue.jsm @@ -162,7 +162,6 @@ let LEGACY_ACTORS = { "AboutLogins:LoginRemoved", "AboutLogins:MasterPasswordResponse", "AboutLogins:SendFavicons", - "AboutLogins:ShowLoginItemError", "AboutLogins:SyncState", "AboutLogins:UpdateBreaches", ], diff --git a/browser/components/aboutlogins/AboutLoginsChild.jsm b/browser/components/aboutlogins/AboutLoginsChild.jsm index d2937daa27da..ffe5937785f7 100644 --- a/browser/components/aboutlogins/AboutLoginsChild.jsm +++ b/browser/components/aboutlogins/AboutLoginsChild.jsm @@ -191,9 +191,6 @@ class AboutLoginsChild extends ActorChild { case "AboutLogins:SendFavicons": this.sendToContent("SendFavicons", message.data); break; - case "AboutLogins:ShowLoginItemError": - this.sendToContent("ShowLoginItemError", message.data); - break; case "AboutLogins:SyncState": this.sendToContent("SyncState", message.data); break; diff --git a/browser/components/aboutlogins/AboutLoginsParent.jsm b/browser/components/aboutlogins/AboutLoginsParent.jsm index e437de88cb9e..97ee2f7fc522 100644 --- a/browser/components/aboutlogins/AboutLoginsParent.jsm +++ b/browser/components/aboutlogins/AboutLoginsParent.jsm @@ -121,11 +121,7 @@ var AboutLoginsParent = { usernameField: "", passwordField: "", }); - try { - Services.logins.addLogin(LoginHelper.vanillaObjectToLogin(newLogin)); - } catch (error) { - this.handleLoginStorageErrors(newLogin, error, message); - } + Services.logins.addLogin(LoginHelper.vanillaObjectToLogin(newLogin)); break; } case "AboutLogins:DeleteLogin": { @@ -493,25 +489,13 @@ var AboutLoginsParent = { if (loginUpdates.hasOwnProperty("password")) { modifiedLogin.password = loginUpdates.password; } - try { - Services.logins.modifyLogin(logins[0], modifiedLogin); - } catch (error) { - this.handleLoginStorageErrors(modifiedLogin, error, message); - } + + Services.logins.modifyLogin(logins[0], modifiedLogin); break; } } }, - handleLoginStorageErrors(login, error, message) { - const messageManager = message.target.messageManager; - const errorMessage = error.message; - messageManager.sendAsyncMessage("AboutLogins:ShowLoginItemError", { - login: augmentVanillaLoginObject(login), - errorMessage, - }); - }, - async observe(subject, topic, type) { if (!ChromeUtils.nondeterministicGetWeakSetKeys(this._subscribers).length) { Services.obs.removeObserver(this, "passwordmgr-crypto-login"); diff --git a/browser/components/aboutlogins/content/aboutLogins.html b/browser/components/aboutlogins/content/aboutLogins.html index 82ed4e07bd03..407881c191b6 100644 --- a/browser/components/aboutlogins/content/aboutLogins.html +++ b/browser/components/aboutlogins/content/aboutLogins.html @@ -137,9 +137,6 @@ -
- -
diff --git a/browser/components/aboutlogins/content/aboutLogins.js b/browser/components/aboutlogins/content/aboutLogins.js index 597de806a519..7e635f518d56 100644 --- a/browser/components/aboutlogins/content/aboutLogins.js +++ b/browser/components/aboutlogins/content/aboutLogins.js @@ -65,10 +65,6 @@ window.addEventListener("AboutLoginsChromeToContent", event => { gElements.loginList.addFavicons(event.detail.value); break; } - case "ShowLoginItemError": { - gElements.loginItem.showLoginItemError(event.detail.value); - break; - } case "SyncState": { gElements.fxAccountsButton.updateState(event.detail.value); gElements.loginFooter.hidden = event.detail.value.hideMobileFooter; diff --git a/browser/components/aboutlogins/content/components/login-item.css b/browser/components/aboutlogins/content/components/login-item.css index c11920e9be80..a7362a583e97 100644 --- a/browser/components/aboutlogins/content/components/login-item.css +++ b/browser/components/aboutlogins/content/components/login-item.css @@ -291,26 +291,6 @@ a.breach-alert-link { background-color: transparent; } -.error-message { - color: #fff; - background-color: var(--red-60); - border: 1px solid transparent; - padding-block: 6px; - display: inline-block; - padding-inline-start: 32px; - padding-inline-end: 16px; - background-image: url("chrome://global/skin/icons/warning.svg"); - background-repeat: no-repeat; - background-position: left 10px center; - -moz-context-properties: fill; - fill: currentColor; - margin-bottom: 38px; -} - -.error-message:dir(rtl) { - background-position-x: right 10px; -} - @supports -moz-bool-pref("browser.in-content.dark-mode") { @media (prefers-color-scheme: dark) { :host { diff --git a/browser/components/aboutlogins/content/components/login-item.js b/browser/components/aboutlogins/content/components/login-item.js index 47146df33e15..4af595af1f0e 100644 --- a/browser/components/aboutlogins/content/components/login-item.js +++ b/browser/components/aboutlogins/content/components/login-item.js @@ -41,7 +41,6 @@ export default class LoginItem extends HTMLElement { ); this._deleteButton = this.shadowRoot.querySelector(".delete-button"); this._editButton = this.shadowRoot.querySelector(".edit-button"); - this._errorMessage = this.shadowRoot.querySelector(".error-message"); this._form = this.shadowRoot.querySelector("form"); this._originInput = this.shadowRoot.querySelector("input[name='origin']"); this._usernameInput = this.shadowRoot.querySelector( @@ -88,9 +87,7 @@ export default class LoginItem extends HTMLElement { } async render() { - [this._errorMessage, this._breachAlert].forEach(el => { - el.hidden = true; - }); + this._breachAlert.hidden = true; if (this._breachesMap && this._breachesMap.has(this._login.guid)) { const breachDetails = this._breachesMap.get(this._login.guid); const breachAlertLink = this._breachAlert.querySelector( @@ -153,30 +150,6 @@ export default class LoginItem extends HTMLElement { ); } - showLoginItemError(error) { - const errorMessageText = this._errorMessage.querySelector( - ".error-message-text" - ); - if (!error.errorMessage) { - return; - } - if (error.errorMessage.includes("This login already exists")) { - document.l10n.setAttributes( - errorMessageText, - "about-logins-error-message-duplicate-login", - { - loginTitle: error.login.title, - } - ); - } else { - document.l10n.setAttributes( - errorMessageText, - "about-logins-error-message-default" - ); - } - this._errorMessage.hidden = false; - } - async handleEvent(event) { switch (event.type) { case "AboutLoginsInitialLoginSelected": { diff --git a/browser/components/aboutlogins/tests/browser/browser.ini b/browser/components/aboutlogins/tests/browser/browser.ini index 61f9c7d41d37..6237f04e4795 100644 --- a/browser/components/aboutlogins/tests/browser/browser.ini +++ b/browser/components/aboutlogins/tests/browser/browser.ini @@ -17,7 +17,6 @@ skip-if = asan || debug || verify # bug 1574023 [browser_deleteLogin.js] [browser_dismissFooter.js] [browser_fxAccounts.js] -[browser_loginItemErrors.js] [browser_loginListChanges.js] [browser_masterPassword.js] skip-if = (os == 'linux') # bug 1569789 diff --git a/browser/components/aboutlogins/tests/browser/browser_loginItemErrors.js b/browser/components/aboutlogins/tests/browser/browser_loginItemErrors.js deleted file mode 100644 index ed66eed663bb..000000000000 --- a/browser/components/aboutlogins/tests/browser/browser_loginItemErrors.js +++ /dev/null @@ -1,126 +0,0 @@ -/* Any copyright is dedicated to the Public Domain. - * http://creativecommons.org/publicdomain/zero/1.0/ */ - -add_task(async function setup() { - await BrowserTestUtils.openNewForegroundTab({ - gBrowser, - url: "about:logins", - }); - registerCleanupFunction(() => { - BrowserTestUtils.removeTab(gBrowser.selectedTab); - Services.logins.removeAllLogins(); - }); -}); - -add_task(async function test_showLoginItemErrors() { - const browser = gBrowser.selectedBrowser; - let LOGIN_TO_UPDATE = new nsLoginInfo( - "https://example.com/", - "https://example.com/", - null, - "user2", - "pass2", - "username", - "password" - ); - LOGIN_TO_UPDATE = Services.logins.addLogin(LOGIN_TO_UPDATE); - - await ContentTask.spawn( - browser, - LoginHelper.loginToVanillaObject(LOGIN_TO_UPDATE), - async loginToUpdate => { - const loginItem = Cu.waiveXrays( - content.document.querySelector("login-item") - ); - const loginItemErrorMessage = Cu.waiveXrays( - loginItem.shadowRoot.querySelector(".error-message") - ); - const loginList = Cu.waiveXrays( - content.document.querySelector("login-list") - ); - - const createButton = loginList._createLoginButton; - createButton.click(); - - const loginUpdates = { - origin: "https://example.com/", - password: "my1GoodPassword", - username: "user1", - }; - - const event = Cu.cloneInto( - { - bubbles: true, - detail: loginUpdates, - }, - content - ); - - content.dispatchEvent( - // adds first lgoin - new content.CustomEvent("AboutLoginsCreateLogin", event) - ); - - ok( - loginItemErrorMessage.hidden, - "An error message should not be displayed after adding a new login." - ); - - content.dispatchEvent( - // adds a duplicate of the first login - new content.CustomEvent("AboutLoginsCreateLogin", event) - ); - - const loginItemErrorMessageVisible = await ContentTaskUtils.waitForCondition( - () => { - return !loginItemErrorMessage.hidden; - }, - "Waiting for error message to be shown after attempting to create a duplicate login." - ); - ok( - loginItemErrorMessageVisible, - "An error message should be shown after user attempts to add a login that already exists." - ); - - const loginItemErrorMessageText = loginItemErrorMessage.querySelector( - "span" - ); - ok( - loginItemErrorMessageText.dataset.l10nId === - "about-logins-error-message-duplicate-login", - "The correct error message is displayed." - ); - - let loginListItem = Cu.waiveXrays( - loginList.shadowRoot.querySelector( - `.login-list-item[data-guid='${loginToUpdate.guid}']` - ) - ); - loginListItem.click(); - - ok( - loginItemErrorMessage.hidden, - "The error message should no longer be visible." - ); - - const editButton = loginItem.shadowRoot.querySelector(".edit-button"); - editButton.click(); - - content.dispatchEvent( - // attempt to update LOGIN_TO_UPDATE to a username/origin combination that already exists. - new content.CustomEvent("AboutLoginsUpdateLogin", event) - ); - - const loginAlreadyExistsErrorShownAfterUpdate = ContentTaskUtils.waitForCondition( - () => { - return !loginItemErrorMessage.hidden; - }, - "Waiting for error message to show after updating login to existing login." - ); - ok( - loginAlreadyExistsErrorShownAfterUpdate, - "An error message should be shown after updating a login to a username/origin combination that already exists." - ); - } - ); -}); diff --git a/browser/locales/en-US/browser/aboutLogins.ftl b/browser/locales/en-US/browser/aboutLogins.ftl index e1f08e35bba5..ddb3b7997760 100644 --- a/browser/locales/en-US/browser/aboutLogins.ftl +++ b/browser/locales/en-US/browser/aboutLogins.ftl @@ -140,14 +140,3 @@ breach-alert-text = Passwords were leaked or stolen from this website since you breach-alert-link = Learn more about this breach. breach-alert-dismiss = .title = Close this alert - -## Error Messages - -# This is an error message that appears when a user attempts to save -# a new login that is identical to an existing saved login. -# Variables: -# $loginTitle (String) - The title of the website associated with the login. -about-logins-error-message-duplicate-login = An entry for { $loginTitle } with that username already exists. - -# This is a generic error message. -about-logins-error-message-default = An error occurred while trying to save this password. From 02f319bd8cfcec7726b31f0c968c1b2e56703c3d Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Tue, 27 Aug 2019 23:19:17 +0000 Subject: [PATCH 28/41] Bug 1577046 - Tie extra-bindgen-flags file creation to cranelift. r=chmanchester We only recurse into js/src/rust when jsrust is built, which it may not be in Gecko builds. But cranelift, which may be enabled either way, needs the extra-bindgen-flags file. Differential Revision: https://phabricator.services.mozilla.com/D43699 --HG-- extra : moz-landing-system : lando --- js/src/moz.build | 3 +++ js/src/rust/moz.build | 2 -- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/js/src/moz.build b/js/src/moz.build index 42687087a86f..32d1abfbeb7a 100755 --- a/js/src/moz.build +++ b/js/src/moz.build @@ -47,6 +47,9 @@ for stlfile in ['jsdate.*', 'jsnum.*']: with Files('builtin/intl/*'): BUG_COMPONENT = component_intl +if CONFIG['ENABLE_WASM_CRANELIFT']: + CONFIGURE_SUBST_FILES += ['rust/extra-bindgen-flags'] + if not CONFIG['JS_DISABLE_SHELL']: DIRS += [ 'rust', diff --git a/js/src/rust/moz.build b/js/src/rust/moz.build index 36b3c5682b23..ca3001de62f8 100644 --- a/js/src/rust/moz.build +++ b/js/src/rust/moz.build @@ -18,8 +18,6 @@ if CONFIG['ENABLE_WASM_CRANELIFT']: RustLibrary('jsrust', features) -CONFIGURE_SUBST_FILES += ['extra-bindgen-flags'] - if CONFIG['JS_SHARED_LIBRARY']: FINAL_LIBRARY = 'js' From 95a283d459a5080445f2963289f23d4e9269c873 Mon Sep 17 00:00:00 2001 From: Tim Nguyen Date: Wed, 28 Aug 2019 01:15:10 +0000 Subject: [PATCH 29/41] Bug 1575604 - Align the search field to the left of login-item. r=jaws Differential Revision: https://phabricator.services.mozilla.com/D43683 --HG-- extra : moz-landing-system : lando --- .../components/aboutlogins/content/aboutLogins.css | 13 ++++++++----- .../content/components/fxaccounts-button.css | 3 +++ 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/browser/components/aboutlogins/content/aboutLogins.css b/browser/components/aboutlogins/content/aboutLogins.css index d183a901ea77..b731ed464ead 100644 --- a/browser/components/aboutlogins/content/aboutLogins.css +++ b/browser/components/aboutlogins/content/aboutLogins.css @@ -3,8 +3,9 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ body { + --sidebar-width: 320px; display: grid; - grid-template-columns: 320px 1fr; + grid-template-columns: var(--sidebar-width) 1fr; grid-template-rows: 75px 1fr; grid-template-areas: "header header" "logins login"; @@ -17,12 +18,13 @@ header { align-items: center; background-color: var(--in-content-box-background); border-bottom: 1px solid var(--in-content-box-border-color); - padding-inline: 60px 23px; + padding-inline-end: 23px; } login-filter { - max-width: 30%; - margin: auto; + min-width: 320px; + max-width: 400px; + margin-inline: 40px auto; flex-grow: 1; align-self: center; } @@ -52,8 +54,9 @@ login-item { } #branding-logo { + flex-basis: var(--sidebar-width); + flex-shrink: 0; height: 32px; - margin-inline-end: 18px; -moz-context-properties: fill; fill: #20123a; } diff --git a/browser/components/aboutlogins/content/components/fxaccounts-button.css b/browser/components/aboutlogins/content/components/fxaccounts-button.css index 0ee590457c63..139bf974cd57 100644 --- a/browser/components/aboutlogins/content/components/fxaccounts-button.css +++ b/browser/components/aboutlogins/content/components/fxaccounts-button.css @@ -22,6 +22,9 @@ .fxaccounts-avatar-button { cursor: pointer; + white-space: nowrap; + text-overflow: ellipsis; + overflow: hidden; } .fxaccount-email { From 9e850c74a60c432c971a66148fba9ff462520158 Mon Sep 17 00:00:00 2001 From: Ted Campbell Date: Tue, 27 Aug 2019 23:02:37 +0000 Subject: [PATCH 30/41] Bug 1576865 - Add missing call to ReportOutOfMemory in LazyScriptCreationData. r=jwalden The closedOverBinding set uses the SystemAllocPolicy so we must manually raise OOM exceptions. Differential Revision: https://phabricator.services.mozilla.com/D43650 --HG-- extra : moz-landing-system : lando --- js/src/frontend/SharedContext.h | 1 + js/src/jit-test/tests/parser/bug-1576865-1.js | 49 +++++++++++++++++++ js/src/jit-test/tests/parser/bug-1576865-2.js | 44 +++++++++++++++++ 3 files changed, 94 insertions(+) create mode 100644 js/src/jit-test/tests/parser/bug-1576865-1.js create mode 100644 js/src/jit-test/tests/parser/bug-1576865-2.js diff --git a/js/src/frontend/SharedContext.h b/js/src/frontend/SharedContext.h index bf8dad626662..939d87868363 100644 --- a/js/src/frontend/SharedContext.h +++ b/js/src/frontend/SharedContext.h @@ -291,6 +291,7 @@ struct LazyScriptCreationData { } if (!closedOverBindings.appendAll(COB)) { + ReportOutOfMemory(cx); // closedOverBindings uses SystemAllocPolicy. return false; } return true; diff --git a/js/src/jit-test/tests/parser/bug-1576865-1.js b/js/src/jit-test/tests/parser/bug-1576865-1.js new file mode 100644 index 000000000000..f31539b8ea21 --- /dev/null +++ b/js/src/jit-test/tests/parser/bug-1576865-1.js @@ -0,0 +1,49 @@ +// |jit-test| skip-if: !('oomTest' in this) +var sourceText = ` + function Outer() { + var X00, X01, X02, X03, X04, X05, X06, X07; + var X08, X09, X10, X11, X12, X13, X14, X15; + var X16, X17, X18, X19, X20, X21, X22, X23; + var X24, X25, X26, X27, X28, X29, X30, X31; + + function LazyFunction() { + // Lots of closed-over bindings. + { X00 = true; }; + { X01 = true; }; + { X02 = true; }; + { X03 = true; }; + { X04 = true; }; + { X05 = true; }; + { X06 = true; }; + { X07 = true; }; + { X08 = true; }; + { X09 = true; }; + { X10 = true; }; + { X11 = true; }; + { X12 = true; }; + { X13 = true; }; + { X14 = true; }; + { X15 = true; }; + { X16 = true; }; + { X17 = true; }; + { X18 = true; }; + { X19 = true; }; + { X20 = true; }; + { X21 = true; }; + { X22 = true; }; + { X23 = true; }; + { X24 = true; }; + { X25 = true; }; + { X26 = true; }; + { X27 = true; }; + { X28 = true; }; + { X29 = true; }; + { X30 = true; }; + { X31 = true; }; + } + } +`; + +oomTest(function() { + evaluate(sourceText); + }); diff --git a/js/src/jit-test/tests/parser/bug-1576865-2.js b/js/src/jit-test/tests/parser/bug-1576865-2.js new file mode 100644 index 000000000000..d053c2472822 --- /dev/null +++ b/js/src/jit-test/tests/parser/bug-1576865-2.js @@ -0,0 +1,44 @@ +// |jit-test| skip-if: !('oomTest' in this) +var sourceText = ` + function Outer() { + function LazyFunction() { + // Lots of inner functions. + function F00() { } + function F01() { } + function F02() { } + function F03() { } + function F04() { } + function F05() { } + function F06() { } + function F07() { } + function F08() { } + function F09() { } + function F10() { } + function F11() { } + function F12() { } + function F13() { } + function F14() { } + function F15() { } + function F16() { } + function F17() { } + function F18() { } + function F19() { } + function F20() { } + function F21() { } + function F22() { } + function F23() { } + function F24() { } + function F25() { } + function F26() { } + function F27() { } + function F28() { } + function F29() { } + function F30() { } + function F31() { } + } + } +`; + +oomTest(function() { + evaluate(sourceText); + }); From ef58a63b92994afbbbea93f9b3db8eb5d6c4c324 Mon Sep 17 00:00:00 2001 From: David Walsh Date: Wed, 28 Aug 2019 01:24:15 +0000 Subject: [PATCH 31/41] Bug 1576165 - Prevent react warning in inline preview r=jlast Differential Revision: https://phabricator.services.mozilla.com/D43647 --HG-- extra : moz-landing-system : lando --- devtools/client/debugger/src/components/Editor/InlinePreviews.js | 1 + 1 file changed, 1 insertion(+) diff --git a/devtools/client/debugger/src/components/Editor/InlinePreviews.js b/devtools/client/debugger/src/components/Editor/InlinePreviews.js index b3df0e82a507..25d50d2d9f03 100644 --- a/devtools/client/debugger/src/components/Editor/InlinePreviews.js +++ b/devtools/client/debugger/src/components/Editor/InlinePreviews.js @@ -54,6 +54,7 @@ class InlinePreviews extends Component { return ( Date: Wed, 28 Aug 2019 05:10:33 +0300 Subject: [PATCH 32/41] Backed out changeset 7cb87169e4cf (bug 1567219) for xpcshell failures in marAppApplyUpdateAppBinInUseStageSuccessSvc_win.js. --- browser/app/winlauncher/moz.build | 1 - toolkit/components/telemetry/Scalars.yaml | 19 ------ toolkit/xre/WinTokenUtils.cpp | 72 ----------------------- toolkit/xre/WinTokenUtils.h | 18 ------ toolkit/xre/moz.build | 2 - toolkit/xre/nsAppRunner.cpp | 10 ---- 6 files changed, 122 deletions(-) delete mode 100644 toolkit/xre/WinTokenUtils.cpp delete mode 100644 toolkit/xre/WinTokenUtils.h diff --git a/browser/app/winlauncher/moz.build b/browser/app/winlauncher/moz.build index be2be08e32a5..f50cc8f1302c 100644 --- a/browser/app/winlauncher/moz.build +++ b/browser/app/winlauncher/moz.build @@ -60,7 +60,6 @@ TEST_DIRS += [ if CONFIG['MOZ_LAUNCHER_PROCESS']: UNIFIED_SOURCES += [ '/toolkit/xre/LauncherRegistryInfo.cpp', - '/toolkit/xre/WinTokenUtils.cpp', ] for var in ('MOZ_APP_BASENAME', 'MOZ_APP_VENDOR'): DEFINES[var] = '"%s"' % CONFIG[var] diff --git a/toolkit/components/telemetry/Scalars.yaml b/toolkit/components/telemetry/Scalars.yaml index aae2012b4118..4530dd594c6e 100644 --- a/toolkit/components/telemetry/Scalars.yaml +++ b/toolkit/components/telemetry/Scalars.yaml @@ -682,25 +682,6 @@ sandbox: operating_systems: - "windows" -os.environment: - is_admin_without_uac: - bug_numbers: - - 1567219 - description: > - Indicates that the process is lauched with Admin privileges but without - UAC. - expires: never - kind: boolean - notification_emails: - - tkikuchi@mozilla.com - release_channel_collection: opt-out - products: - - 'firefox' - record_in_processes: - - main - operating_systems: - - "windows" - pictureinpicture: opened_method: bug_numbers: diff --git a/toolkit/xre/WinTokenUtils.cpp b/toolkit/xre/WinTokenUtils.cpp deleted file mode 100644 index ed45e0c8fac8..000000000000 --- a/toolkit/xre/WinTokenUtils.cpp +++ /dev/null @@ -1,72 +0,0 @@ -/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ -/* vim: set ts=8 sts=2 et sw=2 tw=80: */ -/* 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 https://mozilla.org/MPL/2.0/. */ - -#include "WinTokenUtils.h" -#include "nsWindowsHelpers.h" - -using namespace mozilla; - -// If |aToken| is nullptr, CheckTokenMembership uses the calling thread's -// primary token to check membership for. -static LauncherResult IsMemberOfAdministrators( - const nsAutoHandle& aToken) { - BYTE adminsGroupSid[SECURITY_MAX_SID_SIZE]; - DWORD adminsGroupSidSize = sizeof(adminsGroupSid); - if (!CreateWellKnownSid(WinBuiltinAdministratorsSid, nullptr, adminsGroupSid, - &adminsGroupSidSize)) { - return LAUNCHER_ERROR_FROM_LAST(); - } - - BOOL isMember; - if (!CheckTokenMembership(aToken, adminsGroupSid, &isMember)) { - return LAUNCHER_ERROR_FROM_LAST(); - } - return !!isMember; -} - -static LauncherResult IsUacEnabled() { - DWORD len = sizeof(DWORD); - DWORD value; - LSTATUS status = RegGetValueW( - HKEY_LOCAL_MACHINE, - L"SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Policies\\System", - L"EnableLUA", RRF_RT_DWORD, nullptr, &value, &len); - if (status != ERROR_SUCCESS) { - return LAUNCHER_ERROR_FROM_WIN32(status); - } - - // UAC is disabled only when EnableLUA is 0. - return (value != 0); -} - -namespace mozilla { - -LauncherResult IsAdminWithoutUac() { - // To check whether the process was launched with Administrator priviledges - // or not, we cannot simply check the integrity level of the current process - // because the launcher process spawns the browser process with the medium - // integrity level even though the launcher process is high integrity level. - // We check whether the thread's token contains Administratos SID or not - // instead. - LauncherResult containsAdminGroup = - IsMemberOfAdministrators(nsAutoHandle()); - if (containsAdminGroup.isErr()) { - return LAUNCHER_ERROR_FROM_RESULT(containsAdminGroup); - } - - if (!containsAdminGroup.unwrap()) { - return false; - } - - LauncherResult isUacEnabled = IsUacEnabled(); - if (isUacEnabled.isErr()) { - return LAUNCHER_ERROR_FROM_RESULT(isUacEnabled); - } - - return !isUacEnabled.unwrap(); -} - -} // namespace mozilla diff --git a/toolkit/xre/WinTokenUtils.h b/toolkit/xre/WinTokenUtils.h deleted file mode 100644 index f582bd0696b2..000000000000 --- a/toolkit/xre/WinTokenUtils.h +++ /dev/null @@ -1,18 +0,0 @@ -/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ -/* vim: set ts=8 sts=2 et sw=2 tw=80: */ -/* 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 https://mozilla.org/MPL/2.0/. */ - -#ifndef mozilla_WinTokenUtils_h -#define mozilla_WinTokenUtils_h - -#include "mozilla/LauncherResult.h" - -namespace mozilla { - -LauncherResult IsAdminWithoutUac(); - -} // namespace mozilla - -#endif // mozilla_WinTokenUtils_h diff --git a/toolkit/xre/moz.build b/toolkit/xre/moz.build index f53f7b38f4c0..57ae50a68c0d 100644 --- a/toolkit/xre/moz.build +++ b/toolkit/xre/moz.build @@ -50,7 +50,6 @@ if CONFIG['MOZ_WIDGET_TOOLKIT'] == 'windows': 'ModuleVersionInfo_windows.h', 'PolicyChecks.h', 'WinDllServices.h', - 'WinTokenUtils.h', ] UNIFIED_SOURCES += [ '/toolkit/mozapps/update/common/updateutils_win.cpp', @@ -58,7 +57,6 @@ if CONFIG['MOZ_WIDGET_TOOLKIT'] == 'windows': 'ModuleVersionInfo_windows.cpp', 'nsNativeAppSupportWin.cpp', 'WinDllServices.cpp', - 'WinTokenUtils.cpp', ] DEFINES['PROXY_PRINTING'] = 1 LOCAL_INCLUDES += [ diff --git a/toolkit/xre/nsAppRunner.cpp b/toolkit/xre/nsAppRunner.cpp index 80b926d71367..d7883d9f70a0 100644 --- a/toolkit/xre/nsAppRunner.cpp +++ b/toolkit/xre/nsAppRunner.cpp @@ -113,7 +113,6 @@ # include "mozilla/WinHeaderOnlyUtils.h" # include "mozilla/mscom/ProcessRuntime.h" # include "mozilla/widget/AudioSession.h" -# include "WinTokenUtils.h" # if defined(MOZ_LAUNCHER_PROCESS) # include "mozilla/LauncherRegistryInfo.h" @@ -4564,15 +4563,6 @@ nsresult XREMain::XRE_mainRun() { CrashReporter::Annotation::ContentSandboxCapabilities, flagsString); #endif /* MOZ_SANDBOX && XP_LINUX */ -#if defined(XP_WIN) - LauncherResult isAdminWithoutUac = IsAdminWithoutUac(); - if (isAdminWithoutUac.isOk()) { - Telemetry::ScalarSet( - Telemetry::ScalarID::OS_ENVIRONMENT_IS_ADMIN_WITHOUT_UAC, - isAdminWithoutUac.unwrap()); - } -#endif /* XP_WIN */ - #if defined(MOZ_SANDBOX) AddSandboxAnnotations(); #endif /* MOZ_SANDBOX */ From 22abd7da9b93edf1d6802e26f43392ed5bd692ad Mon Sep 17 00:00:00 2001 From: Paul Bone Date: Tue, 27 Aug 2019 19:05:41 +0000 Subject: [PATCH 33/41] Bug 1576814 - Remove fail-if tests that are now passing r=kmag Differential Revision: https://phabricator.services.mozilla.com/D43571 --HG-- extra : moz-landing-system : lando --- devtools/server/tests/browser/browser.ini | 1 - dom/tests/mochitest/bugs/mochitest.ini | 1 - dom/tests/mochitest/whatwg/mochitest.ini | 5 ----- 3 files changed, 7 deletions(-) diff --git a/devtools/server/tests/browser/browser.ini b/devtools/server/tests/browser/browser.ini index 8542a737e1c2..a628f0a9eb48 100644 --- a/devtools/server/tests/browser/browser.ini +++ b/devtools/server/tests/browser/browser.ini @@ -144,7 +144,6 @@ skip-if = e10s # Bug 1183605 - devtools/server/tests/browser/ tests are still di [browser_storage_browser_toolbox_indexeddb.js] [browser_storage_cookies-duplicate-names.js] [browser_storage_dynamic_windows.js] -fail-if = fission [browser_storage_listings.js] fail-if = fission [browser_storage_updates.js] diff --git a/dom/tests/mochitest/bugs/mochitest.ini b/dom/tests/mochitest/bugs/mochitest.ini index f28b98ce8686..786558a97061 100644 --- a/dom/tests/mochitest/bugs/mochitest.ini +++ b/dom/tests/mochitest/bugs/mochitest.ini @@ -156,7 +156,6 @@ skip-if = toolkit == 'android' [test_bug1112040.html] [test_bug1160342_marquee.html] [test_bug1171215.html] -fail-if = fission support-files = window_bug1171215.html [test_bug1530292.html] fail-if = fission diff --git a/dom/tests/mochitest/whatwg/mochitest.ini b/dom/tests/mochitest/whatwg/mochitest.ini index 5b67e0da16a9..f0970238cfb6 100644 --- a/dom/tests/mochitest/whatwg/mochitest.ini +++ b/dom/tests/mochitest/whatwg/mochitest.ini @@ -25,15 +25,11 @@ support-files = skip-if = toolkit == 'android' #bug 894914 - wrong data - got FAIL, expected message [test_postMessage_hash.html] [test_postMessage.html] -fail-if = fission [test_postMessage_idn.xhtml] -fail-if = fission [test_postMessage_joined.html] -fail-if = fission [test_postMessage_onOther.html] skip-if = fission #Bug 1571273 [test_postMessage_origin.xhtml] -fail-if = fission [test_postMessage_override.html] skip-if = fission [test_postMessage_special.xhtml] @@ -43,7 +39,6 @@ skip-if = fission #Bug 1570918 [test_postMessage_transfer.html] skip-if = fission #Bug 1571208 [test_postMessage_userpass.html] -fail-if = fission [test_bug500328.html] skip-if = true || toolkit=='android' # bug 696306, #TIMED_OUT android support-files = file_bug500328_1.html file_bug500328_2.html From 36bed861bfa994782babe4b07b56aaf630077505 Mon Sep 17 00:00:00 2001 From: Paul Bone Date: Tue, 27 Aug 2019 23:42:25 +0000 Subject: [PATCH 34/41] Bug 1576814 - These tests are currently failing for fission r=kmag Depends on D43571 Differential Revision: https://phabricator.services.mozilla.com/D43572 --HG-- extra : moz-landing-system : lando --- dom/security/test/general/mochitest.ini | 1 + dom/tests/mochitest/localstorage/mochitest.ini | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/dom/security/test/general/mochitest.ini b/dom/security/test/general/mochitest.ini index 4ac3ce750dc6..97b0847f939b 100644 --- a/dom/security/test/general/mochitest.ini +++ b/dom/security/test/general/mochitest.ini @@ -47,6 +47,7 @@ skip-if = toolkit == 'android' [test_same_site_cookies_toplevel_nav.html] skip-if = fission # Crashes: @ mozilla::dom::ContentParent::CommonCreateWindow(mozilla::dom::PBrowserParent*, bool, unsigned int const&, bool const&, bool const&, bool const&, nsIURI*, nsTString const&, float const&, unsigned long, nsTString const&, nsresult&, nsCOMPtr&, bool*, int&, nsIPrincipal*, nsIReferrerInfo*, bool, nsIContentSecurityPolicy*) [test_same_site_cookies_cross_origin_context.html] +fail-if = fission [test_same_site_cookies_from_script.html] [test_same_site_cookies_redirect.html] [test_same_site_cookies_toplevel_set_cookie.html] diff --git a/dom/tests/mochitest/localstorage/mochitest.ini b/dom/tests/mochitest/localstorage/mochitest.ini index 75684eff0d73..1a5ea9589926 100644 --- a/dom/tests/mochitest/localstorage/mochitest.ini +++ b/dom/tests/mochitest/localstorage/mochitest.ini @@ -27,6 +27,7 @@ skip-if = os == "android" || verify # bug 962029 [test_cookieBlock.html] [test_embededNulls.html] [test_keySync.html] +fail-if = fission [test_localStorageBase.html] skip-if = e10s [test_localStorageBaseSessionOnly.html] @@ -34,12 +35,17 @@ skip-if = e10s [test_localStorageEnablePref.html] [test_localStorageKeyOrder.html] [test_localStorageOriginsDiff.html] +fail-if = fission [test_localStorageOriginsDomainDiffs.html] +fail-if = fission [test_localStorageOriginsEquals.html] skip-if = toolkit == 'android' +fail-if = fission [test_localStorageOriginsPortDiffs.html] +fail-if = fission [test_localStorageOriginsSchemaDiffs.html] skip-if = toolkit == 'android' #TIMED_OUT +fail-if = fission [test_localStorageQuota.html] skip-if = toolkit == 'android' #TIMED_OUT [test_localStorageQuotaSessionOnly.html] From 08bc01ab377c68f9727dbac9e1dbb4e0e1107659 Mon Sep 17 00:00:00 2001 From: Paul Bone Date: Tue, 27 Aug 2019 19:07:43 +0000 Subject: [PATCH 35/41] Bug 1576814 - Skip tests that timeout r=kmag Depends on D43572 Differential Revision: https://phabricator.services.mozilla.com/D43573 --HG-- extra : moz-landing-system : lando --- browser/base/content/test/siteIdentity/browser.ini | 1 + browser/components/extensions/test/browser/browser.ini | 1 + .../components/resistfingerprinting/test/browser/browser.ini | 1 + browser/components/urlbar/tests/browser/browser.ini | 1 + devtools/client/inspector/test/browser.ini | 2 +- devtools/client/styleeditor/test/browser.ini | 2 +- devtools/server/tests/browser/browser.ini | 1 + dom/websocket/tests/mochitest.ini | 1 + dom/workers/test/mochitest.ini | 1 + netwerk/test/mochitests/mochitest.ini | 1 + 10 files changed, 10 insertions(+), 2 deletions(-) diff --git a/browser/base/content/test/siteIdentity/browser.ini b/browser/base/content/test/siteIdentity/browser.ini index d4a83c3480b8..60013efb838b 100644 --- a/browser/base/content/test/siteIdentity/browser.ini +++ b/browser/base/content/test/siteIdentity/browser.ini @@ -112,6 +112,7 @@ support-files = test_no_mcb_on_http_site_font2.html test_no_mcb_on_http_site_font2.css [browser_no_mcb_for_loopback.js] +skip-if = fission # Timeouts tags = mcb support-files = ../general/moz.png diff --git a/browser/components/extensions/test/browser/browser.ini b/browser/components/extensions/test/browser/browser.ini index 152a978136e0..c6c39315f4cf 100644 --- a/browser/components/extensions/test/browser/browser.ini +++ b/browser/components/extensions/test/browser/browser.ini @@ -164,6 +164,7 @@ skip-if = (verify && debug && (os == 'mac')) [browser_ext_pageAction_simple.js] [browser_ext_pageAction_telemetry.js] [browser_ext_pageAction_title.js] +skip-if = fission # Timeouts Bug 1574926 [browser_ext_popup_api_injection.js] [browser_ext_popup_background.js] [browser_ext_popup_corners.js] diff --git a/browser/components/resistfingerprinting/test/browser/browser.ini b/browser/components/resistfingerprinting/test/browser/browser.ini index 74292f589183..72647001a7f1 100644 --- a/browser/components/resistfingerprinting/test/browser/browser.ini +++ b/browser/components/resistfingerprinting/test/browser/browser.ini @@ -13,6 +13,7 @@ support-files = [browser_block_mozAddonManager.js] [browser_dynamical_window_rounding.js] [browser_navigator.js] +skip-if = fission # Timeouts [browser_netInfo.js] [browser_performanceAPI.js] [browser_roundedWindow_dialogWindow.js] diff --git a/browser/components/urlbar/tests/browser/browser.ini b/browser/components/urlbar/tests/browser/browser.ini index dcdd0a7088fe..fd2a526a38cc 100644 --- a/browser/components/urlbar/tests/browser/browser.ini +++ b/browser/components/urlbar/tests/browser/browser.ini @@ -72,6 +72,7 @@ support-files = skip-if = (os == 'mac' && os_version == '10.14') # bug 1554807 [browser_locationBarExternalLoad.js] [browser_moz_action_link.js] +skip-if = fission # Timeouts [browser_new_tab_urlbar_reset.js] [browser_openViewOnFocus.js] [browser_pasteAndGo.js] diff --git a/devtools/client/inspector/test/browser.ini b/devtools/client/inspector/test/browser.ini index 221b9bcbf0ef..d50650a6f775 100644 --- a/devtools/client/inspector/test/browser.ini +++ b/devtools/client/inspector/test/browser.ini @@ -192,7 +192,7 @@ skip-if = fission && debug # Leaks windows. [browser_inspector_search-04.js] fail-if = fission [browser_inspector_search-05.js] -fail-if = fission +skip-if = fission # Timeouts [browser_inspector_search-06.js] [browser_inspector_search-07.js] [browser_inspector_search-08.js] diff --git a/devtools/client/styleeditor/test/browser.ini b/devtools/client/styleeditor/test/browser.ini index 5ba9eec82501..757560550ccf 100644 --- a/devtools/client/styleeditor/test/browser.ini +++ b/devtools/client/styleeditor/test/browser.ini @@ -79,7 +79,7 @@ support-files = [browser_styleeditor_autocomplete-disabled.js] [browser_styleeditor_bom.js] [browser_styleeditor_bug_740541_iframes.js] -fail-if = fission +skip-if = fission # Timeouts [browser_styleeditor_bug_851132_middle_click.js] [browser_styleeditor_bug_870339.js] [browser_styleeditor_bug_1405342_serviceworker_iframes.js] diff --git a/devtools/server/tests/browser/browser.ini b/devtools/server/tests/browser/browser.ini index a628f0a9eb48..9216389fd667 100644 --- a/devtools/server/tests/browser/browser.ini +++ b/devtools/server/tests/browser/browser.ini @@ -144,6 +144,7 @@ skip-if = e10s # Bug 1183605 - devtools/server/tests/browser/ tests are still di [browser_storage_browser_toolbox_indexeddb.js] [browser_storage_cookies-duplicate-names.js] [browser_storage_dynamic_windows.js] +skip-if = fission # Timeouts [browser_storage_listings.js] fail-if = fission [browser_storage_updates.js] diff --git a/dom/websocket/tests/mochitest.ini b/dom/websocket/tests/mochitest.ini index 960468887400..932bb8a49040 100644 --- a/dom/websocket/tests/mochitest.ini +++ b/dom/websocket/tests/mochitest.ini @@ -16,6 +16,7 @@ support-files = [test_bug1081686.html] [test_bug1384658.html] +skip-if = fission # Timeouts support-files = window_bug1384658.html frame_bug1384658.html file_bug1384658.html [test_event_listener_leaks.html] support-files = file_websocket_bigBlob_wsh.py diff --git a/dom/workers/test/mochitest.ini b/dom/workers/test/mochitest.ini index 1badfa706580..9cd29cdc93f0 100644 --- a/dom/workers/test/mochitest.ini +++ b/dom/workers/test/mochitest.ini @@ -145,6 +145,7 @@ tags = mcb [test_location.html] [test_longThread.html] [test_multi_sharedWorker.html] +skip-if = fission # Timeouts [test_multi_sharedWorker_lifetimes.html] [test_navigator.html] support-files = diff --git a/netwerk/test/mochitests/mochitest.ini b/netwerk/test/mochitests/mochitest.ini index 71071eb39ed6..5d0856a0715a 100644 --- a/netwerk/test/mochitests/mochitest.ini +++ b/netwerk/test/mochitests/mochitest.ini @@ -83,6 +83,7 @@ skip-if = verify [test_1503201.html] [test_origin_header.html] [test_1502055.html] +skip-if = fission # Timeouts support-files = sw_1502055.js file_1502055.sjs iframe_1502055.html [test_accept_header.html] support-files = test_accept_header.sjs From 6c79cd1c3306c919859955364da57ca764d3109c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A3o=20Gottwald?= Date: Tue, 27 Aug 2019 19:53:23 +0000 Subject: [PATCH 36/41] Bug 1576847 - Remove obsolete #wrapper-urlbar-container[place="palette"] rule. r=mak Differential Revision: https://phabricator.services.mozilla.com/D43589 --HG-- extra : moz-landing-system : lando --- browser/themes/osx/browser.css | 4 ---- 1 file changed, 4 deletions(-) diff --git a/browser/themes/osx/browser.css b/browser/themes/osx/browser.css index dfb98d1dca56..0ed855b233b4 100644 --- a/browser/themes/osx/browser.css +++ b/browser/themes/osx/browser.css @@ -274,10 +274,6 @@ %include ../shared/identity-block/identity-block.inc.css -#wrapper-urlbar-container[place="palette"] { - max-width: 20em; -} - #pageAction-urlbar-shareURL, #pageAction-panel-shareURL { list-style-image: url("chrome://browser/skin/share.svg"); From 59d63f63ccca5feba4e3b91a420397aa7e75db3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A3o=20Gottwald?= Date: Wed, 28 Aug 2019 03:43:00 +0000 Subject: [PATCH 37/41] Bug 1576657 - Stop increasing .urlbarView's font size since it already inherits the font size set on #urlbar. r=mak Differential Revision: https://phabricator.services.mozilla.com/D43597 --HG-- extra : moz-landing-system : lando --- browser/themes/windows/browser.css | 4 ---- 1 file changed, 4 deletions(-) diff --git a/browser/themes/windows/browser.css b/browser/themes/windows/browser.css index 5d4a6e5711f9..6136ccabcde2 100644 --- a/browser/themes/windows/browser.css +++ b/browser/themes/windows/browser.css @@ -574,10 +574,6 @@ menuitem.bookmark-item { %include ../shared/autocomplete.inc.css %include ../shared/urlbar-autocomplete.inc.css -.urlbarView { - font-size: 1.15em; -} - #PopupAutoComplete > richlistbox > richlistitem[originaltype~="datalist-first"] { border-top: 1px solid ThreeDShadow; } From 74b4cd52824271afac38954bb6d715bc6f97864b Mon Sep 17 00:00:00 2001 From: Nicolas Chevobbe Date: Tue, 27 Aug 2019 17:41:29 +0000 Subject: [PATCH 38/41] Bug 1576841 - Enable multi-proxy WebConsole in Browser Toolbox when fission pref is true. r=ochameau. We remove the multiple this.fissionSupport properties to only check the permission in the couple place we need it. Differential Revision: https://phabricator.services.mozilla.com/D43585 --HG-- extra : moz-landing-system : lando --- .../webconsole/browser-console-manager.js | 25 +++++++++---------- devtools/client/webconsole/browser-console.js | 5 ++-- devtools/client/webconsole/constants.js | 1 + .../webconsole/webconsole-connection-proxy.js | 15 ++++++++--- devtools/client/webconsole/webconsole-ui.js | 23 ++++++++++++++--- devtools/client/webconsole/webconsole.js | 9 +------ 6 files changed, 46 insertions(+), 32 deletions(-) diff --git a/devtools/client/webconsole/browser-console-manager.js b/devtools/client/webconsole/browser-console-manager.js index 51f6b0a70ef0..e74cacccb605 100644 --- a/devtools/client/webconsole/browser-console-manager.js +++ b/devtools/client/webconsole/browser-console-manager.js @@ -18,6 +18,12 @@ loader.lazyRequireGetter( "BrowserConsole", "devtools/client/webconsole/browser-console" ); +loader.lazyRequireGetter( + this, + "PREFS", + "devtools/client/webconsole/constants", + true +); const BC_WINDOW_FEATURES = "chrome,titlebar,toolbar,centerscreen,resizable,dialog=no"; @@ -46,12 +52,11 @@ class BrowserConsoleManager { * The target that the browser console will connect to. * @param nsIDOMWindow iframeWindow * The window where the browser console UI is already loaded. - * @param Boolean fissionSupport * @return object * A promise object for the opening of the new BrowserConsole instance. */ - async openBrowserConsole(target, win, fissionSupport = false) { - const hud = new BrowserConsole(target, win, win, fissionSupport); + async openBrowserConsole(target, win) { + const hud = new BrowserConsole(target, win, win); this._browserConsole = hud; hud.once("destroyed", () => { this._browserConsole = null; @@ -73,11 +78,6 @@ class BrowserConsoleManager { return this._browserConsoleInitializing; } - const fissionSupport = Services.prefs.getBoolPref( - "devtools.browsertoolbox.fission", - false - ); - async function connect() { // The Browser console ends up using the debugger in autocomplete. // Because the debugger can't be running in the same compartment than its debuggee, @@ -124,6 +124,9 @@ class BrowserConsoleManager { win.addEventListener("DOMContentLoaded", resolve, { once: true }); }); + const fissionSupport = Services.prefs.getBoolPref( + PREFS.FEATURES.BROWSER_TOOLBOX_FISSION + ); const title = fissionSupport ? `💥 Fission Browser Console 💥` : l10n.getStr("browserConsole.title"); @@ -137,11 +140,7 @@ class BrowserConsoleManager { const target = await connect(); await target.attach(); const win = await openWindow(target); - const browserConsole = await this.openBrowserConsole( - target, - win, - fissionSupport - ); + const browserConsole = await this.openBrowserConsole(target, win); return browserConsole; })(); diff --git a/devtools/client/webconsole/browser-console.js b/devtools/client/webconsole/browser-console.js index 8805e24b33b2..c45ffc72d1dc 100644 --- a/devtools/client/webconsole/browser-console.js +++ b/devtools/client/webconsole/browser-console.js @@ -35,10 +35,9 @@ class BrowserConsole extends WebConsole { * The window where the browser console UI is already loaded. * @param nsIDOMWindow chromeWindow * The window of the browser console owner. - * @param Boolean fissionSupport */ - constructor(target, iframeWindow, chromeWindow, fissionSupport = false) { - super(null, iframeWindow, chromeWindow, true, fissionSupport); + constructor(target, iframeWindow, chromeWindow) { + super(null, iframeWindow, chromeWindow, true); this._browserConsoleTarget = target; this._telemetry = new Telemetry(); diff --git a/devtools/client/webconsole/constants.js b/devtools/client/webconsole/constants.js index f90d940fdeca..e6f0203c41bc 100644 --- a/devtools/client/webconsole/constants.js +++ b/devtools/client/webconsole/constants.js @@ -83,6 +83,7 @@ const prefs = { AUTOCOMPLETE: "devtools.webconsole.input.autocomplete", GROUP_WARNINGS: "devtools.webconsole.groupWarningMessages", EDITOR: "devtools.webconsole.features.editor", + BROWSER_TOOLBOX_FISSION: "devtools.browsertoolbox.fission", }, }, }; diff --git a/devtools/client/webconsole/webconsole-connection-proxy.js b/devtools/client/webconsole/webconsole-connection-proxy.js index 8873933550fe..65a681b9dfac 100644 --- a/devtools/client/webconsole/webconsole-connection-proxy.js +++ b/devtools/client/webconsole/webconsole-connection-proxy.js @@ -20,13 +20,20 @@ class WebConsoleConnectionProxy { * @constructor * @param {WebConsoleUI} webConsoleUI * A WebConsoleUI instance that owns this connection proxy. - * @param RemoteTarget target + * @param {RemoteTarget} target * The target that the console will connect to. + * @param {Boolean} needContentProcessMessagesListener + * Set to true to specifically add a ContentProcessMessages listener. This is + * needed for non-fission Browser Console for example. */ - constructor(webConsoleUI, target) { + constructor( + webConsoleUI, + target, + needContentProcessMessagesListener = false + ) { this.webConsoleUI = webConsoleUI; this.target = target; - this.fissionSupport = this.webConsoleUI.fissionSupport; + this.needContentProcessMessagesListener = needContentProcessMessagesListener; this._connecter = null; @@ -116,7 +123,7 @@ class WebConsoleConnectionProxy { // when we open the Browser Console or Toolbox without fission support. If Fission // is enabled, we don't use the ContentProcessMessages listener, but attach to the // content processes directly. - if (this.target.chrome && !this.target.isAddon && !this.fissionSupport) { + if (this.needContentProcessMessagesListener) { listeners.push("ContentProcessMessages"); } return this.webConsoleClient.startListeners(listeners); diff --git a/devtools/client/webconsole/webconsole-ui.js b/devtools/client/webconsole/webconsole-ui.js index 7d78d324d78d..4a3448cfb2d4 100644 --- a/devtools/client/webconsole/webconsole-ui.js +++ b/devtools/client/webconsole/webconsole-ui.js @@ -24,6 +24,12 @@ loader.lazyRequireGetter( "resource://gre/modules/AppConstants.jsm", true ); +loader.lazyRequireGetter( + this, + "PREFS", + "devtools/client/webconsole/constants", + true +); const ZoomKeys = require("devtools/client/shared/zoom-keys"); @@ -45,7 +51,6 @@ class WebConsoleUI { this.hud = hud; this.hudId = this.hud.hudId; this.isBrowserConsole = this.hud.isBrowserConsole; - this.fissionSupport = this.hud.fissionSupport; this.window = this.hud.iframeWindow; this._onPanelSelected = this._onPanelSelected.bind(this); @@ -257,10 +262,20 @@ class WebConsoleUI { * A promise object that is resolved/reject based on the proxies connections. */ async _initConnection() { - this.proxy = new WebConsoleConnectionProxy(this, this.hud.currentTarget); - const target = this.hud.currentTarget; - if (this.fissionSupport && target.chrome && !target.isAddon) { + const fissionSupport = Services.prefs.getBoolPref( + PREFS.FEATURES.BROWSER_TOOLBOX_FISSION + ); + const needContentProcessMessagesListener = + target.isParentProcess && !target.isAddon && !fissionSupport; + + this.proxy = new WebConsoleConnectionProxy( + this, + target, + needContentProcessMessagesListener + ); + + if (fissionSupport && target.isParentProcess && !target.isAddon) { const { mainRoot } = target.client; const { processes } = await mainRoot.listProcesses(); diff --git a/devtools/client/webconsole/webconsole.js b/devtools/client/webconsole/webconsole.js index 01f6a0be09ac..eaba3f07452e 100644 --- a/devtools/client/webconsole/webconsole.js +++ b/devtools/client/webconsole/webconsole.js @@ -59,20 +59,13 @@ class WebConsole { * The window of the web console owner. * @param bool isBrowserConsole */ - constructor( - toolbox, - iframeWindow, - chromeWindow, - isBrowserConsole = false, - fissionSupport = false - ) { + constructor(toolbox, iframeWindow, chromeWindow, isBrowserConsole = false) { this.toolbox = toolbox; this.iframeWindow = iframeWindow; this.chromeWindow = chromeWindow; this.hudId = "hud_" + ++gHudId; this.browserWindow = this.chromeWindow.top; this.isBrowserConsole = isBrowserConsole; - this.fissionSupport = fissionSupport; const element = this.browserWindow.document.documentElement; if (element.getAttribute("windowtype") != gDevTools.chromeWindowType) { From dffd5a7984d604226a78cd518a382d3781b5c25a Mon Sep 17 00:00:00 2001 From: Martin Stransky Date: Wed, 28 Aug 2019 02:13:03 +0000 Subject: [PATCH 39/41] Bug 1576910 - [Wayland] Add SurfaceDescriptorDMABuf to enable dma buf surface import/export on Wayland and (potentially) X11, r=sotaro Differential Revision: https://phabricator.services.mozilla.com/D43611 --HG-- extra : moz-landing-system : lando --- gfx/layers/ipc/LayersSurfaces.ipdlh | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/gfx/layers/ipc/LayersSurfaces.ipdlh b/gfx/layers/ipc/LayersSurfaces.ipdlh index c06bcd83ee34..8b2a21d418f8 100644 --- a/gfx/layers/ipc/LayersSurfaces.ipdlh +++ b/gfx/layers/ipc/LayersSurfaces.ipdlh @@ -58,6 +58,16 @@ struct SurfaceDescriptorMacIOSurface { YUVColorSpace yUVColorSpace; }; +struct SurfaceDescriptorDMABuf { + uint32_t width; + uint32_t height; + uint32_t format; + uint32_t flags; + FileDescriptor fd; + uint32_t stride; + uint32_t offset; +}; + struct SurfaceTextureDescriptor { uint64_t handle; IntSize size; @@ -148,6 +158,7 @@ union SurfaceDescriptor { SurfaceDescriptorFileMapping; SurfaceDescriptorDXGIYCbCr; SurfaceDescriptorX11; + SurfaceDescriptorDMABuf; SurfaceTextureDescriptor; EGLImageDescriptor; SurfaceDescriptorMacIOSurface; From fd573a0948e96f1243d5dda3f75a2a7d79122b8d Mon Sep 17 00:00:00 2001 From: Cameron Kaiser Date: Wed, 28 Aug 2019 05:03:54 +0000 Subject: [PATCH 40/41] Bug 817058 - VMX acceleration for nsTextFragment. r=bzbarsky Differential Revision: https://phabricator.services.mozilla.com/D43566 --HG-- extra : moz-landing-system : lando --- dom/base/moz.build | 6 ++ dom/base/nsTextFragment.cpp | 13 +++++ dom/base/nsTextFragmentVMX.cpp | 100 +++++++++++++++++++++++++++++++++ 3 files changed, 119 insertions(+) create mode 100644 dom/base/nsTextFragmentVMX.cpp diff --git a/dom/base/moz.build b/dom/base/moz.build index 0cbdb0e73433..d6ace9259ad0 100644 --- a/dom/base/moz.build +++ b/dom/base/moz.build @@ -468,6 +468,12 @@ if CONFIG['INTEL_ARCHITECTURE']: SOURCES += ['nsTextFragmentSSE2.cpp'] SOURCES['nsTextFragmentSSE2.cpp'].flags += CONFIG['SSE2_FLAGS'] +# Are we targeting PowerPC? If so, we can enable a SIMD version for +# nsTextFragment.cpp as well. +if CONFIG['CPU_ARCH'].startswith('ppc'): + SOURCES += ['nsTextFragmentVMX.cpp'] + SOURCES['nsTextFragmentVMX.cpp'].flags += CONFIG['PPC_VMX_FLAGS'] + EXTRA_JS_MODULES += [ 'ContentAreaDropListener.jsm', 'DOMRequestHelper.jsm', diff --git a/dom/base/nsTextFragment.cpp b/dom/base/nsTextFragment.cpp index 6276e422b2d9..8b33db4df967 100644 --- a/dom/base/nsTextFragment.cpp +++ b/dom/base/nsTextFragment.cpp @@ -19,6 +19,7 @@ #include "mozilla/CheckedInt.h" #include "mozilla/MemoryReporting.h" #include "mozilla/SSE.h" +#include "mozilla/ppc.h" #include "nsTextFragmentImpl.h" #include @@ -166,6 +167,14 @@ int32_t FirstNon8Bit(const char16_t* str, const char16_t* end); } // namespace mozilla #endif +#ifdef __powerpc__ +namespace mozilla { +namespace VMX { +int32_t FirstNon8Bit(const char16_t* str, const char16_t* end); +} // namespace VMX +} // namespace mozilla +#endif + /* * This function returns -1 if all characters in str are 8 bit characters. * Otherwise, it returns a value less than or equal to the index of the first @@ -178,6 +187,10 @@ static inline int32_t FirstNon8Bit(const char16_t* str, const char16_t* end) { if (mozilla::supports_sse2()) { return mozilla::SSE2::FirstNon8Bit(str, end); } +#elif defined(__powerpc__) + if (mozilla::supports_vmx()) { + return mozilla::VMX::FirstNon8Bit(str, end); + } #endif return FirstNon8BitUnvectorized(str, end); diff --git a/dom/base/nsTextFragmentVMX.cpp b/dom/base/nsTextFragmentVMX.cpp new file mode 100644 index 000000000000..d359c5cf2a00 --- /dev/null +++ b/dom/base/nsTextFragmentVMX.cpp @@ -0,0 +1,100 @@ +/* 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/. */ + +// This file should only be compiled if you're on Power ISA. + +#include "nscore.h" +#include "nsAlgorithm.h" +#include "nsTextFragmentImpl.h" +#include + +namespace mozilla { +namespace VMX { + +int32_t FirstNon8Bit(const char16_t* str, const char16_t* end) { + const uint32_t numUnicharsPerVector = 8; + const uint32_t numCharsPerVector = 16; + // Paranoia. If this assertion is wrong, change the vector loop below. + MOZ_ASSERT((numCharsPerVector / numUnicharsPerVector) == sizeof(char16_t)); + + typedef Non8BitParameters p; + const uint32_t alignMask = p::alignMask(); + const size_t mask = p::mask(); + const uint32_t numUnicharsPerWord = p::numUnicharsPerWord(); + + const uint32_t len = end - str; + + // i shall count the index in unichars; i2 shall count the index in chars. + uint32_t i = 0; + uint32_t i2 = 0; + + // Align ourselves to a 16-byte boundary, as required by VMX loads. + uint32_t alignLen = std::min( + len, uint32_t(((-NS_PTR_TO_UINT32(str)) & 0xf) / sizeof(char16_t))); + + if ((len - alignLen) >= numUnicharsPerVector) { + for (; i < alignLen; i++) { + if (str[i] > 255) return i; + } + + // Construct a vector of shorts. +#if __LITTLE_ENDIAN__ + register const vector unsigned short gtcompare = + reinterpret_cast( + vec_mergel(vec_splat_s8(-1), vec_splat_s8(0))); +#else + register const vector unsigned short gtcompare = + reinterpret_cast( + vec_mergel(vec_splat_s8(0), vec_splat_s8(-1))); +#endif + const uint32_t vectWalkEnd = + ((len - i) / numUnicharsPerVector) * numUnicharsPerVector; + i2 = i * sizeof(char16_t); + + while (1) { + register vector unsigned short vect; + + // Check one VMX register (8 unichars) at a time. The vec_any_gt + // intrinsic does exactly what we want. This loop is manually unrolled; + // it yields notable performance improvements this way. +#define CheckForASCII \ + vect = vec_ld(i2, reinterpret_cast(str)); \ + if (vec_any_gt(vect, gtcompare)) return i; \ + i += numUnicharsPerVector; \ + if (!(i < vectWalkEnd)) break; \ + i2 += numCharsPerVector; + + CheckForASCII CheckForASCII + +#undef CheckForASCII + } + } else { + // Align ourselves to a word boundary. + alignLen = std::min(len, uint32_t(((-NS_PTR_TO_UINT32(str)) & alignMask) / + sizeof(char16_t))); + for (; i < alignLen; i++) { + if (str[i] > 255) return i; + } + } + + // Check one word at a time. + const uint32_t wordWalkEnd = + ((len - i) / numUnicharsPerWord) * numUnicharsPerWord; + for (; i < wordWalkEnd; i += numUnicharsPerWord) { + const size_t word = *reinterpret_cast(str + i); + if (word & mask) return i; + } + + // Take care of the remainder one character at a time. + for (; i < len; i++) { + if (str[i] > 255) { + return i; + } + } + + return -1; +} + +} // namespace VMX +} // namespace mozilla From 451bc0629cd8e00d190465d19a4e33b51cf166a2 Mon Sep 17 00:00:00 2001 From: Nicolas Chevobbe Date: Wed, 28 Aug 2019 06:37:18 +0000 Subject: [PATCH 41/41] Bug 1572671 - Don't load content process server startup script if it was already loaded. r=ochameau. Previously, we were simply having a flag on the ContentProcessConnector that we would flip once we loaded the content process server startup script. This was working fine until multi-proxy browser console. Each time we open the Browser Console, we create a new loader in a new compartment. Which means in the end we'll have a new ContentProcessConnector instance, and the flag would be resetted, and thus we would load a new content process server startup script, which finally, would emit some packet twice, and thus would break how we manage new packet in the client. This patch fixes that by replacing the flag by checking if the script was already loaded. Differential Revision: https://phabricator.services.mozilla.com/D43618 --HG-- extra : moz-landing-system : lando --- .../server/connectors/content-process-connector.js | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/devtools/server/connectors/content-process-connector.js b/devtools/server/connectors/content-process-connector.js index ab2c164862e8..a5867eaeb106 100644 --- a/devtools/server/connectors/content-process-connector.js +++ b/devtools/server/connectors/content-process-connector.js @@ -21,9 +21,6 @@ const CONTENT_PROCESS_SERVER_STARTUP_SCRIPT = loader.lazyRequireGetter(this, "EventEmitter", "devtools/shared/event-emitter"); const ContentProcessConnector = { - // Flag to check if the content process server startup script was already loaded. - _contentProcessServerStartupScriptLoaded: false, - /** * Start a DevTools server in a content process (representing the entire process, not * just a single frame) and add it as a child server for an active connection. @@ -36,8 +33,6 @@ const ContentProcessConnector = { mm.addMessageListener("debug:content-process-actor", function listener( msg ) { - // Arbitrarily choose the first content process to reply - // XXX: This code needs to be updated if we use more than one content process mm.removeMessageListener("debug:content-process-actor", listener); // Pipe Debugger message from/to parent/child via the message manager @@ -58,13 +53,15 @@ const ContentProcessConnector = { }); // Load the content process server startup script only once. - if (!this._contentProcessServerStartupScriptLoaded) { + const isContentProcessServerStartupScripLoaded = Services.ppmm + .getDelayedProcessScripts() + .some(([uri]) => uri === CONTENT_PROCESS_SERVER_STARTUP_SCRIPT); + if (!isContentProcessServerStartupScripLoaded) { // Load the process script that will receive the debug:init-content-server message Services.ppmm.loadProcessScript( CONTENT_PROCESS_SERVER_STARTUP_SCRIPT, true ); - this._contentProcessServerStartupScriptLoaded = true; } // Send a message to the content process server startup script to forward it the