From 9a1b56a899575d71e07eb3466e5fbf16d5a18571 Mon Sep 17 00:00:00 2001
From: mrmr1993
Date: Mon, 1 May 2017 00:15:51 +0100
Subject: FF - Fix updates from the exclusions popup

This stops |Exclusions| from holding a reference to the |value|
parameter passed to |Settings.set|. In Firefox, this object is garbage
collected when the owning context (the exclusions popup) is closed.

The fix for all such cases in the future is to switch to using
|Settings.get|, which implicitly does |JSON.parse JSON.stringify value|
and thus returns an object in the same context as |Settings|.

We could fix this generally by doing this for the
|Settings.performPostUpdateHook| call in |Settings.set| instead.
However, I'm not convinced that it warrants the overhead of a
|JSON.parse| for every |Settings.set| call.
---
 background_scripts/exclusions.coffee   | 9 ++++++---
 lib/settings.coffee                    | 3 +++
 tests/unit_tests/exclusion_test.coffee | 5 +++--
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/background_scripts/exclusions.coffee b/background_scripts/exclusions.coffee
index 42d3b872..fec66f4c 100644
--- a/background_scripts/exclusions.coffee
+++ b/background_scripts/exclusions.coffee
@@ -20,12 +20,12 @@ Exclusions =
   # Make RegexpCache, which is required on the page popup, accessible via the Exclusions object.
   RegexpCache: RegexpCache
 
-  rules: Settings.get("exclusionRules")
+  rules: Settings.get "exclusionRules"
 
   # Merge the matching rules for URL, or null.  In the normal case, we use the configured @rules; hence, this
   # is the default.  However, when called from the page popup, we are testing what effect candidate new rules
   # would have on the current tab.  In this case, the candidate rules are provided by the caller.
-  getRule: (url, rules=@rules) ->
+  getRule: (url, rules = @rules) ->
     matchingRules = (rule for rule in rules when rule.pattern and 0 <= url.search RegexpCache.get rule.pattern)
     # An absolute exclusion rule (one with no passKeys) takes priority.
     for rule in matchingRules
@@ -47,7 +47,10 @@ Exclusions =
     @rules = rules.filter (rule) -> rule and rule.pattern
     Settings.set "exclusionRules", @rules
 
-  postUpdateHook: (@rules) ->
+  postUpdateHook: (rules) ->
+    # NOTE(mrmr1993): In FF, the |rules| argument will be garbage collected when the exclusions popup is
+    # closed. Do NOT store it/use it asynchronously.
+    @rules = Settings.get "exclusionRules"
     RegexpCache.clear()
 
 # Register postUpdateHook for exclusionRules setting.
diff --git a/lib/settings.coffee b/lib/settings.coffee
index 9fa27c5f..08986723 100644
--- a/lib/settings.coffee
+++ b/lib/settings.coffee
@@ -89,6 +89,9 @@ Settings =
         # Remove options installed by the "copyNonDefaultsToChromeStorage-20150717" migration; see below.
         @log "   chrome.storage.local.remove(#{key})"
         chrome.storage.local.remove key
+    # NOTE(mrmr1993): In FF, |value| will be garbage collected when the page owning it is unloaded.
+    # Any postUpdateHooks that can be called from the options page/exclusions popup should be careful not to
+    # use |value| asynchronously, or else it may refer to a |DeadObject| and accesses will throw an error.
     @performPostUpdateHook key, value
 
   clear: (key) ->
diff --git a/tests/unit_tests/exclusion_test.coffee b/tests/unit_tests/exclusion_test.coffee
index f53d23f6..06e0a51f 100644
--- a/tests/unit_tests/exclusion_test.coffee
+++ b/tests/unit_tests/exclusion_test.coffee
@@ -29,7 +29,7 @@ isEnabledForUrl = (request) ->
 context "Excluded URLs and pass keys",
 
   setup ->
-    Exclusions.postUpdateHook(
+    Settings.set "exclusionRules",
       [
         { pattern: "http*://mail.google.com/*", passKeys: "" }
         { pattern: "http*://www.facebook.com/*", passKeys: "abab" }
@@ -39,7 +39,8 @@ context "Excluded URLs and pass keys",
         { pattern: "http*://www.example.com/*", passKeys: "a bb c bba a" }
         { pattern: "http*://www.duplicate.com/*", passKeys: "ace" }
         { pattern: "http*://www.duplicate.com/*", passKeys: "bdf" }
-      ])
+      ]
+    Exclusions.postUpdateHook()
 
   should "be disabled for excluded sites", ->
     rule = isEnabledForUrl({ url: 'http://mail.google.com/calendar/page' })
-- 
cgit v1.2.3