Bug 1403052 - Limit tab sync max_record_payload_size to 512k. r?markh draft
authorThom Chiovoloni <tchiovoloni@mozilla.com>
Mon, 02 Oct 2017 19:27:54 -0400
changeset 673851 e2ab0ecf73e4de6920401c11fd08a5b1699cd6e2
parent 673722 4efdb5f6cfc4a9a9529b5e10b84087aa26221c3b
child 734178 41e5392c6b34e9c348d692c7170fad775e2bf2e3
push id82673
push userbmo:tchiovoloni@mozilla.com
push dateMon, 02 Oct 2017 23:29:47 +0000
reviewersmarkh
bugs1403052
milestone58.0a1
Bug 1403052 - Limit tab sync max_record_payload_size to 512k. r?markh Also fixes an issue where we wouldn't encode to utf8 when comparing the actual size to the limit after the first time. MozReview-Commit-ID: Cf3byjI1FTZ
services/sync/modules/engines/tabs.js
services/sync/tests/unit/test_tab_store.js
--- a/services/sync/modules/engines/tabs.js
+++ b/services/sync/modules/engines/tabs.js
@@ -200,39 +200,46 @@ TabStore.prototype = {
           lastUsed: Math.floor((tabState.lastAccessed || 0) / 1000),
         });
       }
     }
 
     return allTabs;
   },
 
+  getMaxRecordPayloadSize() {
+    // Tabs have a different max size due to being stored using memcached on the
+    // server (See bug 1403052), but we still check the server config to make
+    // sure we respect the global limits it sets.
+    return Math.min(512 * 1024, this.engine.service.getMaxRecordPayloadSize());
+  },
+
   async createRecord(id, collection) {
     let record = new TabSetRecord(collection, id);
     record.clientName = this.engine.service.clientsEngine.localName;
 
     // Sort tabs in descending-used order to grab the most recently used
     let tabs = this.getAllTabs(true).sort(function(a, b) {
       return b.lastUsed - a.lastUsed;
     });
-
+    let encoder = new TextEncoder("utf-8");
     // Figure out how many tabs we can pack into a payload.
     // We use byteLength here because the data is not encrypted in ascii yet.
-    let size = new TextEncoder("utf-8").encode(JSON.stringify(tabs)).byteLength;
+    let size = encoder.encode(JSON.stringify(tabs)).byteLength;
     let origLength = tabs.length;
-    const maxPayloadSize = this.engine.service.getMaxRecordPayloadSize();
+    const maxPayloadSize = this.getMaxRecordPayloadSize();
     // See bug 535326 comment 8 for an explanation of the estimation
     const MAX_TAB_SIZE = maxPayloadSize / 4 * 3 - 1500;
     if (size > MAX_TAB_SIZE) {
       // Estimate a little more than the direct fraction to maximize packing
       let cutoff = Math.ceil(tabs.length * MAX_TAB_SIZE / size);
       tabs = tabs.slice(0, cutoff + 1);
 
       // Keep dropping off the last entry until the data fits
-      while (JSON.stringify(tabs).length > MAX_TAB_SIZE)
+      while (encoder.encode(JSON.stringify(tabs)).byteLength > MAX_TAB_SIZE)
         tabs.pop();
     }
 
     this._log.trace("Created tabs " + tabs.length + " of " + origLength);
     tabs.forEach(function(tab) {
       this._log.trace("Wrapping tab: " + JSON.stringify(tab));
     }, this);
 
--- a/services/sync/tests/unit/test_tab_store.js
+++ b/services/sync/tests/unit/test_tab_store.js
@@ -103,9 +103,17 @@ add_task(async function test_createRecor
   ok(record instanceof TabSetRecord);
   equal(record.tabs.length, 1);
 
   _("create a big record");
   store.getWindowEnumerator = mockGetWindowEnumerator.bind(this, "http://foo.com", 1, numtabs);
   record = await store.createRecord("fake-guid");
   ok(record instanceof TabSetRecord);
   equal(record.tabs.length, 2501);
+
+  store.getMaxRecordPayloadSize = () => 512 * 1024;
+  numtabs = 5200
+  _("Modify the max record payload size and create a big record");
+  store.getWindowEnumerator = mockGetWindowEnumerator.bind(this, "http://foo.com", 1, numtabs);
+  record = await store.createRecord("fake-guid");
+  ok(record instanceof TabSetRecord);
+  equal(record.tabs.length, 5021);
 });