Bug 1400626 - Metadata not saved when opening multiple pages. r?ursula draft
authorEd Lee <edilee@mozilla.com>
Sat, 16 Sep 2017 18:05:29 -0700
changeset 666030 d6b664bba77f8eead41b8075c3780ca49b7e7830
parent 665954 34e2566a71f160eb3c5c3d92626453852e818f18
child 731953 ae14845c368fbfd43e694d0fc224af4f90325d94
push id80247
push userbmo:edilee@mozilla.com
push dateSun, 17 Sep 2017 06:01:25 +0000
reviewersursula
bugs1400626
milestone57.0a1
Bug 1400626 - Metadata not saved when opening multiple pages. r?ursula MozReview-Commit-ID: 3am9OptKj4z
browser/base/content/test/metaTags/browser.ini
browser/base/content/test/metaTags/browser_meta_tags.js
browser/base/content/test/metaTags/head.js
browser/base/content/test/metaTags/meta_tags.html
browser/modules/ContentMetaHandler.jsm
--- a/browser/base/content/test/metaTags/browser.ini
+++ b/browser/base/content/test/metaTags/browser.ini
@@ -1,4 +1,5 @@
 [DEFAULT]
 support-files =
+  head.js
   meta_tags.html
 [browser_meta_tags.js]
--- a/browser/base/content/test/metaTags/browser_meta_tags.js
+++ b/browser/base/content/test/metaTags/browser_meta_tags.js
@@ -1,30 +1,48 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/
  */
 
-/* globals gBrowser */
-/* This tests that with the page meta_tags.html, ContentMetaHandler.jsm parses out
- * the meta tags avilable and only stores the best one for description and one for
- * preview image url. In the case of this test, the best defined meta tags are
- * "og:description" and "og:image:url". The list of meta tags and their order of
- * preference is found in ContentMetaHandler.jsm. Because there is debounce logic
- * in ContentLinkHandler.jsm to only make one single SQL update, we have to wait
- * for some time before checking that the page info was stored correctly.
+const URL = "https://example.com/browser/browser/base/content/test/metaTags/meta_tags.html";
+
+/**
+ * This tests that with the page meta_tags.html, ContentMetaHandler.jsm parses
+ * out the meta tags avilable and only stores the best one for description and
+ * one for preview image url. In the case of this test, the best defined meta
+ * tags are "og:description" and "og:image:secure_url". The list of meta tags
+ * and order of preference is found in ContentMetaHandler.jsm. Because there is
+ * debounce logic in ContentLinkHandler.jsm to only make one single SQL update,
+ * we have to wait for some time before checking that the page info was stored.
  */
-add_task(async function test() {
-    Components.utils.import("resource://gre/modules/PlacesUtils.jsm");
-    const URL = "https://example.com/browser/browser/base/content/test/metaTags/meta_tags.html";
-    let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, URL);
+add_task(async function test_metadata() {
+  const tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, URL);
 
-    // Wait until places has stored the page info
-    let pageInfo;
-    await BrowserTestUtils.waitForCondition(async () => {
-      pageInfo = await PlacesUtils.history.fetch(URL, {"includeMeta": true});
-      const {previewImageURL, description} = pageInfo;
-      return previewImageURL && description;
-    });
-    is(pageInfo.description, "og:description", "got the correct description");
-    is(pageInfo.previewImageURL.href, "og:image:url", "got the correct preview image");
-    await BrowserTestUtils.removeTab(tab);
+  // Wait until places has stored the page info
+  const pageInfo = await waitForPageInfo(URL);
+  is(pageInfo.description, "og:description", "got the correct description");
+  is(pageInfo.previewImageURL.href, "og:image:secure_url", "got the correct preview image");
+
+  await BrowserTestUtils.removeTab(tab);
+  await PlacesTestUtils.clearHistory();
 });
 
