Commit 98969866 authored by Sam Foster's avatar Sam Foster
Browse files

Bug 1803505 - Simplify the tab-pickup-container initialization. r=sclements 

* Adds a promise that resolves when the tab-pickup-list is ready
* Use that promise to trigger tab data request for the list when we enter the setup-complete state
* Remove the workarounds for overlapping setup and tabs requests

Differential Revision: https://phabricator.services.mozilla.com/D163750
parent 8f12c546

browser/components/firefoxview/firefoxview.html

+31 −36
Original line number Diff line number Diff line
@@ -35,10 +35,23 @@ 
      </div>

    </div>

    <main>

      <template id="sync-setup-template">

        <named-deck class="sync-setup-container" role="region" aria-labelledby="collapsible-synced-tabs-header" data-prefix="id:">

          <div name="sync-setup-view0" data-prefix="id:-view0" class="card card-no-hover error-state" data-prefix="aria-labelledby:-view0-header">

            <icon class="icon info primary"></icon><h3 data-prefix="id:-view0-header" class="card-header"></h3>

      <details class="content-container" is="tab-pickup-container" id="tab-pickup-container" open>

        <summary class="page-section-header">

          <span class="twisty icon" data-l10n-id="firefoxview-collapse-button-hide" aria-role="presentation"></span>

          <h1 id="collapsible-synced-tabs-header" data-l10n-id="firefoxview-tabpickup-header"></h1>

          <p class="section-description" data-l10n-id="firefoxview-tabpickup-description"></p>

        </summary>



        <div class="confirmation-message-box message-box card card-no-hover" hidden>

          <div class="message-content">

            <h2 data-l10n-id="firefoxview-mobile-confirmation-header" class="message-header"></h2>

            <span class="message-description" data-l10n-id="firefoxview-mobile-confirmation-description"></span>

          </div>

          <button data-action="mobile-confirmation-dismiss" class="close icon-button ghost-button" data-l10n-id="firefoxview-close-button"></button>

        </div>

        <named-deck class="sync-setup-container" role="region" aria-labelledby="collapsible-synced-tabs-header" id="tabpickup-steps">

          <div name="sync-setup-view0" id="tabpickup-steps-view0" class="card card-no-hover error-state" aria-labelledby="tabpickup-steps-view0-header">

            <icon class="icon info primary"></icon><h3 id="tabpickup-steps-view0-header" class="card-header"></h3>

            <section>

              <p>

                <span id="error-state-description"></span>
@@ -47,22 +60,22 @@ 
              <button id="error-state-button" class="primary"></button>

            </section>

          </div>

          <div name="sync-setup-view1" data-prefix="id:-view1" class="card card-no-hover zap-card setup-step" data-prefix="aria-labelledby:-view1-header">

            <h2 data-prefix="id:-view1-header" data-l10n-id="firefoxview-tabpickup-step-signin-header" class="card-header"></h2>

          <div name="sync-setup-view1" id="tabpickup-steps-view1" class="card card-no-hover zap-card setup-step" aria-labelledby="tabpickup-steps-view1-header">

            <h2 id="tabpickup-steps-view1-header" data-l10n-id="firefoxview-tabpickup-step-signin-header" class="card-header"></h2>

            <section class="card-body">

              <p class="step-content" data-l10n-id="firefoxview-tabpickup-step-signin-description"></p>

              <button id="firefoxview-tabpickup-step-signin-primarybutton" class="primary" data-action="view1-primary-action" data-l10n-id="firefoxview-tabpickup-step-signin-primarybutton"></button>

            </section>

            <footer>

              <progress data-prefix="id:-view1-progress" class="step-progress" max="100" value="11"></progress>

              <progress id="tabpickup-steps-view1-progress" class="step-progress" max="100" value="11"></progress>

              <label

                data-prefix="for:-view1-progress"

                for="tabpickup-steps-view1-progress"

                data-l10n-id="firefoxview-tabpickup-progress-label"

                data-l10n-args='{"percentValue":"11"}'></label>

            </footer>

          </div>

          <div name="sync-setup-view2" data-prefix="id:-view2" class="card card-no-hover zap-card setup-step" data-prefix="aria-labelledby:-view2-header">

            <h2 data-prefix="id:-view2-header" data-l10n-id="firefoxview-tabpickup-adddevice-header" class="card-header"></h2>

          <div name="sync-setup-view2" id="tabpickup-steps-view2" class="card card-no-hover zap-card setup-step" aria-labelledby="tabpickup-steps-view2-header">

            <h2 id="tabpickup-steps-view2-header" data-l10n-id="firefoxview-tabpickup-adddevice-header" class="card-header"></h2>

            <section class="card-body">

              <p class="step-content">

                <span data-l10n-id="firefoxview-tabpickup-adddevice-description"></span>
