Skip to content

Commit

Permalink
Bug 1766714 - Advance fxview sync setup to next state when services.s…
Browse files Browse the repository at this point in the history 
…ync.engine.tabs is enabled. r=Gijs

* Observe the 'services.sync.engine.tabs' pref and skip the tab sync step when it is enabled
* Hook up the primary button on that step to enable the pref
* Add tests for both paths through this setup

Differential Revision: https://phabricator.services.mozilla.com/D146003
  • Loading branch information
@sfoster
sfoster committed May 19, 2022
1 parent d73e62a commit 394a570
Show file tree
Hide file tree
Showing 2 changed files with 117 additions and 46 deletions.
27 changes: 18 additions & 9 deletions browser/components/firefoxview/tabs-pickup.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const { switchToTabHavingURI } = window.docShell.chromeEventHandler.ownerGlobal;
const { XPCOMUtils } = ChromeUtils.import(
"resource://gre/modules/XPCOMUtils.jsm"
);
const SYNC_TABS_PREF = "services.sync.engine.tabs";

const tabsSetupFlowManager = new (class {
constructor() {
Expand All @@ -28,6 +29,17 @@ const tabsSetupFlowManager = new (class {
"resource://services-sync/UIState.jsm"
);

// this.syncTabsPrefEnabled will track the value of the tabs pref
XPCOMUtils.defineLazyPreferenceGetter(
this,
"syncTabsPrefEnabled",
SYNC_TABS_PREF,
false,
() => {
this.maybeUpdateUI();
}
);

this.registerSetupState({
uiStateIndex: 0,
name: "not-signed-in",
Expand All @@ -47,9 +59,7 @@ const tabsSetupFlowManager = new (class {
uiStateIndex: 2,
name: "disabled-tab-sync",
exitConditions: () => {
// Bug 1763139 - Implement the actual logic to advance to next step
// This will basically be a check for the pref to enable tab-syncing
return false;
return this.syncTabsPrefEnabled;
},
});
this.registerSetupState({
Expand All @@ -62,7 +72,7 @@ const tabsSetupFlowManager = new (class {
});
this.registerSetupState({
uiStateIndex: 4,
name: "show-synced-tabs-agreement",
name: "show-synced-tabs-loading",
exitConditions: () => {
// Bug 1763139 - Implement the actual logic to advance to next step
return false;
Expand Down Expand Up @@ -165,15 +175,14 @@ const tabsSetupFlowManager = new (class {
switchToTabHavingURI(url, true);
}
syncOpenTabs(containerElem) {
// Bug 1763139 - Implement the actual logic to advance to next step
this.elem.updateSetupState(
this.setupState.get("synced-tabs-not-ready").uiStateIndex
);
// Flip the pref on.
// The observer should trigger re-evaluating state and advance to next step
this.Services.prefs.setBoolPref(SYNC_TABS_PREF, true);
}
confirmSetupComplete(containerElem) {
// Bug 1763139 - Implement the actual logic to advance to next step
this.elem.updateSetupState(
this.setupState.get("show-synced-tabs-agreement").uiStateIndex
this.setupState.get("show-synced-tabs-loading").uiStateIndex
);
}
})();
Expand Down
136 changes: 99 additions & 37 deletions browser/components/firefoxview/tests/browser/browser_setup_state.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,34 @@ function setupMocks({ fxaDevices = null, state = UIState.STATUS_SIGNED_IN }) {
return sandbox;
}

function testSetupState(browser, expected) {
async function waitForVisibleStep(browser, expected) {
const { document } = browser.contentWindow;
for (let [selector, shouldBeVisible] of Object.entries(
expected.expectedVisible
)) {
const elem = document.querySelector(selector);
if (shouldBeVisible) {

const deck = document.querySelector(".sync-setup-container");
const nextStepElem = deck.querySelector(expected.expectedVisible);
const stepElems = deck.querySelectorAll(".setup-step");

await BrowserTestUtils.waitForMutationCondition(
deck,
{
attributeFilter: ["selected-view"],
},
() => {
return BrowserTestUtils.is_visible(nextStepElem);
}
);

for (let elem of stepElems) {
if (elem == nextStepElem) {
ok(
BrowserTestUtils.is_visible(elem),
`Expected ${selector} to be visible`
`Expected ${elem.id || elem.className} to be visible`
);
} else {
ok(BrowserTestUtils.is_hidden(elem), `Expected ${selector} to be hidden`);
ok(
BrowserTestUtils.is_hidden(elem),
`Expected ${elem.id || elem.className} to be hidden`
);
}
}
}
Expand All @@ -58,6 +73,11 @@ add_setup(async function() {
);
registerCleanupFunction(async function() {
BrowserTestUtils.removeTab(tab);
Services.prefs.clearUserPref("services.sync.engine.tabs");
});
// set tab sync false so we don't skip setup states
await SpecialPowers.pushPrefEnv({
set: [["services.sync.engine.tabs", false]],
});
});

Expand All @@ -66,13 +86,8 @@ add_task(async function test_unconfigured_initial_state() {
const sandbox = setupMocks({ state: UIState.STATUS_NOT_CONFIGURED });

Services.obs.notifyObservers(null, UIState.ON_UPDATE);
testSetupState(browser, {
expectedVisible: {
"#tabpickup-steps-view0": true,
"#tabpickup-steps-view1": false,
"#tabpickup-steps-view2": false,
"#tabpickup-steps-view3": false,
},
await waitForVisibleStep(browser, {
expectedVisible: "#tabpickup-steps-view0",
});

sandbox.restore();
Expand All @@ -91,16 +106,11 @@ add_task(async function test_signed_in() {
},
],
});

Services.obs.notifyObservers(null, UIState.ON_UPDATE);
testSetupState(browser, {
expectedVisible: {
"#tabpickup-steps-view0": false,
"#tabpickup-steps-view1": true,
"#tabpickup-steps-view2": false,
"#tabpickup-steps-view3": false,
},
await waitForVisibleStep(browser, {
expectedVisible: "#tabpickup-steps-view1",
});

is(fxAccounts.device.recentDeviceList?.length, 1, "Just 1 device connected");

sandbox.restore();
Expand All @@ -126,14 +136,10 @@ add_task(async function test_2nd_desktop_connected() {
});

Services.obs.notifyObservers(null, UIState.ON_UPDATE);
testSetupState(browser, {
expectedVisible: {
"#tabpickup-steps-view0": false,
"#tabpickup-steps-view1": true,
"#tabpickup-steps-view2": false,
"#tabpickup-steps-view3": false,
},
await waitForVisibleStep(browser, {
expectedVisible: "#tabpickup-steps-view1",
});

is(fxAccounts.device.recentDeviceList?.length, 2, "2 devices connected");
ok(
fxAccounts.device.recentDeviceList?.every(
Expand Down Expand Up @@ -163,16 +169,17 @@ add_task(async function test_mobile_connected() {
},
],
});
// ensure tab sync is false so we don't skip onto next step
ok(
!Services.prefs.getBoolPref("services.sync.engine.tabs", false),
"services.sync.engine.tabs is initially false"
);

Services.obs.notifyObservers(null, UIState.ON_UPDATE);
testSetupState(browser, {
expectedVisible: {
"#tabpickup-steps-view0": false,
"#tabpickup-steps-view1": false,
"#tabpickup-steps-view2": true,
"#tabpickup-steps-view3": false,
},
await waitForVisibleStep(browser, {
expectedVisible: "#tabpickup-steps-view2",
});

is(fxAccounts.device.recentDeviceList?.length, 2, "2 devices connected");
ok(
fxAccounts.device.recentDeviceList?.some(
Expand All @@ -183,3 +190,58 @@ add_task(async function test_mobile_connected() {

sandbox.restore();
});

add_task(async function test_tab_sync_enabled() {
const browser = gBrowser.selectedBrowser;
const sandbox = setupMocks({
state: UIState.STATUS_SIGNED_IN,
fxaDevices: [
{
id: 1,
name: "This Device",
isCurrentDevice: true,
type: "desktop",
},
{
id: 2,
name: "Other Device",
type: "mobile",
},
],
});
Services.obs.notifyObservers(null, UIState.ON_UPDATE);

// test initial state, with the pref not enabled
await waitForVisibleStep(browser, {
expectedVisible: "#tabpickup-steps-view2",
});

// test with the pref toggled on
await SpecialPowers.pushPrefEnv({
set: [["services.sync.engine.tabs", true]],
});
await waitForVisibleStep(browser, {
expectedVisible: "#tabpickup-steps-view3",
});

// reset and test clicking the action button
await SpecialPowers.popPrefEnv();
await waitForVisibleStep(browser, {
expectedVisible: "#tabpickup-steps-view2",
});

const actionButton = browser.contentWindow.document.querySelector(
"#tabpickup-steps-view2 button.primary"
);
actionButton.click();
await waitForVisibleStep(browser, {
expectedVisible: "#tabpickup-steps-view3",
});
ok(
Services.prefs.getBoolPref("services.sync.engine.tabs", false),
"tab sync pref should be enabled after button click"
);

sandbox.restore();
Services.prefs.clearUserPref("services.sync.engine.tabs");
});

0 comments on commit 394a570

Please sign in to comment.