Commit cf62b56b authored by Sammy Khamis's avatar Sammy Khamis
Browse files

Bug 1752664 - sync shortly after a tab change event r=Gijs,markh 

Differential Revision: https://phabricator.services.mozilla.com/D137376
parent d724bbe1

browser/app/profile/firefox.js

+5 −0
Original line number Diff line number Diff line
@@ -1430,6 +1430,11 @@ pref("services.sync.prefs.dangerously_allow_arbitrary", false); 
// user's tabs and bookmarks. Note this pref is also synced.

pref("services.sync.syncedTabs.showRemoteIcons", true);



// A preference (in milliseconds) controlling if we sync after a tab change and

// how long to delay before we schedule the sync

// Anything <= 0 means disabled

pref("services.sync.syncedTabs.syncDelayAfterTabChange", 0);



// Whether the character encoding menu is under the main Firefox button. This

// preference is a string so that localizers can alter it.

pref("browser.menu.showCharacterEncoding", "chrome://browser/locale/browser.properties");

services/sync/modules/engines/tabs.js

+26 −5
Original line number Diff line number Diff line
@@ -41,6 +41,16 @@ XPCOMUtils.defineLazyModuleGetters(this, { 
  PlacesUtils: "resource://gre/modules/PlacesUtils.jsm",

});



ChromeUtils.defineModuleGetter(

  this,

  "ExperimentAPI",

  "resource://nimbus/ExperimentAPI.jsm"

);



XPCOMUtils.defineLazyModuleGetters(this, {

  NimbusFeatures: "resource://nimbus/ExperimentAPI.jsm",

});