@@ -72,15 +85,15 @@ 
              <button class="primary" data-action="view2-primary-action" data-l10n-id="firefoxview-tabpickup-adddevice-primarybutton"></button>

            </section>

            <footer>

              <progress data-prefix="id:-view2-progress" class="step-progress" max="100" value="33"></progress>

              <progress id="tabpickup-steps-view2-progress" class="step-progress" max="100" value="33"></progress>

              <label

                data-prefix="for:-view2-progress"

                for="tabpickup-steps-view2-progress"

                data-l10n-id="firefoxview-tabpickup-progress-label"

                data-l10n-args='{"percentValue":"33"}'></label>

            </footer>

          </div>

          <div name="sync-setup-view3" data-prefix="id:-view3" class="card card-no-hover zap-card setup-step" data-prefix="aria-labelledby:-view3-header">

            <h2 data-prefix="id:-view3-header" data-l10n-id="firefoxview-tabpickup-synctabs-header" class="card-header"></h2>

          <div name="sync-setup-view3" id="tabpickup-steps-view3" class="card card-no-hover zap-card setup-step" aria-labelledby="tabpickup-steps-view3-header">

            <h2 id="tabpickup-steps-view3-header" data-l10n-id="firefoxview-tabpickup-synctabs-header" class="card-header"></h2>

            <section class="card-body">

              <p class="step-content">

                <span  data-l10n-id="firefoxview-tabpickup-synctabs-description"></span>
@@ -90,17 +103,16 @@ 
              <button class="primary" data-action="view3-primary-action" data-l10n-id="firefoxview-tabpickup-synctabs-primarybutton"></button>

            </section>

            <footer>

              <progress data-prefix="id:-view3-progress" class="step-progress" max="100" value="66"></progress>

              <progress id="tabpickup-steps-view3-progress" class="step-progress" max="100" value="66"></progress>

              <label

                data-prefix="for:-view3-progress"

                for="tabpickup-steps-view3-progress"

                data-l10n-id="firefoxview-tabpickup-progress-label"

                data-l10n-args='{"percentValue":"66"}'></label>

            </footer>

          </div>

        </named-deck>

      </template>

      <template id="synced-tabs-template">

        <div id="synced-tabs" role="region" aria-labelledby="collapsible-synced-tabs-header" class="synced-tabs-container" data-prefix="id:" hidden>



        <div id="tabpickup-tabs-container" role="region" aria-labelledby="collapsible-synced-tabs-header" class="synced-tabs-container" hidden>

          <tab-pickup-list>

            <ol hidden="true" class="synced-tabs-list"></ol>

          </tab-pickup-list>
@@ -116,24 +128,7 @@ 
            <p data-l10n-id="firefoxview-tabpickup-syncing"></p>

          </div>

        </div>

      </template>



      <details class="content-container" is="tab-pickup-container" id="tab-pickup-container" open>

        <summary class="page-section-header">

          <span class="twisty icon" data-l10n-id="firefoxview-collapse-button-hide" aria-role="presentation"></span>

          <h1 id="collapsible-synced-tabs-header" data-l10n-id="firefoxview-tabpickup-header"></h1>

          <p class="section-description" data-l10n-id="firefoxview-tabpickup-description"></p>

        </summary>



        <div class="confirmation-message-box message-box card card-no-hover" hidden>

          <div class="message-content">

            <h2 data-l10n-id="firefoxview-mobile-confirmation-header" class="message-header"></h2>

            <span class="message-description" data-l10n-id="firefoxview-mobile-confirmation-description"></span>

          </div>

          <button data-action="mobile-confirmation-dismiss" class="close icon-button ghost-button" data-l10n-id="firefoxview-close-button"></button>

        </div>

        <div id="sync-setup-placeholder" hidden></div>

        <div id="synced-tabs-placeholder" hidden></div>

        <div class="promo-box message-box zap-card card-no-hover card" hidden>

          <div class="card-body">

            <div class="message-content">

browser/components/firefoxview/tab-pickup-container.mjs

