Bug 1595285 - Do not track TRANSITION_EMBED visits for link-coloring purposes. r=mak

Other browsers don't, plus it blocks work I want to do to query multiple links
at the same time.

Differential Revision: https://phabricator.services.mozilla.com/D52443

--HG--
extra : moz-landing-system : lando

dom/html/test/test_bug209275.xhtml

@@ -120,11 +120,12 @@ function* run() {
   $('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);
@@ -147,18 +148,20 @@ function* run() {
   // 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/
@@ -220,7 +223,6 @@ function* run() {
   // 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,

dom/html/test/test_bug481335.xhtml

@@ -54,13 +54,15 @@ function checkLinkColor(aElmId, aExpectedColor, aMessage) {
     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"));
@@ -71,8 +73,8 @@ function* testIterator() {
   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"))
@@ -82,6 +84,7 @@ function* testIterator() {
             "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";
@@ -96,20 +99,21 @@ function* testIterator() {
   }
   */
 
-  $("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>

layout/reftests/css-visited/color-on-visited-text-1.html

@@ -14,7 +14,7 @@
     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>

layout/reftests/css-visited/link-root-1.xhtml

@@ -17,7 +17,7 @@ a { display: block; width: 100%; height: 100% }
 
 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");
 

layout/reftests/css-visited/mathml-links.html

@@ -7,7 +7,7 @@
     </style>
   </head>
   <body>
-    <math href="" display="block">
+    <math href="visited-page.html" display="block">
       <mi>x</mi>
       <mo>=</mo>
       <mfrac>

layout/reftests/css-visited/svg-paint-currentcolor-visited.svg

@@ -12,5 +12,5 @@
     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>

layout/reftests/svg/as-image/svg-image-visited-1a-helper.svg

@@ -14,10 +14,10 @@
   ]]>
   </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>
 

layout/reftests/svg/as-image/svg-image-visited-1b-helper.svg

@@ -13,10 +13,10 @@
   ]]>
   </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>
 

layout/reftests/svg/pseudo-classes-02.svg

@@ -27,7 +27,7 @@ a:visited > tspan       { fill: lime; }
   </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>
 
@@ -40,7 +40,7 @@ a:visited > tspan       { fill: lime; }
 
   <!-- 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>

layout/style/test/test_visited_image_loading.html

@@ -10,7 +10,6 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=557287
 </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">
@@ -26,10 +25,11 @@ 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) {
@@ -59,6 +59,7 @@ function paint_listener(event)
   var s = document.createElement("script");
   s.src = "visited_image_loading.sjs?waitforresult";
   document.body.appendChild(s);
+  subwin.close();
 }
 
 </script>

layout/style/test/test_visited_image_loading_empty.html

@@ -10,7 +10,6 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=557287
 </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">
@@ -26,10 +25,11 @@ 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) {
@@ -59,6 +59,7 @@ function paint_listener(event)
   var s = document.createElement("script");
   s.src = "visited_image_loading.sjs?waitforresult";
   document.body.appendChild(s);
+  subwin.close();
 }
 
 </script>

layout/style/test/test_visited_lying.html

@@ -36,7 +36,7 @@ function start()
   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);

layout/style/test/test_visited_pref.html

@@ -63,7 +63,7 @@ function step1()
   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();
 

layout/style/test/test_visited_reftests.html

@@ -128,8 +128,14 @@ async function runTests() {
   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();
 }
 

toolkit/components/places/History.cpp

@@ -378,18 +378,6 @@ class VisitedQuery final : public AsyncStatementCallback {
         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);
@@ -467,9 +455,8 @@ class VisitedQuery final : public AsyncStatementCallback {
  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() {}
 
@@ -1375,9 +1362,11 @@ class SetPageTitle : public Runnable {
  *        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!");
@@ -1385,13 +1374,10 @@ void StoreAndNotifyEmbedVisit(VisitData& aPlace,
   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>(
@@ -1988,10 +1974,10 @@ History::VisitURI(nsIWidget* aWidget, nsIURI* aURI, nsIURI* aLastVisitedURI,
     }
   }
 
-  // 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);
@@ -2043,18 +2029,10 @@ History::SetURITitle(nsIURI* aURI, const nsAString& aTitle) {
     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);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -2175,7 +2153,7 @@ History::UpdatePlaces(JS::Handle<JS::Value> aPlaceInfos,
       // 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;

toolkit/components/places/History.jsm

@@ -908,9 +908,6 @@ var clear = async function(db) {
                         WHERE frecency > 0`);
   });
 
-  // Clear the registered embed visits.
-  PlacesUtils.history.clearEmbedVisits();
-
   let observers = PlacesUtils.history.getObservers();
   notify(observers, "onClearHistory");
   // Notify frecency change observers.
@@ -1277,62 +1274,57 @@ var removeVisitsByFilter = async function(db, filter, onResult = null) {
     }
   );
 
-  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;
 };
@@ -1423,26 +1415,22 @@ var removeByFilter = async function(db, filter, onResult = null) {
     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;
 };
@@ -1503,33 +1491,28 @@ var remove = async function(db, { guids, urls }, onResult = null) {
     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;
 };

toolkit/components/places/nsINavHistoryService.idl

@@ -1257,11 +1257,6 @@ interface nsINavHistoryService : nsISupports
    */
   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.)

toolkit/components/places/nsNavHistory.cpp

@@ -130,9 +130,6 @@ using namespace mozilla::places;
 // 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
 
@@ -377,7 +374,6 @@ nsNavHistory::nsNavHistory()
       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),
@@ -2759,29 +2755,6 @@ nsresult nsNavHistory::FilterResultSet(
   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))) {

toolkit/components/places/nsNavHistory.h

@@ -268,16 +268,6 @@ class nsNavHistory final : public nsSupportsWeakReference,
    */
   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.
    *
@@ -487,18 +477,6 @@ class nsNavHistory final : public nsSupportsWeakReference,
   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);
 

toolkit/components/places/tests/browser/461710_iframe.html

@@ -1,8 +0,0 @@
-<!DOCTYPE HTML>
-<html>
-  <head>
-  </head>
-  <body>
-    <iframe id="iframe"></iframe>
-  </body>
-</html>

toolkit/components/places/tests/browser/browser.ini

@@ -12,7 +12,6 @@ support-files =
   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

toolkit/components/places/tests/browser/browser_bug461710.js

@@ -6,20 +6,10 @@ const prefix =
 
 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,
@@ -51,37 +41,42 @@ add_task(async function() {
 
   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");

toolkit/components/places/tests/history/test_async_history_api.js

@@ -624,6 +624,9 @@ add_task(async function test_add_visit() {
     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));
   }
@@ -684,6 +687,9 @@ 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(

toolkit/components/places/tests/moz.build

@@ -43,7 +43,6 @@ TEST_HARNESS_FILES.testing.mochitest.tests.toolkit.components.places.tests.brows
     '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',

toolkit/components/places/tests/unit/test_425563.js

@@ -14,7 +14,6 @@ add_task(async function test_execute() {
   ];
 
   let notcount_visited_URIs = [
-    "http://www.test-embed.com/",
     "http://www.test-download.com/",
     "http://www.test-framed.com/",
     "http://www.test-reload.com/",
@@ -28,7 +27,6 @@ add_task(async function test_execute() {
       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,

toolkit/components/places/tests/unit/test_isURIVisited.js

@@ -38,6 +38,9 @@ add_task(async function test_isURIVisited() {
   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];