| ☠☠ backed out by 56036a1ba781 ☠ ☠ | |
| author | Emilio Cobos Álvarez <emilio@crisal.io> |
| Mon, 11 Nov 2019 13:28:52 +0000 | |
| changeset 501488 | e19c5df398de58747cceb53d36db418d1d63f8e3 |
| parent 501487 | 07a300e0fe29815262e1f562e0cee5fc04762cbd |
| child 501489 | e847a6cb715f4ab4dbc0db13ec1bbad421809259 |
| push id | 100233 |
| push user | ealvarez@mozilla.com |
| push date | Mon, 11 Nov 2019 16:01:12 +0000 |
| treeherder | autoland@e19c5df398de [default view] [failures only] |
| perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
| reviewers | mak |
| bugs | 1595285 |
| milestone | 72.0a1 |
| first release with | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
| last release without | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
--- a/dom/html/test/test_bug209275.xhtml +++ b/dom/html/test/test_bug209275.xhtml @@ -115,21 +115,22 @@ function* run() { // everything from the location up to and including the final forward slash var path = /(.*\/)[^\/]*/.exec(location)[1]; // Set colorlink's href so we can check that it changes colors after we // change the base href. $('colorlink').href = "http://example.com/" + rand; setXlinkHref($("ellipselink"), "http://example.com/" + rand); - // Load http://example.com/${rand} into an iframe so we can test that changing - // the document's base changes the visitedness of our links. - iframe.onload = continueTest; - iframeCw.location = "http://example.com/" + rand; - yield undefined; // wait for onload to fire. + // Load http://example.com/${rand} into a new window so we can test that + // changing the document's base changes the visitedness of our links. + // + // cross-origin window.open'd windows don't fire load / error events, so we + // wait to close it until we observed the visited color. + let win = window.open("http://example.com/" + rand, "_blank"); // Make sure things are what as we expect them at the beginning. link123HrefIs("http://mochi.test:8888/", 1); is($('link4').href, loc + "#", "link 4 test 1"); is($('link5').href, loc + "#", "link 5 test 1"); // Remove link5 from the document. We're going to test that its href changes // properly when we change our base. @@ -142,28 +143,30 @@ function* run() { link123HrefIs("http://example.com/", 2); is($('link4').href, "http://example.com/#", "link 4 test 2"); is(link5.href, "http://example.com/#", "link 5 test 2"); // Were colorlink's color and ellipse's fill updated appropriately? // Because link coloring is asynchronous, we wait until it is updated (or we // timeout and fail anyway). while (getColor($('colorlink')) != visitedColor) { - setTimeout(continueTest, 0); + requestIdleCallback(continueTest); yield undefined; } is(getColor($('colorlink')), visitedColor, "Wrong link color after base change."); while (getFill($('ellipselink')) != visitedFill) { - setTimeout(continueTest, 0); + requestIdleCallback(continueTest); yield undefined; } is(getFill($('ellipselink')), visitedFill, "Wrong ellipse fill after base change."); + win.close(); + $('base1').href = "foo/"; // Should be interpreted relative to current URI (not the current base), so // base should now be http://mochi.test:8888/foo/ link123HrefIs("http://mochi.test:8888/", 3); is($('link4').href, path + "foo/#", "link 4 test 3"); // Changing base2 shouldn't affect anything, because it's not the first base @@ -215,17 +218,16 @@ function* run() { is($('link4').href, loc + "#", "link 4 test 10"); // We load into an iframe a document with a <base href="...">, then remove // the document element. Then we add an <html>, <body>, and <a>, and make // sure that the <a> is resolved relative to the page's location, not its // original base. We do this twice, rebuilding the document in a different // way each time. - iframe.onload = null; iframeCw.location = "file_bug209275_1.html"; yield undefined; // wait for our child to call us back. is(iframeCw.document.getElementById("link").href, path + "file_bug209275_1.html#", "Wrong href after nuking document."); iframeCw.location = "file_bug209275_2.html"; yield undefined; // wait for callback from child
--- a/dom/html/test/test_bug481335.xhtml +++ b/dom/html/test/test_bug481335.xhtml @@ -49,70 +49,74 @@ function continueTest() { function checkLinkColor(aElmId, aExpectedColor, aMessage) { // Because link coloring is asynchronous, we wait until we get the right // result, or we will time out (resulting in a failure). function getColor() { var utils = SpecialPowers.getDOMWindowUtils(window); return utils.getVisitedDependentComputedStyle($(aElmId), "", "color"); } while (getColor() != aExpectedColor) { - setTimeout(continueTest, 0); + requestIdleCallback(continueTest); return false; } is(getColor(), aExpectedColor, aMessage); return true; } +let win; + function* testIterator() { // After first load $("newparent").appendChild($("t")); is($("t").href, "http://www.example.com/" + rand, "Unexpected href after move"); is($("t").href, "http://www.example.com/" + rand, "Unexpected cached href after move"); while (!checkLinkColor("t", unvisitedColor, "Should be unvisited now")) yield undefined; - $("i").src = $("t").href; - yield undefined; + win.close(); + win = window.open($("t").href, "_blank"); // After second load while (!checkLinkColor("t", visitedColor, "Should be visited now")) yield undefined; $("t").pathname = rand; while (!checkLinkColor("t", visitedColor, "Should still be visited after setting pathname to its existing value")) { yield undefined; } + /* TODO uncomment this test with the landing of bug 534526. See * https://bugzilla.mozilla.org/show_bug.cgi?id=461199#c167 $("t").pathname += "x"; while (!checkLinkColor("t", unvisitedColor, "Should not be visited after changing pathname")) { yield undefined; } $("t").pathname = $("t").pathname; while (!checkLinkColor("t", unvisitedColor, "Should not be visited after setting unvisited pathname to existing value")) { yield undefined; } */ - $("i").src = $("t").href; - yield undefined; + win.close(); + win = window.open($("t").href, "_blank"); // After third load while (!checkLinkColor("t", visitedColor, "Should be visited now after third load")) { yield undefined; } + win.close(); SimpleTest.finish(); } addLoadEvent(function() { - $("i").onload = continueTest; - $("i").src = $("t").href; + win = window.open($("t").href, "_blank"); + requestIdleCallback(continueTest); }); ]]> </script> </pre> </body> </html>
--- a/layout/reftests/css-visited/color-on-visited-text-1.html +++ b/layout/reftests/css-visited/color-on-visited-text-1.html @@ -9,12 +9,12 @@ } :visited { color: purple; } .outer::first-line { color: green; } </style> -<a class="outer" href>Visited</a> -<a class="outer" href><span>Visited with span</span></a> -<a class="outer" href><a href="unvisited-page.html">Visited with inner unvisited</a></a> -<a class="outer" href><a href>Visited with inner visited</a></a> +<a class="outer" href="visited-page.html">Visited</a> +<a class="outer" href="visited-page.html"><span>Visited with span</span></a> +<a class="outer" href="visited-page.html"><a href="unvisited-page.html">Visited with inner unvisited</a></a> +<a class="outer" href="visited-page.html"><a href="visited-page.html">Visited with inner visited</a></a>
--- a/layout/reftests/css-visited/link-root-1.xhtml +++ b/layout/reftests/css-visited/link-root-1.xhtml @@ -12,15 +12,15 @@ a { display: block; width: 100%; height: ]]> </style> <script> <![CDATA[ var a = document.documentElement; getComputedStyle(a, "").backgroundColor; // flush style -a.href = ""; +a.href = "visited-page.html"; getComputedStyle(a, "").backgroundColor; // flush style document.documentElement.removeAttribute("class"); ]]> </script> </a>
--- a/layout/reftests/css-visited/mathml-links.html +++ b/layout/reftests/css-visited/mathml-links.html @@ -2,17 +2,17 @@ <html> <head> <style> :link { color: red; } :visited { color: green; } </style> </head> <body> - <math href="" display="block"> + <math href="visited-page.html" display="block"> <mi>x</mi> <mo>=</mo> <mfrac> <mrow> <msqrt> <mn>5</mn> </msqrt> </mrow>
--- a/layout/reftests/css-visited/svg-paint-currentcolor-visited.svg +++ b/layout/reftests/css-visited/svg-paint-currentcolor-visited.svg @@ -7,10 +7,10 @@ </metadata> <style> a:link { color: red; } a:visited { color: green; } rect { fill: currentcolor; stroke: currentcolor; stroke-width: 20px; } </style> - <a href=""><rect x="10" y="10" width="80" height="80"/></a> + <a href="visited-page.html"><rect x="10" y="10" width="80" height="80"/></a> </svg>
--- a/layout/reftests/svg/as-image/svg-image-visited-1a-helper.svg +++ b/layout/reftests/svg/as-image/svg-image-visited-1a-helper.svg @@ -9,20 +9,20 @@ <style> <![CDATA[ a:link {/* Note: an a:link block was needed to trigger bug 641731. */ } a:link > rect { fill: lime; } a:visited > rect { fill: purple; } ]]> </style> - <!-- Note: the <a> element below links to _this document_, so it'll + <!-- Note: the <a> element below links to a visited page, so it'll normally be treated as visited. However, in an image context, we want to ignore visitedness. --> - <a xlink:href="" id="foo"> + <a xlink:href="visited-page.html" id="foo"> <rect x="0" y="0" width="100" height="100" fill="orange"/> </a> <!-- This trivial SMIL animation ensures that we *won't* get repainted via imagelib's SurfaceCache optimization. Specifically, we want to bypass the SurfaceCache so that we can ensure that repaints of this file (as an image) will *actually repaint the SVG content*, rather than painting a previously-rasterized snapshot (which may've been rasterized before we
--- a/layout/reftests/svg/as-image/svg-image-visited-1b-helper.svg +++ b/layout/reftests/svg/as-image/svg-image-visited-1b-helper.svg @@ -8,20 +8,20 @@ height="100" width="100"> <style> <![CDATA[ a:link > rect { fill: lime; } a:visited > rect { fill: purple; } ]]> </style> - <!-- Note: the <a> element below links to _this document_, so it'll + <!-- Note: the <a> element below links to a visited page, so it'll normally be treated as visited. However, in an image context, we want to ignore visitedness. --> - <a xlink:href="" id="foo"> + <a xlink:href="visited-page.html" id="foo"> <rect x="0" y="0" width="100" height="100" fill="orange"/> </a> <!-- This trivial SMIL animation ensures that we *won't* get repainted via imagelib's SurfaceCache optimization. Specifically, we want to bypass the SurfaceCache so that we can ensure that repaints of this file (as an image) will *actually repaint the SVG content*, rather than painting a previously-rasterized snapshot (which may've been rasterized before we
--- a/layout/reftests/svg/pseudo-classes-02.svg +++ b/layout/reftests/svg/pseudo-classes-02.svg @@ -22,27 +22,27 @@ a:visited > tspan { fill: lime; } </style> <!-- link in text --> <text x="10" y="25"> <a xlink:href="do-not-visit-me.xxx" fill="red">This should be green</a> </text> <!-- text in link --> - <a xlink:href=""> + <a xlink:href="visited-page.html"> <text x="10" y="50" fill="red">This should be green</text> </a> <!-- link in tspan --> <text> <tspan x="10" y="75"> <a xlink:href="do-not-visit-me.xxx" fill="red">This should be green</a> </tspan> </text> <!-- tspan in link --> <text> - <a xlink:href=""> + <a xlink:href="visited-page.html"> <tspan x="10" y="100" fill="red">This should be green</tspan> </a> </text> </svg>
--- a/layout/style/test/test_visited_image_loading.html +++ b/layout/style/test/test_visited_image_loading.html @@ -5,36 +5,36 @@ https://bugzilla.mozilla.org/show_bug.cg --> <head> <title>Test for Bug 557287</title> <script src="/tests/SimpleTest/SimpleTest.js"></script> <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/> </head> <body> <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=557287">Mozilla Bug 147777</a> -<iframe id="display" src="visited_image_loading_frame.html"></iframe> <pre id="test"> <script type="application/ecmascript" src="visited_image_loading.sjs?reset"></script> <script type="application/javascript"> /** Test for Bug 557287 **/ SimpleTest.waitForExplicitFinish(); SimpleTest.requestFlakyTimeout("untriaged"); var subdoc, subwin; window.addEventListener("load", run); function run() { - var frame = document.getElementById("display"); - subdoc = frame.contentDocument; - subwin = frame.contentWindow; - setTimeout(check_link_styled, 50); + subwin = window.open("visited_image_loading_frame.html", "_blank"); + subwin.addEventListener("load", function() { + subdoc = subwin.document; + setTimeout(check_link_styled, 50); + }); } function visitedDependentComputedStyle(win, elem, property) { return SpecialPowers.DOMWindowUtils .getVisitedDependentComputedStyle(elem, "", property); } function check_link_styled() @@ -54,14 +54,15 @@ function check_link_styled() } function paint_listener(event) { subwin.removeEventListener("MozAfterPaint", paint_listener); var s = document.createElement("script"); s.src = "visited_image_loading.sjs?waitforresult"; document.body.appendChild(s); + subwin.close(); } </script> </pre> </body> </html>
--- a/layout/style/test/test_visited_image_loading_empty.html +++ b/layout/style/test/test_visited_image_loading_empty.html @@ -5,36 +5,36 @@ https://bugzilla.mozilla.org/show_bug.cg --> <head> <title>Test for Bug 557287</title> <script src="/tests/SimpleTest/SimpleTest.js"></script> <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/> </head> <body> <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=557287">Mozilla Bug 147777</a> -<iframe id="display" src="visited_image_loading_frame_empty.html"></iframe> <pre id="test"> <script type="application/ecmascript" src="visited_image_loading.sjs?reset"></script> <script type="application/javascript"> /** Test for Bug 557287 **/ SimpleTest.waitForExplicitFinish(); SimpleTest.requestFlakyTimeout("untriaged"); var subdoc, subwin; window.addEventListener("load", run); function run() { - var frame = document.getElementById("display"); - subdoc = frame.contentDocument; - subwin = frame.contentWindow; - setTimeout(check_link_styled, 50); + subwin = window.open("visited_image_loading_frame_empty.html", "_blank"); + subwin.addEventListener("load", function() { + subdoc = subwin.document; + setTimeout(check_link_styled, 50); + }); } function visitedDependentComputedStyle(win, elem, property) { return SpecialPowers.DOMWindowUtils .getVisitedDependentComputedStyle(elem, "", property); } function check_link_styled() @@ -54,14 +54,15 @@ function check_link_styled() } function paint_listener(event) { subwin.removeEventListener("MozAfterPaint", paint_listener); var s = document.createElement("script"); s.src = "visited_image_loading.sjs?waitforresult"; document.body.appendChild(s); + subwin.close(); } </script> </pre> </body> </html>
--- a/layout/style/test/test_visited_lying.html +++ b/layout/style/test/test_visited_lying.html @@ -31,17 +31,17 @@ function start() iframe = document.getElementById("iframe"); visitedlink = iframe.contentDocument.getElementById("visitedlink"); unvisitedlink = iframe.contentDocument.getElementById("unvisitedlink"); // First, take a snapshot of it with both links unvisited. snapshot1 = snapshotWindow(iframe.contentWindow, false); // Then, change one of the links in the iframe to being visited. - visitedlink.href = window.location; + visitedlink.href = top.location; // Then, start polling to see when the history has updated the display. setTimeout(poll_for_restyle, 100); } function poll_for_restyle() { var snapshot2 = snapshotWindow(iframe.contentWindow, false);
--- a/layout/style/test/test_visited_pref.html +++ b/layout/style/test/test_visited_pref.html @@ -58,17 +58,17 @@ function step1() iframe = document.getElementById("iframe"); subdoc = iframe.contentDocument; subwin = iframe.contentWindow; link = subdoc.getElementById("link"); unvisref = snapshotWindow(subwin, false); // Now set the href of the link to a location that's actually visited. - link.href = window.location; + link.href = top.location; start = Date.now(); // And wait for the link to get restyled when the history lets us // know it is (asynchronously). setTimeout(poll_for_visited_style, 100); }
--- a/layout/style/test/test_visited_reftests.html +++ b/layout/style/test/test_visited_reftests.html @@ -123,18 +123,24 @@ function startIframe(url) { }); } async function runTests() { SimpleTest.waitForExplicitFinish(); SimpleTest.requestFlakyTimeout("async link coloring"); // Set caret to a known size, for tests of :visited caret-color styling await SpecialPowers.pushPrefEnv({'set': [['ui.caretWidth', 16]]}); - await startIframe("visited-page.html"); + info("opening visited page"); + let win = window.open("css-visited/visited-page.html", "_blank"); + await new Promise(resolve => { + win.onload = resolve; + }); + info("running tests"); await Promise.all(gTests.map(runTest)); + win.close(); SimpleTest.finish(); } function passes(equal, shot1, shot2) { let [correct] = compareSnapshots(shot1, shot2, equal); return correct; }
--- a/toolkit/components/places/History.cpp +++ b/toolkit/components/places/History.cpp @@ -373,28 +373,16 @@ class VisitedQuery final : public AsyncS mozIVisitedStatusCallback* aCallback = nullptr) { MOZ_ASSERT(aURI, "Null URI"); MOZ_ASSERT(XRE_IsParentProcess()); nsMainThreadPtrHandle<mozIVisitedStatusCallback> callback( new nsMainThreadPtrHolder<mozIVisitedStatusCallback>( "mozIVisitedStatusCallback", aCallback)); - nsNavHistory* navHistory = nsNavHistory::GetHistoryService(); - NS_ENSURE_STATE(navHistory); - if (navHistory->hasEmbedVisit(aURI)) { - RefPtr<VisitedQuery> query = new VisitedQuery(aURI, callback, true); - // As per IHistory contract, we must notify asynchronously. - NS_DispatchToMainThread( - NewRunnableMethod("places::VisitedQuery::NotifyVisitedStatus", query, - &VisitedQuery::NotifyVisitedStatus)); - - return NS_OK; - } - History* history = History::GetService(); NS_ENSURE_STATE(history); RefPtr<VisitedQuery> query = new VisitedQuery(aURI, callback); return history->QueueVisitedStatement(std::move(query)); } void Execute(mozIStorageAsyncStatement& aStatement) { // Bind by index for performance. @@ -462,19 +450,18 @@ class VisitedQuery final : public AsyncS (void)observerService->NotifyObservers(mURI, URI_VISITED_RESOLUTION_TOPIC, status); } } private: explicit VisitedQuery( nsIURI* aURI, - const nsMainThreadPtrHandle<mozIVisitedStatusCallback>& aCallback, - bool aIsVisited = false) - : mURI(aURI), mCallback(aCallback), mIsVisited(aIsVisited) {} + const nsMainThreadPtrHandle<mozIVisitedStatusCallback>& aCallback) + : mURI(aURI), mCallback(aCallback), mIsVisited(false) {} ~VisitedQuery() {} nsCOMPtr<nsIURI> mURI; nsMainThreadPtrHandle<mozIVisitedStatusCallback> mCallback; bool mIsVisited; }; @@ -1370,33 +1357,32 @@ class SetPageTitle : public Runnable { /** * Stores an embed visit, and notifies observers. * * @param aPlace * The VisitData of the visit to store as an embed visit. * @param [optional] aCallback * The mozIVisitInfoCallback to notify, if provided. + * + * FIXME(emilio, bug 1595484): We should get rid of EMBED visits completely. */ -void StoreAndNotifyEmbedVisit(VisitData& aPlace, - mozIVisitInfoCallback* aCallback = nullptr) { +void NotifyEmbedVisit(VisitData& aPlace, + mozIVisitInfoCallback* aCallback = nullptr) { MOZ_ASSERT(aPlace.transitionType == nsINavHistoryService::TRANSITION_EMBED, "Must only pass TRANSITION_EMBED visits to this!"); MOZ_ASSERT(NS_IsMainThread(), "Must be called on the main thread!"); nsCOMPtr<nsIURI> uri; MOZ_ALWAYS_SUCCEEDS(NS_NewURI(getter_AddRefs(uri), aPlace.spec)); - nsNavHistory* navHistory = nsNavHistory::GetHistoryService(); - if (!navHistory || !uri) { + if (!uri) { return; } - navHistory->registerEmbedVisit(uri, aPlace.visitTime); - if (!!aCallback) { nsMainThreadPtrHandle<mozIVisitInfoCallback> callback( new nsMainThreadPtrHolder<mozIVisitInfoCallback>( "mozIVisitInfoCallback", aCallback)); bool ignoreResults = false; Unused << aCallback->GetIgnoreResults(&ignoreResults); if (!ignoreResults) { nsCOMPtr<nsIRunnable> event = @@ -1983,20 +1969,20 @@ History::VisitURI(nsIWidget* aWidget, ns // This can happen for example at a page redirecting to itself. if (!wasHidden || place.hidden) { // We can skip this visit. return NS_OK; } } } - // EMBED visits are session-persistent and should not go through the database. + // EMBED visits should not go through the database. // They exist only to keep track of isVisited status during the session. if (place.transitionType == nsINavHistoryService::TRANSITION_EMBED) { - StoreAndNotifyEmbedVisit(place); + NotifyEmbedVisit(place); } else { mozIStorageConnection* dbConn = GetDBConn(); NS_ENSURE_STATE(dbConn); rv = InsertVisitedURIs::Start(dbConn, placeArray); NS_ENSURE_SUCCESS(rv, rv); } @@ -2038,28 +2024,20 @@ History::SetURITitle(nsIURI* aURI, const bool canAdd; nsresult rv = navHistory->CanAddURI(aURI, &canAdd); NS_ENSURE_SUCCESS(rv, rv); if (!canAdd) { return NS_OK; } - // Embed visits don't have a database entry, thus don't set a title on them. - if (navHistory->hasEmbedVisit(aURI)) { - return NS_OK; - } - mozIStorageConnection* dbConn = GetDBConn(); NS_ENSURE_STATE(dbConn); - rv = SetPageTitle::Start(dbConn, aURI, aTitle); - NS_ENSURE_SUCCESS(rv, rv); - - return NS_OK; + return SetPageTitle::Start(dbConn, aURI, aTitle); } //////////////////////////////////////////////////////////////////////////////// //// mozIAsyncHistory NS_IMETHODIMP History::UpdatePlaces(JS::Handle<JS::Value> aPlaceInfos, mozIVisitInfoCallback* aCallback, @@ -2170,17 +2148,17 @@ History::UpdatePlaces(JS::Handle<JS::Val NS_ENSURE_ARG_RANGE(transitionType, nsINavHistoryService::TRANSITION_LINK, nsINavHistoryService::TRANSITION_RELOAD); data.SetTransitionType(transitionType); data.hidden = GetHiddenState(false, transitionType); // If the visit is an embed visit, we do not actually add it to the // database. if (transitionType == nsINavHistoryService::TRANSITION_EMBED) { - StoreAndNotifyEmbedVisit(data, aCallback); + NotifyEmbedVisit(data, aCallback); visitData.RemoveLastElement(); initialUpdatedCount++; continue; } // The referrer is optional. nsCOMPtr<nsIURI> referrer = GetURIFromJSObject(aCtx, visit, "referrerURI");
--- a/toolkit/components/places/History.jsm +++ b/toolkit/components/places/History.jsm @@ -903,19 +903,16 @@ var clear = async function(db) { WHEN url_hash BETWEEN hash("place", "prefix_lo") AND hash("place", "prefix_hi") THEN 0 ELSE -1 END) WHERE frecency > 0`); }); - // Clear the registered embed visits. - PlacesUtils.history.clearEmbedVisits(); - let observers = PlacesUtils.history.getObservers(); notify(observers, "onClearHistory"); // Notify frecency change observers. notify(observers, "onManyFrecenciesChanged"); // Trigger frecency updates for all affected origins. await db.execute(`DELETE FROM moz_updateoriginsupdate_temp`); }; @@ -1272,72 +1269,67 @@ var removeVisitsByFilter = async functio onResultData.push({ date: new Date(row.getResultByName("date")), transition: row.getResultByName("visit_type"), }); } } ); - try { - if (!visitsToRemove.length) { - // Nothing to do - return false; + if (!visitsToRemove.length) { + // Nothing to do + return false; + } + + let pages = []; + await db.executeTransaction(async function() { + // 2. Remove all offending visits. + for (let chunk of PlacesUtils.chunkArray( + visitsToRemove, + db.variableLimit + )) { + await db.execute( + `DELETE FROM moz_historyvisits + WHERE id IN (${sqlBindPlaceholders(chunk)})`, + chunk + ); } - let pages = []; - await db.executeTransaction(async function() { - // 2. Remove all offending visits. - for (let chunk of PlacesUtils.chunkArray( - visitsToRemove, - db.variableLimit - )) { - await db.execute( - `DELETE FROM moz_historyvisits - WHERE id IN (${sqlBindPlaceholders(chunk)})`, - chunk - ); - } + // 3. Find out which pages have been orphaned + for (let chunk of PlacesUtils.chunkArray( + [...pagesToInspect], + db.variableLimit + )) { + await db.execute( + `SELECT id, url, url_hash, guid, + (foreign_count != 0) AS has_foreign, + (last_visit_date NOTNULL) as has_visits + FROM moz_places + WHERE id IN (${sqlBindPlaceholders(chunk)})`, + chunk, + row => { + let page = { + id: row.getResultByName("id"), + guid: row.getResultByName("guid"), + hasForeign: row.getResultByName("has_foreign"), + hasVisits: row.getResultByName("has_visits"), + url: new URL(row.getResultByName("url")), + hash: row.getResultByName("url_hash"), + }; + pages.push(page); + } + ); + } - // 3. Find out which pages have been orphaned - for (let chunk of PlacesUtils.chunkArray( - [...pagesToInspect], - db.variableLimit - )) { - await db.execute( - `SELECT id, url, url_hash, guid, - (foreign_count != 0) AS has_foreign, - (last_visit_date NOTNULL) as has_visits - FROM moz_places - WHERE id IN (${sqlBindPlaceholders(chunk)})`, - chunk, - row => { - let page = { - id: row.getResultByName("id"), - guid: row.getResultByName("guid"), - hasForeign: row.getResultByName("has_foreign"), - hasVisits: row.getResultByName("has_visits"), - url: new URL(row.getResultByName("url")), - hash: row.getResultByName("url_hash"), - }; - pages.push(page); - } - ); - } + // 4. Clean up and notify + await cleanupPages(db, pages); + }); - // 4. Clean up and notify - await cleanupPages(db, pages); - }); - - notifyCleanup(db, pages, transition); - notifyOnResult(onResultData, onResult); // don't wait - } finally { - // Ensure we cleanup embed visits, even if we bailed out early. - PlacesUtils.history.clearEmbedVisits(); - } + notifyCleanup(db, pages, transition); + notifyOnResult(onResultData, onResult); // don't wait return !!visitsToRemove.length; }; // Inner implementation of History.removeByFilter var removeByFilter = async function(db, filter, onResult = null) { // 1. Create fragment for date filtration let dateFilterSQLFragment = ""; @@ -1418,36 +1410,32 @@ var removeByFilter = async function(db, } }); if (pages.length === 0) { // Nothing to do return false; } - try { - await db.executeTransaction(async function() { - // 4. Actually remove visits - let pageIds = pages.map(p => p.id); - for (let chunk of PlacesUtils.chunkArray(pageIds, db.variableLimit)) { - await db.execute( - `DELETE FROM moz_historyvisits - WHERE place_id IN(${sqlBindPlaceholders(chunk)})`, - chunk - ); - } - // 5. Clean up and notify - await cleanupPages(db, pages); - }); + await db.executeTransaction(async function() { + // 4. Actually remove visits + let pageIds = pages.map(p => p.id); + for (let chunk of PlacesUtils.chunkArray(pageIds, db.variableLimit)) { + await db.execute( + `DELETE FROM moz_historyvisits + WHERE place_id IN(${sqlBindPlaceholders(chunk)})`, + chunk + ); + } + // 5. Clean up and notify + await cleanupPages(db, pages); + }); - notifyCleanup(db, pages); - notifyOnResult(onResultData, onResult); - } finally { - PlacesUtils.history.clearEmbedVisits(); - } + notifyCleanup(db, pages); + notifyOnResult(onResultData, onResult); return hasPagesToRemove; }; // Inner implementation of History.remove. var remove = async function(db, { guids, urls }, onResult = null) { // 1. Find out what needs to be removed let onResultData = onResult ? [] : null; @@ -1498,43 +1486,38 @@ var remove = async function(db, { guids, let query = `SELECT id, url, url_hash, guid, foreign_count, title, frecency FROM moz_places WHERE url_hash IN (${variables.map(v => `hash(${v})`).join(",")}) AND url IN (${variables.join(",")}) `; await db.execute(query, chunk, onRow); } - try { - if (!pages.length) { - // Nothing to do - return false; + if (!pages.length) { + // Nothing to do + return false; + } + + await db.executeTransaction(async function() { + // 2. Remove all visits to these pages. + let pageIds = pages.map(p => p.id); + for (let chunk of PlacesUtils.chunkArray(pageIds, db.variableLimit)) { + await db.execute( + `DELETE FROM moz_historyvisits + WHERE place_id IN (${sqlBindPlaceholders(chunk)})`, + chunk + ); } - await db.executeTransaction(async function() { - // 2. Remove all visits to these pages. - let pageIds = pages.map(p => p.id); - for (let chunk of PlacesUtils.chunkArray(pageIds, db.variableLimit)) { - await db.execute( - `DELETE FROM moz_historyvisits - WHERE place_id IN (${sqlBindPlaceholders(chunk)})`, - chunk - ); - } + // 3. Clean up and notify + await cleanupPages(db, pages); + }); - // 3. Clean up and notify - await cleanupPages(db, pages); - }); - - notifyCleanup(db, pages); - notifyOnResult(onResultData, onResult); // don't wait - } finally { - // Ensure we cleanup embed visits, even if we bailed out early. - PlacesUtils.history.clearEmbedVisits(); - } + notifyCleanup(db, pages); + notifyOnResult(onResultData, onResult); // don't wait return hasPagesToRemove; }; /** * Merges an updateInfo object, as returned by asyncHistory.updatePlaces * into a PageInfo object as defined in this file. *
--- a/toolkit/components/places/nsINavHistoryService.idl +++ b/toolkit/components/places/nsINavHistoryService.idl @@ -1253,21 +1253,16 @@ interface nsINavHistoryService : nsISupp /** * True if history is disabled. currently, * history is disabled if the places.history.enabled pref is false. */ readonly attribute boolean historyDisabled; /** - * Clear all TRANSITION_EMBED visits. - */ - void clearEmbedVisits(); - - /** * Generate a guid. * Guids can be used for any places purposes (history, bookmarks, etc.) * Returns null if the generation of the guid failed. */ ACString makeGuid(); /** * Returns a 48-bit hash for a URI spec.
--- a/toolkit/components/places/nsNavHistory.cpp +++ b/toolkit/components/places/nsNavHistory.cpp @@ -125,19 +125,16 @@ using namespace mozilla::places; // We use a guess of the number of months considering all of them 30 days // long, but we split only the last 6 months. #define HISTORY_DATE_CONT_NUM(_daysFromOldestVisit) \ (HISTORY_ADDITIONAL_DATE_CONT_NUM + \ std::min(6, (int32_t)ceilf((float)_daysFromOldestVisit / 30))) // Max number of containers, used to initialize the params hash. #define HISTORY_DATE_CONT_LENGTH 8 -// Initial length of the embed visits cache. -#define EMBED_VISITS_INITIAL_CACHE_LENGTH 64 - // Initial length of the recent events cache. #define RECENT_EVENTS_INITIAL_CACHE_LENGTH 64 // Observed topics. #define TOPIC_IDLE_DAILY "idle-daily" #define TOPIC_PREF_CHANGED "nsPref:changed" #define TOPIC_PROFILE_TEARDOWN "profile-change-teardown" #define TOPIC_PROFILE_CHANGE "profile-before-change" @@ -372,17 +369,16 @@ const int32_t nsNavHistory::kGetInfoInde PLACES_FACTORY_SINGLETON_IMPLEMENTATION(nsNavHistory, gHistoryService) nsNavHistory::nsNavHistory() : mCachedNow(0), mRecentTyped(RECENT_EVENTS_INITIAL_CACHE_LENGTH), mRecentLink(RECENT_EVENTS_INITIAL_CACHE_LENGTH), mRecentBookmark(RECENT_EVENTS_INITIAL_CACHE_LENGTH), - mEmbedVisits(EMBED_VISITS_INITIAL_CACHE_LENGTH), mHistoryEnabled(true), mNumVisitsForFrecency(10), mDecayFrecencyPendingCount(0), mTagsFolder(-1), mDaysOfHistory(-1), mLastCachedStartOfDay(INT64_MAX), mLastCachedEndOfDay(0), mCanNotify(true) @@ -2754,39 +2750,16 @@ nsresult nsNavHistory::FilterResultSet( if (aOptions->MaxResults() > 0 && (uint32_t)aFiltered->Count() >= aOptions->MaxResults()) break; } return NS_OK; } -void nsNavHistory::registerEmbedVisit(nsIURI* aURI, int64_t aTime) { - NS_ASSERTION(NS_IsMainThread(), "This can only be called on the main thread"); - - VisitHashKey* visit = mEmbedVisits.PutEntry(aURI); - if (!visit) { - NS_WARNING("Unable to register a EMBED visit."); - return; - } - visit->visitTime = aTime; -} - -bool nsNavHistory::hasEmbedVisit(nsIURI* aURI) { - NS_ASSERTION(NS_IsMainThread(), "This can only be called on the main thread"); - - return !!mEmbedVisits.GetEntry(aURI); -} - -NS_IMETHODIMP -nsNavHistory::ClearEmbedVisits() { - mEmbedVisits.Clear(); - return NS_OK; -} - NS_IMETHODIMP nsNavHistory::MakeGuid(nsACString& aGuid) { if (NS_FAILED(GenerateGUID(aGuid))) { MOZ_ASSERT(false, "Shouldn't fail to create a guid!"); aGuid.SetIsVoid(true); } return NS_OK; }
--- a/toolkit/components/places/nsNavHistory.h +++ b/toolkit/components/places/nsNavHistory.h @@ -264,26 +264,16 @@ class nsNavHistory final : public nsSupp /** * Whether there are visits. * Note: This may cause synchronous I/O. */ bool hasHistoryEntries(); /** - * Registers a TRANSITION_EMBED visit for the session. - * - * @param aURI - * URI of the page. - * @param aTime - * Visit time. Only the last registered visit time is retained. - */ - void registerEmbedVisit(nsIURI* aURI, int64_t aTime); - - /** * Returns whether the specified url has a embed visit. * * @param aURI * URI of the page. * @return whether the page has a embed visit. */ bool hasEmbedVisit(nsIURI* aURI); @@ -482,28 +472,16 @@ class nsNavHistory final : public nsSupp nsCOMPtr<nsICollation> mCollation; // recent events typedef nsDataHashtable<nsCStringHashKey, int64_t> RecentEventHash; RecentEventHash mRecentTyped; RecentEventHash mRecentLink; RecentEventHash mRecentBookmark; - // Embed visits tracking. - class VisitHashKey : public nsURIHashKey { - public: - explicit VisitHashKey(const nsIURI* aURI) : nsURIHashKey(aURI) {} - VisitHashKey(VisitHashKey&& aOther) : nsURIHashKey(std::move(aOther)) { - MOZ_ASSERT_UNREACHABLE("Do not call me!"); - } - PRTime visitTime; - }; - - nsTHashtable<VisitHashKey> mEmbedVisits; - bool CheckIsRecentEvent(RecentEventHash* hashTable, const nsACString& url); void ExpireNonrecentEvents(RecentEventHash* hashTable); // Whether history is enabled or not. // Will mimic value of the places.history.enabled preference. bool mHistoryEnabled; // Frecency preferences.
deleted file mode 100644 --- a/toolkit/components/places/tests/browser/461710_iframe.html +++ /dev/null @@ -1,8 +0,0 @@ -<!DOCTYPE HTML> -<html> - <head> - </head> - <body> - <iframe id="iframe"></iframe> - </body> -</html>
--- a/toolkit/components/places/tests/browser/browser.ini +++ b/toolkit/components/places/tests/browser/browser.ini @@ -7,17 +7,16 @@ support-files = 399606-history.go-0.html 399606-httprefresh.html 399606-location.reload.html 399606-location.replace.html 399606-window.location.html 399606-window.location.href.html [browser_bug461710.js] support-files = - 461710_iframe.html 461710_link_page-2.html 461710_link_page-3.html 461710_link_page.html 461710_visited_page.html [browser_bug646422.js] [browser_bug680727.js] skip-if = verify [browser_double_redirect.js]
--- a/toolkit/components/places/tests/browser/browser_bug461710.js +++ b/toolkit/components/places/tests/browser/browser_bug461710.js @@ -1,30 +1,20 @@ const kRed = "rgb(255, 0, 0)"; const kBlue = "rgb(0, 0, 255)"; const prefix = "http://example.com/tests/toolkit/components/places/tests/browser/461710_"; add_task(async function() { registerCleanupFunction(PlacesUtils.history.clear); - let contentPage = prefix + "iframe.html"; - let normalWindow = await BrowserTestUtils.openNewBrowserWindow(); - let normalBrowser = normalWindow.gBrowser.selectedBrowser; - await BrowserTestUtils.loadURI(normalBrowser, contentPage); - await BrowserTestUtils.browserLoaded(normalBrowser, false, contentPage); - let privateWindow = await BrowserTestUtils.openNewBrowserWindow({ private: true, }); - let privateBrowser = privateWindow.gBrowser.selectedBrowser; - BrowserTestUtils.loadURI(privateBrowser, contentPage); - await BrowserTestUtils.browserLoaded(privateBrowser, false, contentPage); - let tests = [ { private: false, topic: "uri-visit-saved", subtest: "visited_page.html", }, { private: false, @@ -46,46 +36,51 @@ add_task(async function() { subtest: "link_page-3.html", color: kRed, message: "Visited link coloring should work outside of private mode", }, ]; let uri = Services.io.newURI(prefix + tests[0].subtest); for (let test of tests) { + info(test.subtest); let promise = TestUtils.topicObserved(test.topic, subject => uri.equals(subject.QueryInterface(Ci.nsIURI)) ); - let browser = test.private ? privateBrowser : normalBrowser; - await ContentTask.spawn(browser, prefix + test.subtest, async function( - aSrc - ) { - content.document.getElementById("iframe").src = aSrc; - }); - await promise; - - if (test.color) { - // In e10s waiting for visited-status-resolution is not enough to ensure links - // have been updated, because it only tells us that messages to update links - // have been dispatched. We must still wait for the actual links to update. - await BrowserTestUtils.waitForCondition(async function() { - let color = await ContentTask.spawn(browser, null, async function() { - let iframe = content.document.getElementById("iframe"); - let elem = iframe.contentDocument.getElementById("link"); - return content.windowUtils.getVisitedDependentComputedStyle( - elem, - "", - "color" - ); - }); - return color == test.color; - }, test.message); - // The harness will consider the test as failed overall if there were no - // passes or failures, so record it as a pass. - ok(true, test.message); - } + await BrowserTestUtils.withNewTab( + { + gBrowser: test.private ? privateWindow.gBrowser : normalWindow.gBrowser, + url: prefix + test.subtest, + }, + async function(browser) { + await promise; + if (test.color) { + // In e10s waiting for visited-status-resolution is not enough to ensure links + // have been updated, because it only tells us that messages to update links + // have been dispatched. We must still wait for the actual links to update. + await TestUtils.waitForCondition(async function() { + let color = await ContentTask.spawn( + browser, + null, + async function() { + let elem = content.document.getElementById("link"); + return content.windowUtils.getVisitedDependentComputedStyle( + elem, + "", + "color" + ); + } + ); + return color == test.color; + }, test.message); + // The harness will consider the test as failed overall if there were no + // passes or failures, so record it as a pass. + ok(true, test.message); + } + } + ); } let promisePBExit = TestUtils.topicObserved("last-pb-context-exited"); await BrowserTestUtils.closeWindow(privateWindow); await promisePBExit; await BrowserTestUtils.closeWindow(normalWindow); });
--- a/toolkit/components/places/tests/history/test_async_history_api.js +++ b/toolkit/components/places/tests/history/test_async_history_api.js @@ -619,16 +619,19 @@ add_task(async function test_handleCompl add_task(async function test_add_visit() { const VISIT_TIME = Date.now() * 1000; let place = { uri: NetUtil.newURI(TEST_DOMAIN + "test_add_visit"), title: "test_add_visit title", visits: [], }; for (let t in PlacesUtils.history.TRANSITIONS) { + if (t == "EMBED") { + continue; + } let transitionType = PlacesUtils.history.TRANSITIONS[t]; place.visits.push(new VisitInfo(transitionType, VISIT_TIME)); } Assert.equal(false, await PlacesUtils.history.hasVisits(place.uri)); let callbackCount = 0; let placesResult = await promiseUpdatePlaces(place); if (placesResult.errors.length) { @@ -679,16 +682,19 @@ add_task(async function test_add_visit() } } }); add_task(async function test_properties_saved() { // Check each transition type to make sure it is saved properly. let places = []; for (let t in PlacesUtils.history.TRANSITIONS) { + if (t == "EMBED") { + continue; + } let transitionType = PlacesUtils.history.TRANSITIONS[t]; let place = { uri: NetUtil.newURI( TEST_DOMAIN + "test_properties_saved/" + transitionType ), title: "test_properties_saved test", visits: [new VisitInfo(transitionType)], };
--- a/toolkit/components/places/tests/moz.build +++ b/toolkit/components/places/tests/moz.build @@ -38,17 +38,16 @@ TEST_HARNESS_FILES.xpcshell.toolkit.comp TEST_HARNESS_FILES.testing.mochitest.tests.toolkit.components.places.tests.browser += [ 'browser/399606-history.go-0.html', 'browser/399606-httprefresh.html', 'browser/399606-location.reload.html', 'browser/399606-location.replace.html', 'browser/399606-window.location.href.html', 'browser/399606-window.location.html', - 'browser/461710_iframe.html', 'browser/461710_link_page-2.html', 'browser/461710_link_page-3.html', 'browser/461710_link_page.html', 'browser/461710_visited_page.html', 'browser/begin.html', 'browser/favicon-normal16.png', 'browser/favicon-normal32.png', 'browser/favicon.html',
--- a/toolkit/components/places/tests/unit/test_425563.js +++ b/toolkit/components/places/tests/unit/test_425563.js @@ -9,31 +9,29 @@ add_task(async function test_execute() { "http://www.test-link.com/", "http://www.test-typed.com/", "http://www.test-bookmark.com/", "http://www.test-redirect-permanent.com/", "http://www.test-redirect-temporary.com/", ]; let notcount_visited_URIs = [ - "http://www.test-embed.com/", "http://www.test-download.com/", "http://www.test-framed.com/", "http://www.test-reload.com/", ]; // add visits, one for each transition type await PlacesTestUtils.addVisits([ { uri: uri("http://www.test-link.com/"), transition: TRANSITION_LINK }, { uri: uri("http://www.test-typed.com/"), transition: TRANSITION_TYPED }, { uri: uri("http://www.test-bookmark.com/"), transition: TRANSITION_BOOKMARK, }, - { uri: uri("http://www.test-embed.com/"), transition: TRANSITION_EMBED }, { uri: uri("http://www.test-framed.com/"), transition: TRANSITION_FRAMED_LINK, }, { uri: uri("http://www.test-redirect-permanent.com/"), transition: TRANSITION_REDIRECT_PERMANENT, },
--- a/toolkit/components/places/tests/unit/test_isURIVisited.js +++ b/toolkit/components/places/tests/unit/test_isURIVisited.js @@ -33,16 +33,19 @@ add_task(async function test_isURIVisite resolve([receivedURI, visited]); }); }); } for (let scheme in SCHEMES) { info("Testing scheme " + scheme); for (let t in PlacesUtils.history.TRANSITIONS) { + if (t == "EMBED") { + continue; + } info("With transition " + t); let aTransition = PlacesUtils.history.TRANSITIONS[t]; let aURI = Services.io.newURI(scheme + "mozilla.org/"); let [receivedURI1, visited1] = await visitsPromise(aURI); Assert.ok(aURI.equals(receivedURI1)); Assert.ok(!visited1);