+33 −51
Original line number Diff line number Diff line
@@ -19,6 +19,7 @@ class TabPickupContainer extends HTMLDetailsElement { 
    this.boundObserve = (...args) => this.observe(...args);

    this._currentSetupStateIndex = -1;

    this.errorState = null;

    this.tabListAdded = null;

  }

  get setupContainerElem() {

    return this.querySelector(".sync-setup-container");
@@ -28,6 +29,10 @@ class TabPickupContainer extends HTMLDetailsElement { 
    return this.querySelector(".synced-tabs-container");

  }



  get tabPickupListElem() {

    return this.querySelector(".synced-tabs-container tab-pickup-list");

  }



  getWindow() {

    return this.ownerGlobal.browsingContext.embedderWindowGlobal.browsingContext

      .window;
@@ -38,9 +43,29 @@ class TabPickupContainer extends HTMLDetailsElement { 
    this.addEventListener("toggle", this);

    this.addEventListener("visibilitychange", this);

    Services.obs.addObserver(this.boundObserve, TOPIC_SETUPSTATE_CHANGED);



    // we wait until the list shows up before trying to populate it,

    // when its safe to assume the custom-element's methods will be available

    this.tabListAdded = this.promiseChildAdded();

    this.update();

  }



  promiseChildAdded() {

    return new Promise(resolve => {

      if (typeof this.tabPickupListElem?.getSyncedTabData == "function") {

        resolve();

        return;

      }

      this.addEventListener(

        "list-ready",

        event => {

          resolve();

        },

        { once: true }

      );

    });

  }



  cleanup() {

    Services.obs.removeObserver(this.boundObserve, TOPIC_SETUPSTATE_CHANGED);

  }
@@ -120,35 +145,6 @@ class TabPickupContainer extends HTMLDetailsElement { 
    return this.querySelector(".confirmation-message-box");

  }



  insertTemplatedElement(templateId, elementId, replaceNode) {

    const template = document.getElementById(templateId);

    const templateContent = template.content;

    const cloned = templateContent.cloneNode(true);

    if (elementId) {

      // populate id-prefixed attributes on elements that need them

      for (let elem of cloned.querySelectorAll("[data-prefix]")) {

        let [name, value] = elem.dataset.prefix

          .split(":")

          .map(str => str.trim());

        elem.setAttribute(name, elementId + value);

        delete elem.dataset.prefix;

      }

      for (let elem of cloned.querySelectorAll("a[data-support-url]")) {

        elem.href =

          Services.urlFormatter.formatURLPref("app.support.baseURL") +

          elem.dataset.supportUrl;

      }

    }

    if (replaceNode) {

      if (typeof replaceNode == "string") {

        replaceNode = document.getElementById(replaceNode);

      }

      this.replaceChild(cloned, replaceNode);

    } else {

      this.appendChild(cloned);

    }

  }



  update({

    stateIndex = TabsSetupFlowManager.uiStateIndex,

    showMobilePromo = TabsSetupFlowManager.shouldShowMobilePromo,
@@ -170,6 +166,12 @@ class TabPickupContainer extends HTMLDetailsElement { 
      this._showMobilePairSuccess = showMobilePairSuccess;

      needsRender = true;

    }

    if (stateIndex == 4 && this._currentSetupStateIndex !== stateIndex) {

      // trigger an initial request for the synced tabs list

      this.tabListAdded.then(() => {

        this.tabPickupListElem.getSyncedTabData();

      });

    }

    if (stateIndex !== this._currentSetupStateIndex || stateIndex == 0) {

      this._currentSetupStateIndex = stateIndex;

      needsRender = true;
@@ -289,17 +291,7 @@ class TabPickupContainer extends HTMLDetailsElement { 


    // show/hide either the setup or tab list containers, creating each as necessary

    if (stateIndex < 4) {

      if (!setupElem) {

        this.insertTemplatedElement(

          "sync-setup-template",

          "tabpickup-steps",

          "sync-setup-placeholder"

        );

        setupElem = this.setupContainerElem;

      }

      if (tabsElem) {

      tabsElem.hidden = true;

      }

      setupElem.hidden = false;

      setupElem.selectedViewName = `sync-setup-view${stateIndex}`;
@@ -309,17 +301,7 @@ class TabPickupContainer extends HTMLDetailsElement { 
      return;

    }



    if (!tabsElem) {

      this.insertTemplatedElement(

        "synced-tabs-template",

        "tabpickup-tabs-container",

        "synced-tabs-placeholder"

      );

      tabsElem = this.tabsContainerElem;

    }

    if (setupElem) {

    setupElem.hidden = true;

    }

    tabsElem.hidden = false;

    tabsElem.classList.toggle("loading", isLoading);

  }

browser/components/firefoxview/tab-pickup-list.mjs

+6 −6
Original line number Diff line number Diff line
@@ -26,7 +26,9 @@ class TabPickupList extends HTMLElement { 
  constructor() {

    super();

    this.maxTabsLength = 3;

    this.boundObserve = (...args) => this.getSyncedTabData(...args);

    this.boundObserve = (...args) => {

      this.getSyncedTabData(...args);

    };



    // The recency timestamp update period is stored in a pref to allow tests to easily change it

    XPCOMUtils.defineLazyPreferenceGetter(
@@ -62,9 +64,10 @@ class TabPickupList extends HTMLElement { 
    );



    this.addEventListener("click", this);

    this.getSyncedTabData();



    Services.obs.addObserver(this.boundObserve, SYNCED_TABS_CHANGED);



    // inform ancestor elements our getSyncedTabData method is available to fetch data

    this.dispatchEvent(new CustomEvent("list-ready", { bubbles: true }));

  }



  handleEvent(event) {
@@ -212,9 +215,6 @@ class TabPickupList extends HTMLElement { 


  updateTabsList(syncedTabs) {

    // don't do anything while the loading state is active

    if (this.tabPickupContainer.classList.contains("loading")) {

      return;

    }



    while (this.tabsList.firstChild) {

      this.tabsList.firstChild.remove();

browser/components/firefoxview/tests/browser/browser_tab_pickup_list.js

+169 −132
Original line number Diff line number Diff line
@@ -96,14 +96,19 @@ registerCleanupFunction(async function() { 
});



add_task(async function test_tab_list_ordering() {

  await withFirefoxView({}, async browser => {

    const { document } = browser.contentWindow;



  const sandbox = setupRecentDeviceListMocks();

  const syncedTabsMock = sandbox.stub(SyncedTabs, "getRecentTabs");

  let mockTabs1 = getMockTabData(syncedTabsData1);

  let mockTabs2 = getMockTabData(syncedTabsData2);

    syncedTabsMock.returns(mockTabs1);

  syncedTabsMock.callsFake(() => {

    info(

      `Stubbed SyncedTabs.getRecentTabs returning a promise that resolves to ${mockTabs1.length} tabs\n`

    );

    return Promise.resolve(mockTabs1);

  });



  await withFirefoxView({}, async browser => {

    const { document } = browser.contentWindow;



    await setupListState(browser);
@@ -169,14 +174,19 @@ add_task(async function test_tab_list_ordering() { 
});



add_task(async function test_empty_list_items() {

  await withFirefoxView({}, async browser => {

    const { document } = browser.contentWindow;



  const sandbox = setupRecentDeviceListMocks();

  const syncedTabsMock = sandbox.stub(SyncedTabs, "getRecentTabs");

  let mockTabs1 = getMockTabData(syncedTabsData3);

  let mockTabs2 = getMockTabData(syncedTabsData4);

    syncedTabsMock.returns(mockTabs1);

  syncedTabsMock.callsFake(() => {

    info(

      `Stubbed SyncedTabs.getRecentTabs returning a promise that resolves to ${mockTabs1.length} tabs\n`

    );

    return Promise.resolve(mockTabs1);

  });



  await withFirefoxView({}, async browser => {

    const { document } = browser.contentWindow;



    await setupListState(browser);
@@ -246,17 +256,22 @@ add_task(async function test_empty_list_items() { 


add_task(async function test_empty_list() {

  await clearAllParentTelemetryEvents();

  await withFirefoxView({}, async browser => {

    const { document } = browser.contentWindow;



  const sandbox = setupRecentDeviceListMocks();

  const syncedTabsMock = sandbox.stub(SyncedTabs, "getRecentTabs");

  let mockTabs1 = getMockTabData([]);

  let mockTabs2 = getMockTabData(syncedTabsData4);

    syncedTabsMock.returns(mockTabs1);

  syncedTabsMock.callsFake(() => {

    info(

      `Stubbed SyncedTabs.getRecentTabs returning a promise that resolves to ${mockTabs1.length} tabs\n`

    );

    return Promise.resolve(mockTabs1);

  });



    await setupListState(browser);

  await withFirefoxView({}, async browser => {

    const { document } = browser.contentWindow;



    await setupListState(browser);

    info("setupListState complete, checking placeholder and list visibility");

    testVisibility(browser, {

      expectedVisible: {

        "#synced-tabs-placeholder": true,
@@ -290,7 +305,12 @@ add_task(async function test_empty_list() { 
      { clear: true, process: "parent" }

    );



    syncedTabsMock.returns(mockTabs2);

    syncedTabsMock.callsFake(() => {

      info(

        `Stubbed SyncedTabs.getRecentTabs returning a promise that resolves to ${mockTabs2.length} tabs\n`

      );

      return Promise.resolve(mockTabs2);

    });

    // Initiate a synced tabs update

    Services.obs.notifyObservers(null, "services.sync.tabs.changed");
@@ -322,7 +342,12 @@ add_task(async function test_time_updates_correctly() { 
  const sandbox = setupRecentDeviceListMocks();

  const syncedTabsMock = sandbox.stub(SyncedTabs, "getRecentTabs");

  let mockTabs1 = getMockTabData(syncedTabsData5);

  syncedTabsMock.returns(mockTabs1);

  syncedTabsMock.callsFake(() => {

    info(

      `Stubbed SyncedTabs.getRecentTabs returning a promise that resolves to ${mockTabs1.length} tabs\n`

    );

    return Promise.resolve(mockTabs1);

  });



  await withFirefoxView({}, async browser => {

    const { document } = browser.contentWindow;
@@ -422,15 +447,20 @@ add_task(async function test_time_updates_correctly() { 
 * on page reload.

 */

add_task(async function test_tabs_sync_on_user_page_reload() {

  await withFirefoxView({}, async browser => {

    let reloadButton = browser.ownerDocument.getElementById("reload-button");



  const sandbox = setupRecentDeviceListMocks();

  sandbox.stub(SyncedTabs._internal, "syncTabs").resolves(true);

  const syncedTabsMock = sandbox.stub(SyncedTabs, "getRecentTabs");

  let mockTabs1 = getMockTabData(syncedTabsData1);

  let expectedTabsAfterReload = getMockTabData(syncedTabsData3);

    syncedTabsMock.returns(mockTabs1);

  syncedTabsMock.callsFake(() => {

    info(

      `Stubbed SyncedTabs.getRecentTabs returning a promise that resolves to ${mockTabs1.length} tabs\n`

    );

    return Promise.resolve(mockTabs1);

  });



  await withFirefoxView({}, async browser => {

    let reloadButton = browser.ownerDocument.getElementById("reload-button");



    await setupListState(browser);
@@ -459,18 +489,26 @@ add_task(async function test_tabs_sync_on_user_page_reload() { 
    sandbox.restore();

    cleanup_tab_pickup();

  });

});



add_task(async function test_keyboard_navigation() {

  // Setting this pref allows the test to run as expected on MacOS

  await SpecialPowers.pushPrefEnv({ set: [["accessibility.tabfocus", 7]] });

  TabsSetupFlowManager.resetInternalState();

    await withFirefoxView({}, async browser => {

      const { document } = browser.contentWindow;

      let win = browser.ownerGlobal;



  const sandbox = setupRecentDeviceListMocks();

  const syncedTabsMock = sandbox.stub(SyncedTabs, "getRecentTabs");

  let mockTabs1 = getMockTabData(syncedTabsData1);

      syncedTabsMock.returns(mockTabs1);

  syncedTabsMock.callsFake(() => {

    info(

      `Stubbed SyncedTabs.getRecentTabs returning a promise that resolves to ${mockTabs1.length} tabs\n`

    );

    return Promise.resolve(mockTabs1);

  });



  await withFirefoxView({}, async browser => {

    const { document } = browser.contentWindow;

    let win = browser.ownerGlobal;



    await setupListState(browser);

    const tab = (shiftKey = false) => {
@@ -567,4 +605,3 @@ add_task(async function test_tabs_sync_on_user_page_reload() { 
    cleanup_tab_pickup();

  });

});
 
});

browser/components/firefoxview/tests/browser/head.js

+2 −4
Original line number Diff line number Diff line
@@ -332,10 +332,7 @@ async function setupListState(browser) { 
  const tabsContainer = browser.contentWindow.document.querySelector(

    "#tabpickup-tabs-container"

  );

  // fake a sync completion, as UIState.UI_UPDATE with a signed-in status will have

  // triggered a sync when we have 0 tabs

  Services.obs.notifyObservers(null, "weave:service:sync:finish");



  await tabsContainer.tabListAdded;

  await BrowserTestUtils.waitForMutationCondition(

    tabsContainer,

    { attributeFilter: ["class"], attributes: true },
@@ -343,6 +340,7 @@ async function setupListState(browser) { 
      return !tabsContainer.classList.contains("loading");

    }

  );

  info("tabsContainer isn't loading anymore, returning");

}



function checkMobilePromo(browser, expected = {}) {