function TabSetRecord(collection, id) {

  CryptoWrapper.call(this, collection, id);

}
@@ -312,7 +322,7 @@ TabTracker.prototype = { 
    this.modified = false;

  },



  _topics: ["pageshow", "TabOpen", "TabClose", "TabSelect"],

  _topics: ["TabOpen", "TabClose", "TabSelect"],



  _registerListenersForWindow(window) {

    this._log.trace("Registering tab listeners in window");
@@ -385,10 +395,21 @@ TabTracker.prototype = { 
    this._log.trace("onTab event: " + event.type);

    this.modified = true;



    // For page shows, bump the score 10% of the time, emulating a partial

    // score. We don't want to sync too frequently. For all other page

    // events, always bump the score.

    if (event.type != "pageshow" || Math.random() < 0.1) {

    const delayInMs = NimbusFeatures.syncAfterTabChange.getVariable(

      "syncDelayAfterTabChange"

    );



    // If we are part of the experiment don't use score here

    // and instead schedule a sync once we detect a tab change

    //  to ensure the server always has the most up to date tabs

    if (delayInMs > 0) {

      this._log.debug(

        "Detected a tab change: scheduling a sync in " + delayInMs + "ms"

      );

      this.engine.service.scheduler.scheduleNextSync(delayInMs, {

        why: "tabschanged",

      });

    } else {

      this.score += SCORE_INCREMENT_SMALL;

    }

  },

services/sync/tests/unit/test_tab_tracker.js

+123 −5
Original line number Diff line number Diff line
@@ -4,11 +4,36 @@ 
ChromeUtils.import("resource://services-sync/engines/tabs.js");

const { Service } = ChromeUtils.import("resource://services-sync/service.js");



const { SyncScheduler } = ChromeUtils.import(

  "resource://services-sync/policies.js"

);



const { ExperimentFakes } = ChromeUtils.import(

  "resource://testing-common/NimbusTestUtils.jsm"

);



const { ExperimentAPI } = ChromeUtils.import(

  "resource://nimbus/ExperimentAPI.jsm"

);



var scheduler = new SyncScheduler(Service);

let clientsEngine;



async function setupForExperimentFeature() {

  const sandbox = sinon.createSandbox();

  const manager = ExperimentFakes.manager();

  await manager.onStartup();



  sandbox.stub(ExperimentAPI, "_store").get(() => manager.store);



  return { sandbox, manager };

}



add_task(async function setup() {

  await Service.promiseInitialized;

  clientsEngine = Service.clientsEngine;



  scheduler.setDefaults();

});



function fakeSvcWinMediator() {
@@ -65,8 +90,7 @@ add_task(async function run_test() { 
  tracker.start();

  Assert.equal(logs.length, 2);

  for (let log of logs) {

    Assert.equal(log.addTopics.length, 5);

    Assert.ok(log.addTopics.includes("pageshow"));

    Assert.equal(log.addTopics.length, 4);

    Assert.ok(log.addTopics.includes("TabOpen"));

    Assert.ok(log.addTopics.includes("TabClose"));

    Assert.ok(log.addTopics.includes("TabSelect"));
@@ -82,8 +106,7 @@ add_task(async function run_test() { 
  Assert.equal(logs.length, 2);

  for (let log of logs) {

    Assert.equal(log.addTopics.length, 0);

    Assert.equal(log.remTopics.length, 5);

    Assert.ok(log.remTopics.includes("pageshow"));

    Assert.equal(log.remTopics.length, 4);

    Assert.ok(log.remTopics.includes("TabOpen"));

    Assert.ok(log.remTopics.includes("TabClose"));

    Assert.ok(log.remTopics.includes("TabSelect"));
@@ -112,7 +135,7 @@ add_task(async function run_test() { 
  await tracker.clearChangedIDs();

  Assert.ok(!tracker.modified);



  tracker.onTab({ type: "pageshow", originalTarget: "pageshow" });

  tracker.onTab({ type: "TabOpen", originalTarget: "TabOpen" });

  Assert.ok(

    Utils.deepEquals(Object.keys(await engine.getChangedIDs()), [

      clientsEngine.localID,
@@ -147,3 +170,98 @@ add_task(async function run_test() { 
    ])

  );

});



add_task(async function run_sync_on_tab_change_test() {

  let { manager } = await setupForExperimentFeature();



  await manager.onStartup();

  await ExperimentAPI.ready();



  let testExperimentDelay = 5000;

  // We use this to ensure we're using a different value than the experiment

  let prefDelayOffset = 500;



  // This is the fallback pref if we don't have a experiment running

  Svc.Prefs.set(

    "services.sync.syncedTabs.syncDelayAfterTabChange",

    testExperimentDelay + prefDelayOffset

  );



  let doEnrollmentCleanup = await ExperimentFakes.enrollWithFeatureConfig(

    {

      enabled: true,

      featureId: "syncAfterTabChange",

      value: { syncDelayAfterTabChange: testExperimentDelay },

    },

    {

      manager,

    }

  );



  let engine = Service.engineManager.get("tabs");



  _("We assume that tabs have changed at startup.");

  let tracker = engine._tracker;



  Assert.ok(tracker.modified);

  Assert.ok(

    Utils.deepEquals(Object.keys(await engine.getChangedIDs()), [

      clientsEngine.localID,

    ])

  );



  _("Test sync is scheduled after a tab change if experiment is enabled");

  for (let evttype of ["TabOpen", "TabClose", "TabSelect"]) {

    // Send a fake tab event

    tracker.onTab({ type: evttype, originalTarget: evttype });

    // Ensure the tracker fired

    Assert.ok(tracker.modified);

    // We should be scheduling <= experiment value

    Assert.ok(scheduler.nextSync - Date.now() <= testExperimentDelay);

  }

  await doEnrollmentCleanup();



  _("If there is no experiment, fallback to the pref");

  let delayPref = Svc.Prefs.get(

    "services.sync.syncedTabs.syncDelayAfterTabChange"

  );

  let evttype = "TabOpen";

  Assert.equal(delayPref, testExperimentDelay + prefDelayOffset);

  // Fire ontab event

  tracker.onTab({ type: evttype, originalTarget: evttype });



  // Ensure the tracker fired

  Assert.ok(tracker.modified);

  // We should be scheduling <= preference value

  Assert.ok(scheduler.nextSync - Date.now() <= delayPref);



  _("We should not have a sync if experiment if off and pref is 0");



  Svc.Prefs.set("services.sync.syncedTabs.syncDelayAfterTabChange", 0);

  let doAnotherEnrollmentCleanup = await ExperimentFakes.enrollWithFeatureConfig(

    {

      enabled: true,

      featureId: "syncAfterTabChange",

      value: { syncDelayAfterTabChange: 0 },

    },

    {

      manager,

    }

  );

  // Schedule sync a super long time from now

  scheduler.nextSync = Date.now() + 60000;



  // Fire ontab event

  evttype = "TabOpen";

  tracker.onTab({ type: evttype, originalTarget: evttype });

  // Ensure the tracker fired

  Assert.ok(tracker.modified);



  // We should NOT be scheduled for a sync soon

  Assert.ok(scheduler.nextSync - Date.now() > testExperimentDelay);



  // cleanup

  await doAnotherEnrollmentCleanup();

  scheduler.setDefaults();

  Svc.Prefs.resetBranch("");

});

toolkit/components/nimbus/FeatureManifest.yaml

+10 −0
Original line number Diff line number Diff line
@@ -473,6 +473,16 @@ pbNewtab: 
  schema: >-

    browser/components/newtab/content-src/asrouter/templates/PBNewtab/NewtabPromoMessage.schema.json

  variables: {}

syncAfterTabChange:

  description: "Schedule a sync after any tab change"

  hasExposure: false

  isEarlyStartup: false

  variables:

    syncDelayAfterTabChange:

      type: int

      fallbackPref: "services.sync.syncedTabs.syncDelayAfterTabChange"

      description: >-

        "How long to wait (in milliseconds) to schedule a sync after tab change"

pictureinpicture:

  description: Message for first time Picture-in-Picture users

  hasExposure: true