+/**
+ * This test is almost like the previous one except it opens a second tab to
+ * make sure the extra tab does not cause the debounce logic to be skipped. If
+ * incorrectly skipped, the updated metadata would not include the delayed meta.
+ */
+add_task(async function multiple_tabs() {
+  const tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, URL);
+
+  // Add a background tab to cause another page to load *without* putting the
+  // desired URL in a background tab, which results in its timers being throttled.
+  gBrowser.addTab();
+
+  // Wait until places has stored the page info
+  const pageInfo = await waitForPageInfo(URL);
+  is(pageInfo.description, "og:description", "got the correct description");
+  is(pageInfo.previewImageURL.href, "og:image:secure_url", "got the correct preview image");
+
+  await BrowserTestUtils.removeTab(tab);
+  await BrowserTestUtils.removeTab(gBrowser.selectedTab);
+  await PlacesTestUtils.clearHistory();
+});
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/metaTags/head.js
@@ -0,0 +1,18 @@
+Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
+
+XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils",
+  "resource://gre/modules/PlacesUtils.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "PlacesTestUtils",
+  "resource://testing-common/PlacesTestUtils.jsm");
+
+/**
+ * Wait for url's page info (non-null description and preview url) to be set.
+ */
+async function waitForPageInfo(url) {
+  let pageInfo;
+  await BrowserTestUtils.waitForCondition(async () => {
+    pageInfo = await PlacesUtils.history.fetch(url, {"includeMeta": true});
+    return pageInfo && pageInfo.description && pageInfo.previewImageURL;
+  });
+  return pageInfo;
+}
--- a/browser/base/content/test/metaTags/meta_tags.html
+++ b/browser/base/content/test/metaTags/meta_tags.html
@@ -8,11 +8,23 @@
     <meta name="description" content="description" />
     <meta name="unknown:tag" content="unknown:tag" />
     <meta property="og:image" content="og:image" />
     <meta property="twitter:image" content="twitter:image" />
     <meta property="og:image:url" content="og:image:url" />
     <meta name="thumbnail" content="thumbnail" />
   </head>
   <body>
+    <script>
+      function addMeta(tag) {
+        const meta = document.createElement("meta");
+        meta.content = tag;
+        meta.setAttribute("property", tag);
+        document.head.appendChild(meta);
+      }
+
+      // Delay adding this "best" image tag to test that later tags are used.
+      // Use a delay that is long enough for tests to check for wrong metadata.
+      setTimeout(() => addMeta("og:image:secure_url"), 100);
+    </script>
   </body>
 </html>
 
--- a/browser/modules/ContentMetaHandler.jsm
+++ b/browser/modules/ContentMetaHandler.jsm
@@ -49,44 +49,44 @@ this.EXPORTED_SYMBOLS = [ "ContentMetaHa
 /*
  * This listens to DOMMetaAdded events and collects relevant metadata about the
  * meta tag received. Then, it sends the metadata gathered from the meta tags
  * and the url of the page as it's payload to be inserted into moz_places.
  */
 
 this.ContentMetaHandler = {
   init(chromeGlobal) {
+    // Store a locally-scoped (for this chromeGlobal) mapping of the best
+    // description and preview image collected so far for a given URL
+    const metaTags = new Map();
     chromeGlobal.addEventListener("DOMMetaAdded", event => {
       const metaTag = event.originalTarget;
       const window = metaTag.ownerGlobal;
 
       // If there's no meta tag, or we're in a sub-frame, ignore this
       if (!metaTag || !metaTag.ownerDocument || window != window.top) {
         return;
       }
-      this.handleMetaTag(metaTag, chromeGlobal);
+      this.handleMetaTag(metaTag, chromeGlobal, metaTags);
     });
-    // Stores a mapping of the best description and preview image collected so far
-    // for a given URL
-    this._metaTags = new Map();
   },
 
 
-  handleMetaTag(metaTag, chromeGlobal) {
+  handleMetaTag(metaTag, chromeGlobal, metaTags) {
     const url = metaTag.ownerDocument.documentURI;
 
     let name = metaTag.name;
     let prop = metaTag.getAttributeNS(null, "property");
     if (!name && !prop) {
       return;
     }
 
     let tag = name || prop;
 
-    const entry = this._metaTags.get(url) || {
+    const entry = metaTags.get(url) || {
       description: {value: null, currMaxScore: -1},
       image: {value: null, currMaxScore: -1},
       timeout: null
     };
 
     if (shouldExtractMetadata(DESCRIPTION_RULES, tag, entry.description)) {
       // Extract the description
       const value = metaTag.getAttributeNS(null, "content");
@@ -101,18 +101,18 @@ this.ContentMetaHandler = {
         entry.image.value = new URL(value, url).href;
         entry.image.currMaxScore = PREVIEW_IMAGE_RULES.indexOf(tag);
       }
     } else {
       // We don't care about other meta tags
       return;
     }
 
-    if (!this._metaTags.has(url)) {
-      this._metaTags.set(url, entry);
+    if (!metaTags.has(url)) {
+      metaTags.set(url, entry);
     }
 
     if (entry.timeout) {
       entry.timeout.delay = TIMEOUT_DELAY;
     } else {
       // We want to debounce incoming meta tags until we're certain we have the
       // best one for description and preview image, and only store that one
       entry.timeout = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
@@ -120,13 +120,13 @@ this.ContentMetaHandler = {
         entry.timeout = null;
 
         // Save description and preview image to moz_places
         chromeGlobal.sendAsyncMessage("Meta:SetPageInfo", {
           url,
           description: entry.description.value,
           previewImageURL: entry.image.value
         });
-        this._metaTags.delete(url);
+        metaTags.delete(url);
       }, TIMEOUT_DELAY, Ci.nsITimer.TYPE_ONE_SHOT);
     }
   }
 };