Backed out changeset 74c36888692b (bug 1610298) for causing browser-chrome failures at browser/components/aboutlogins/tests/browser/browser_breachAlertShowingForAddedLogin.js

CLOSED TREE
2020-01-28 12:50:44 +02:00 · 2020-01-28 12:50:44 +02:00 · cd4e1066fd
--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@ -1282,7 +1282,6 @@ pref("browser.newtabpage.activity-stream.asrouter.providers.message-groups", "{\
 // this page over http opens us up to a man-in-the-middle attack that we'd rather not face. If you are a downstream
 // repackager of this code using an alternate snippet url, please keep your users safe
 pref("browser.newtabpage.activity-stream.asrouter.providers.snippets", "{\"id\":\"snippets\",\"enabled\":true,\"type\":\"remote\",\"url\":\"https://snippets.cdn.mozilla.net/%STARTPAGE_VERSION%/%NAME%/%VERSION%/%APPBUILDID%/%BUILD_TARGET%/%LOCALE%/%CHANNEL%/%OS_VERSION%/%DISTRIBUTION%/%DISTRIBUTION_VERSION%/\",\"updateCycleInMs\":14400000}");
-pref("browser.newtabpage.activity-stream.asrouter.providers.snippets-preview", "{\"id\":\"snippets-preview\",\"enabled\":true,\"type\":\"remote\",\"url\":\"\",\"updateCycleInMs\":0}");
 #ifdef NIGHTLY_BUILD
  pref("browser.newtabpage.activity-stream.asrouter.useReleaseSnippets", true);
 #endif
--- a/browser/components/newtab/lib/ASRouter.jsm
+++ b/browser/components/newtab/lib/ASRouter.jsm
@ -68,12 +68,11 @@ const TRAILHEAD_CONFIG = {
 const INCOMING_MESSAGE_NAME = "ASRouter:child-to-parent";
 const OUTGOING_MESSAGE_NAME = "ASRouter:parent-to-child";
 const ONE_DAY_IN_MS = 24 * 60 * 60 * 1000;
-const PREVIEW_SNIPPETS_ID = "snippets-preview";
 // List of hosts for endpoints that serve router messages.
 // Key is allowed host, value is a name for the endpoint host.
 const DEFAULT_WHITELIST_HOSTS = {
  "activity-stream-icons.services.mozilla.com": "production",
-  "snippets-admin.mozilla.org": PREVIEW_SNIPPETS_ID,
+  "snippets-admin.mozilla.org": "preview",
 };
 const SNIPPETS_ENDPOINT_WHITELIST =
  "browser.newtab.activity-stream.asrouter.whitelistHosts";
@ -604,7 +603,7 @@ class _ASRouter {
    const previousProviders = this.state.providers;
    const providers = [
      // If we have added a `preview` provider, hold onto it
-      ...previousProviders.filter(p => p.id === PREVIEW_SNIPPETS_ID),
+      ...previousProviders.filter(p => p.id === "preview"),
      // The provider should be enabled and not have a user preference set to false
      ...ASRouterPreferences.providers.filter(
        p =>
@ -699,9 +698,6 @@ class _ASRouter {
   * @returns bool
   */
  hasGroupsEnabled(groups = []) {
-    if (groups.includes(PREVIEW_SNIPPETS_ID)) {
-      return true;
-    }
    return this.state.groups
      .filter(({ id }) => groups.includes(id))
      .every(({ enabled }) => enabled);
@ -714,7 +710,10 @@ class _ASRouter {
   */
  isExcludedByProvider(message) {
    const provider = this.state.providers.find(p => p.id === message.provider);
-    if (provider && provider.exclude) {
+    if (!provider) {
+      return true;
+    }
+    if (provider.exclude) {
      return provider.exclude.includes(message.id);
    }
    return false;
@ -850,7 +849,7 @@ class _ASRouter {
      }

      // We don't want to cache preview endpoints, remove them after messages are fetched
-      await this.setState(this._disablePreviewEndpoint(newState));
+      await this.setState(this._removePreviewEndpoint(newState));
      await this.cleanupImpressions();
    }
  }
@ -1758,7 +1757,7 @@ class _ASRouter {
    // `preview` so that the updateCycle is 0
    return additionalHosts.reduce(
      (whitelist_hosts, host) => {
-        whitelist_hosts[host] = PREVIEW_SNIPPETS_ID;
+        whitelist_hosts[host] = "preview";
        Services.console.logStringMessage(`Adding ${host} to whitelist hosts.`);
        return whitelist_hosts;
      },
@ -1778,38 +1777,27 @@ class _ASRouter {
    });
  }

-  _disablePreviewEndpoint(state) {
-    const previewProvider = this.state.providers.find(
-      p => p.id === PREVIEW_SNIPPETS_ID
-    );
-    if (previewProvider && previewProvider.enabled) {
-      const providers = this.state.providers.filter(
-        p => p.id !== PREVIEW_SNIPPETS_ID
-      );
-      previewProvider.url = "";
-      previewProvider.enabled = false;
-      providers.push(previewProvider);
-      state.providers = providers;
-    }
-
+  _removePreviewEndpoint(state) {
+    state.providers = state.providers.filter(p => p.id !== "preview");
    return state;
  }

  async _addPreviewEndpoint(url, portID) {
-    const providers = [...this.state.providers].filter(
-      p => p.id !== PREVIEW_SNIPPETS_ID
-    );
-    const previewProvider = this.state.providers.find(
-      p => p.id === PREVIEW_SNIPPETS_ID
-    );
-    if (this._validPreviewEndpoint(url)) {
+    // When you view a preview snippet we want to hide all real content
+    const providers = [...this.state.providers];
+    if (
+      this._validPreviewEndpoint(url) &&
+      !providers.find(p => p.url === url)
+    ) {
      this.dispatchToAS(
-        // When you view a preview snippet we want to hide all real content
        ac.OnlyToOneContent({ type: at.SNIPPETS_PREVIEW_MODE }, portID)
      );
-      previewProvider.enabled = true;
-      previewProvider.url = url;
-      providers.push(previewProvider);
+      providers.push({
+        id: "preview",
+        type: "remote",
+        url,
+        updateCycleInMs: 0,
+      });
      await this.setState({ providers });
    }
  }
@ -2032,9 +2020,7 @@ class _ASRouter {
    await this.loadMessagesFromAllProviders();

    if (endpoint) {
-      message = await this.handleMessageRequest({
-        provider: PREVIEW_SNIPPETS_ID,
-      });
+      message = await this.handleMessageRequest({ provider: "preview" });

      // We don't want to cache preview messages, remove them after we selected the message to show
      if (message) {
@ -2182,7 +2168,12 @@ class _ASRouter {
        await this.setMessageById(action.data.id, target, true, action);
        break;
      case "ADMIN_CONNECT_STATE":
-        await this._updateAdminState(target);
+        if (action.data && action.data.endpoint) {
+          this._addPreviewEndpoint(action.data.endpoint.url, target.portID);
+          await this.loadMessagesFromAllProviders();
+        } else {
+          await this._updateAdminState(target);
+        }
        break;
      case "IMPRESSION":
        await this.addImpression(action.data);
--- a/browser/components/newtab/test/browser/browser.ini
+++ b/browser/components/newtab/test/browser/browser.ini
@ -3,7 +3,6 @@ support-files =
  blue_page.html
  red_page.html
  head.js
-  snippet.json
 prefs =
  browser.newtabpage.activity-stream.debug=false
  browser.newtabpage.activity-stream.discoverystream.enabled=true
--- a/browser/components/newtab/test/browser/browser_asrouter_snippets.js
+++ b/browser/components/newtab/test/browser/browser_asrouter_snippets.js
@ -64,38 +64,3 @@ test_newtab({
    );
  },
 });
-
-add_task(async () => {
-  ASRouter._validPreviewEndpoint = () => true;
-  await BrowserTestUtils.withNewTab(
-    {
-      gBrowser,
-      url:
-        "about:newtab?endpoint=https://example.com/browser/browser/components/newtab/test/browser/snippet.json",
-    },
-    async browser => {
-      let text = await SpecialPowers.spawn(browser, [], async () => {
-        await ContentTaskUtils.waitForCondition(
-          () => content.document.querySelector(".activity-stream"),
-          `Should render Activity Stream`
-        );
-        await ContentTaskUtils.waitForCondition(
-          () =>
-            content.document.querySelector(
-              "#footer-asrouter-container .SimpleSnippet"
-            ),
-          "Should find the snippet inside the footer container"
-        );
-
-        return content.document.querySelector("#footer-asrouter-container")
-          .innerText;
-      });
-
-      Assert.equal(
-        text,
-        "On January 30th Nightly will introduce dedicated profiles, making it simpler to run different installations of Firefox side by side. Learn what this means for you.",
-        "Snippet content match"
-      );
-    }
-  );
-});
--- a/browser/components/newtab/test/browser/snippet.json
+++ b/browser/components/newtab/test/browser/snippet.json
@ -1,25 +0,0 @@
-{
-  "messages": [
-    {
-      "weight": 50,
-      "id": "10533",
-      "template": "simple_snippet",
-      "template_version": "1.0.0",
-      "content": {
-        "icon": "",
-        "text": "On January 30th Nightly will introduce dedicated profiles, making it simpler to run different installations of Firefox side by side. <link0> Learn what this means for you</link0>.",
-        "tall": false,
-        "do_not_autoblock": false,
-        "links": {
-          "link0": {
-            "url": "https://support.mozilla.org/kb/dedicated-profiles-firefox-installation/?utm_source=desktop-snippet&utm_medium=snippet&utm_campaign=nightly_profile_management&utm_term=10533&utm_content=nightly"
-          }
-        }
-      },
-      "campaign": "nightly-profile-management",
-      "targeting": "true",
-      "provider_url": "https://snippets.cdn.mozilla.net/6/Firefox/66.0a1/20190122215349/Darwin_x86_64-gcc3/en-US/default/Darwin%2018.0.0/default/default/",
-      "provider": "snippets"
-    }
-  ]
-}
--- a/browser/components/newtab/test/unit/asrouter/ASRouter.test.js
+++ b/browser/components/newtab/test/unit/asrouter/ASRouter.test.js
@ -50,7 +50,6 @@ function fakeExecuteUserAction(action) {
  return fakeAsyncMessage({ data: action, type: "USER_ACTION" });
 }

-// eslint-disable-next-line max-statements
 describe("ASRouter", () => {
  let Router;
  let globals;
@ -346,11 +345,7 @@ describe("ASRouter", () => {
      getStringPrefStub.returns('["whitelist.com"]');
      await createRouterAndInit();

-      assert.propertyVal(
-        Router.WHITELIST_HOSTS,
-        "whitelist.com",
-        "snippets-preview"
-      );
+      assert.propertyVal(Router.WHITELIST_HOSTS, "whitelist.com", "preview");
      // Should still include the defaults
      assert.lengthOf(Object.keys(Router.WHITELIST_HOSTS), 3);
    });
@ -362,7 +357,7 @@ describe("ASRouter", () => {
      assert.propertyVal(
        Router.WHITELIST_HOSTS,
        "snippets-admin.mozilla.org",
-        "snippets-preview"
+        "preview"
      );
      assert.propertyVal(
        Router.WHITELIST_HOSTS,
@ -1161,28 +1156,6 @@ describe("ASRouter", () => {
      });
      assert.isNull(result);
    });
-    it("should allow messages with undefined providers (like 'preview')", async () => {
-      sandbox.stub(Router, "loadMessagesFromAllProviders");
-      await Router.setState(() => ({
-        providers: [{ id: "snippets", exclude: ["foo"] }],
-      }));
-
-      await Router.setState(() => ({
-        messages: [
-          {
-            id: "foo",
-            provider: "snippets-preview",
-            groups: ["snippets-preview"],
-          },
-        ],
-        messageBlockList: ["foocampaign"],
-      }));
-
-      const result = await Router.handleMessageRequest({
-        provider: "snippets-preview",
-      });
-      assert.propertyVal(result, "provider", "snippets-preview");
-    });
    it("should not return a message if the frequency cap has been hit", async () => {
      sandbox.stub(Router, "isBelowFrequencyCaps").returns(false);
      await Router.setState(() => ({
@ -1663,18 +1636,6 @@ describe("ASRouter", () => {
      });
    });
    describe("#_addPreviewEndpoint", () => {
-      beforeEach(async () => {
-        await Router.setState(state => {
-          const providers = [...state.providers];
-          const snippetsPreview = {
-            id: "snippets-preview",
-            enabled: false,
-            type: "remote",
-          };
-          providers.push(snippetsPreview);
-          return { providers };
-        });
-      });
      it("should make a request to the provided endpoint on NEWTAB_MESSAGE_REQUEST", async () => {
        const url = "https://snippets-admin.mozilla.org/foo";
        const msg = fakeAsyncMessage({
@ -1686,6 +1647,17 @@ describe("ASRouter", () => {
        assert.calledWith(global.fetch, url);
        assert.lengthOf(Router.state.providers.filter(p => p.url === url), 0);
      });
+      it("should make a request to the provided endpoint on ADMIN_CONNECT_STATE and remove the endpoint", async () => {
+        const url = "https://snippets-admin.mozilla.org/foo";
+        const msg = fakeAsyncMessage({
+          type: "ADMIN_CONNECT_STATE",
+          data: { endpoint: { url } },
+        });
+        await Router.onMessage(msg);
+
+        assert.calledWith(global.fetch, url);
+        assert.lengthOf(Router.state.providers.filter(p => p.url === url), 0);
+      });
      it("should dispatch SNIPPETS_PREVIEW_MODE when adding a preview endpoint", async () => {
        const url = "https://snippets-admin.mozilla.org/foo";
        const msg = fakeAsyncMessage({
@ -2041,12 +2013,12 @@ describe("ASRouter", () => {
      it("should return the preview message if that's available and remove it from Router.state", async () => {
        const expectedObj = {
          id: "foo",
-          groups: ["snippets-preview"],
-          provider: "snippets-preview",
+          groups: ["preview"],
+          provider: "preview",
        };
        Router.setState({
          messages: [expectedObj],
-          providers: [{ id: "snippets-preview" }],
+          providers: [{ id: "preview" }],
        });

        await Router.sendNewTabMessage(channel, { endpoint: "foo.com" });
@ -2057,7 +2029,7 @@ describe("ASRouter", () => {
          { type: "SET_MESSAGE", data: expectedObj }
        );
        assert.isUndefined(
-          Router.state.messages.find(m => m.provider === "snippets-preview")
+          Router.state.messages.find(m => m.provider === "preview")
        );
      });
      it("should call _getBundledMessages if we request a message that needs to be bundled", async () => {
@ -3547,23 +3519,6 @@ describe("ASRouter", () => {
        assert.notCalled(Router._storage.set);
        assert.deepEqual(Router.state.messageImpressions, messageImpressions);
      });
-      it("should call setState before calling cleanupImpressions", async () => {
-        // The call order in `loadMessagesFromAllProviders` is important
-        const messages = [
-          { id: "foo", frequency: { lifetime: 10 } },
-          { id: "bar", frequency: { lifetime: 10 } },
-        ];
-        const setStateStub = sandbox.stub(Router, "setState");
-        const cleanupImpressionsStub = sandbox.stub(
-          Router,
-          "cleanupImpressions"
-        );
-        await createRouterAndInit([
-          { id: "onboarding", type: "local", messages, enabled: true },
-        ]);
-
-        cleanupImpressionsStub.calledImmediatelyAfter(setStateStub);
-      });
    });
  });

@ -4083,25 +4038,4 @@ describe("ASRouter", () => {
      });
    });
  });
-  describe("#loadMessagesForProvider", () => {
-    it("should fetch json from url", async () => {
-      let result = await MessageLoaderUtils.loadMessagesForProvider({
-        location: "http://fake.com/endpoint",
-        type: "json",
-      });
-
-      assert.property(result, "messages");
-      assert.lengthOf(result.messages, FAKE_REMOTE_MESSAGES.length);
-    });
-    it("should catch errors", async () => {
-      fetchStub.throws();
-      let result = await MessageLoaderUtils.loadMessagesForProvider({
-        location: "http://fake.com/endpoint",
-        type: "json",
-      });
-
-      assert.property(result, "messages");
-      assert.lengthOf(result.messages, 0);
-    });
-  });
 });