From f16c7a4e5d125b440cc628285341270e038a28f7 Mon Sep 17 00:00:00 2001
From: Kris Maglione <maglione.k@gmail.com>
Date: Wed, 27 Jun 2018 16:44:22 -0700
Subject: [PATCH] Bug 1463587: Part 6 - Add an idle flush task to
 WritableSharedMap. r=erahm

MozReview-Commit-ID: 8Ht7zHo4PD6

--HG--
extra : rebase_source : daaaec86b027d329296e86d508b52342aca64bc0
---
 dom/ipc/SharedMap.cpp           | 16 +++++++++++++
 dom/ipc/SharedMap.h             | 18 ++++++++++-----
 dom/ipc/tests/test_sharedMap.js | 40 +++++++++++++++++++++++++++------
 3 files changed, 61 insertions(+), 13 deletions(-)

diff --git a/dom/ipc/SharedMap.cpp b/dom/ipc/SharedMap.cpp
index 72f61e31cd16..ac6b4f0d7b8c 100644
--- a/dom/ipc/SharedMap.cpp
+++ b/dom/ipc/SharedMap.cpp
@@ -460,12 +460,28 @@ WritableSharedMap::Flush()
 }
 
 void
+WritableSharedMap::IdleFlush()
+{
+  mPendingFlush = false;
+  Flush();
+}
+
+nsresult
 WritableSharedMap::KeyChanged(const nsACString& aName)
 {
   if (!mChangedKeys.ContainsSorted(aName)) {
     mChangedKeys.InsertElementSorted(aName);
   }
   mEntryArray.reset();
+
+  if (!mPendingFlush) {
+      MOZ_TRY(NS_IdleDispatchToCurrentThread(
+        NewRunnableMethod("WritableSharedMap::IdleFlush",
+                          this,
+                          &WritableSharedMap::IdleFlush)));
+      mPendingFlush = true;
+  }
+  return NS_OK;
 }
 
 
diff --git a/dom/ipc/SharedMap.h b/dom/ipc/SharedMap.h
index ac6e36b386f4..212204f75b7a 100644
--- a/dom/ipc/SharedMap.h
+++ b/dom/ipc/SharedMap.h
@@ -38,9 +38,11 @@ namespace ipc {
  * they are updated, and lazily decoded each time they're read.
  *
  * Updates are batched. Rather than each key change triggering an immediate
- * update, combined updates are broadcast after a delay. Currently, this
- * requires an explicit flush() call, or spawning a new content process. In the
- * future, it will happen automatically in idle tasks.
+ * update, combined updates are broadcast after a delay. Changes are flushed
+ * immediately any time a new process is created. Additionally, any time a key
+ * is changed, a flush task is scheduled for the next time the event loop
+ * becomes idle. Changes can be flushed immediately by calling the flush()
+ * method.
  *
  *
  * Whenever a read-only SharedMap is updated, it dispatches a "change" event.
@@ -366,18 +368,22 @@ private:
 
   RefPtr<SharedMap> mReadOnly;
 
+  bool mPendingFlush = false;
+
   // Creates a new snapshot of the map, and updates all Entry instance to
   // reference its data.
   Result<Ok, nsresult> Serialize();
 
+  void IdleFlush();
+
   // If there have been any changes since the last snapshot, creates a new
   // serialization and broadcasts it to all child SharedMap instances.
   void BroadcastChanges();
 
   // Marks the given (UTF-8 encoded) key as having changed. This adds it to
-  // mChangedKeys, if not already present. In the future, it will also schedule
-  // a flush the next time the event loop is idle.
-  void KeyChanged(const nsACString& aName);
+  // mChangedKeys, if not already present, and schedules a flush for the next
+  // time the event loop is idle.
+  nsresult KeyChanged(const nsACString& aName);
 };
 
 } // ipc
diff --git a/dom/ipc/tests/test_sharedMap.js b/dom/ipc/tests/test_sharedMap.js
index 6622b3bf81a8..df33fc15717a 100644
--- a/dom/ipc/tests/test_sharedMap.js
+++ b/dom/ipc/tests/test_sharedMap.js
@@ -46,13 +46,15 @@ function checkParentMap(expected) {
   checkMap(getContents(Services.ppmm.sharedData), expected);
 }
 
-async function checkContentMaps(expected) {
+async function checkContentMaps(expected, parentOnly = false) {
   info("Checking in-process content map");
   checkMap(getContents(Services.cpmm.sharedData), expected);
 
-  info("Checking out-of-process content map");
-  let contents = await contentPage.spawn(undefined, getContents);
-  checkMap(contents, expected);
+  if (!parentOnly) {
+    info("Checking out-of-process content map");
+    let contents = await contentPage.spawn(undefined, getContents);
+    checkMap(contents, expected);
+  }
 }
 
 add_task(async function setup() {
@@ -108,22 +110,26 @@ add_task(async function test_sharedMap() {
 
   setKey("baz-a", {meh: "meh"});
 
+  // When we do several checks in a row, we can't check the values in
+  // the content process, since the async checks may allow the idle
+  // flush task to run, and update it before we're ready.
+
   checkParentMap(expected);
-  await checkContentMaps(oldExpected);
+  checkContentMaps(oldExpected, true);
 
   info("Add another entry. Check that both new entries are only available in the parent");
 
   setKey("baz-a", {meh: 12});
 
   checkParentMap(expected);
-  await checkContentMaps(oldExpected);
+  checkContentMaps(oldExpected, true);
 
   info("Delete an entry. Check that all changes are only visible in the parent");
 
   deleteKey("foo-b");
 
   checkParentMap(expected);
-  await checkContentMaps(oldExpected);
+  checkContentMaps(oldExpected, true);
 
   info("Flush. Check that all entries are available in both parent and children");
 
@@ -131,4 +137,24 @@ add_task(async function test_sharedMap() {
 
   checkParentMap(expected);
   await checkContentMaps(expected);
+
+
+  info("Test that entries are automatically flushed on idle:");
+
+  info("Add a new entry. Check that it is initially only available in the parent");
+
+  // Test the idle flush task.
+  oldExpected = Array.from(expected);
+
+  setKey("thing", "stuff");
+
+  checkParentMap(expected);
+  checkContentMaps(oldExpected, true);
+
+  info("Wait for an idle timeout. Check that changes are now visible in all children");
+
+  await new Promise(resolve => ChromeUtils.idleDispatch(resolve));
+
+  checkParentMap(expected);
+  await checkContentMaps(expected);
 });