Issue 6599 - Detect data corruption of storage.local

lib/subscriptionInit.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 16 matching lines @@
 const {FilterNotifier} = require("../adblockpluscore/lib/filterNotifier");
 const info = require("../buildtools/info");
 const {Prefs} = require("./prefs");
 const {Synchronizer} = require("../adblockpluscore/lib/synchronizer");
 const {Utils} = require("./utils");
 const {initNotifications} = require("./notificationHelper");
 const {updatesVersion} = require("../adblockplusui/lib/prefs");
 
 let firstRun;
 let subscriptionsCallback = null;
+let reinitialized = false;
+let dataCorrupted = false;
 
 /**
  * If there aren't any filters, the default subscriptions are added.
  * However, if patterns.ini already did exist and/or any preference
  * is set to a non-default value, this indicates that this isn't the
  * first run, but something went wrong.
  *
  * This function detects the first run, and makes sure that the user
  * gets notified (on the first run page) if the data appears incomplete
  * and therefore will be reinitialized.
  */
 function detectFirstRun()
 {
   firstRun = FilterStorage.subscriptions.length == 0;
 
   if (firstRun && (!FilterStorage.firstRun || Prefs.currentVersion))
-    exports.reinitialized = true;
+    reinitialized = true;
 
   Prefs.currentVersion = info.addonVersion;
 }
 
 /**
  * Determines whether to add the default ad blocking subscription.
  * Returns true, if there are no filter subscriptions besides those
  * other subscriptions added automatically, and no custom filters.
  *
  * On first run, this logic should always result in true since there
@@ skipping 74 matching lines @@
 function supportsNotificationsWithButtons()
 {
   // Microsoft Edge (as of EdgeHTML 16) doesn't have the notifications API.
   // Opera gives an asynchronous error when buttons are provided (we cannot
   // detect that behavior without attempting to show a notification).
   if (!("notifications" in browser) || info.application == "opera")
     return false;
 
   // Firefox throws synchronously if the "buttons" option is provided.
   // If buttons are supported (i.e. on Chrome), this fails with
-  // a different error message due to missing required options.
+  // an asynchronous error due to missing required options.
   // https://bugzilla.mozilla.org/show_bug.cgi?id=1190681
   try
   {
-    browser.notifications.create({buttons: []});
+    browser.notifications.create({buttons: []}).catch(() => {});
   }
   catch (e)
   {
     if (e.toString().includes('"buttons" is unsupported'))
       return false;
   }
 
   return true;
 }
 
@@ skipping 55 matching lines @@
           }
         }
 
         return subscriptions;
       });
   }
 
   return subscriptions;
 }
 
-function finishInitialization(subscriptions)
+function addSubscriptionsAndNotifyUser(subscriptions)
 {
   if (subscriptionsCallback)
     subscriptions = subscriptionsCallback(subscriptions);
 
   for (let subscription of subscriptions)
   {
     FilterStorage.addSubscription(subscription);
     if (subscription instanceof DownloadableSubscription &&
         !subscription.lastDownload)
       Synchronizer.execute(subscription);
   }
 
-  if (!Prefs.suppress_first_run_page)
-  {
-    let page = null;
-    if (firstRun || exports.dataCorrupted)

[kzar, 2018/04/24 11:18:01]

Well surely this would mean that the first run page constantly opens instead of
the updates page for these users with corrupted data?

Shouldn't we show a page which explains that their data is corrupted and that
they should try reinstalling the extension? Also I wonder if there's another way
to store a flag to somehow avoid constantly showing the page (or if that's a
good idea)?

[Sebastian Noack, 2018/04/24 11:26:26]

Yes, that is the idea, to add this message to the first run page, but hide it
unless dataCorrupted is set. Note that this how we already handle the
"reinitialized" flag.

[kzar, 2018/04/24 13:34:00]

Fair enough, I guess we need to also update the adblockplusui dependency to
include that with along with this change.

+  // Show first run page or the updates page. The latter is only shown
+  // on Chromium (since the current updates page announces features that
+  // aren't new to Firefox users), and only if this version of the
+  // updates page hasn't been shown yet.
+  if (firstRun || info.platform == "chromium" &&
+                  updatesVersion > Prefs.last_updates_page_displayed)
+  {
+    return Prefs.set("last_updates_page_displayed", updatesVersion).catch(() =>
     {
-      page = "firstRun.html";
-    }
-    // For now we're limiting the updates page to users of
-    // Chromium-based browsers to gage its impact
-    else if (info.platform == "chromium" &&
-             updatesVersion > Prefs.last_updates_page_displayed)
+      dataCorrupted = true;
+    }).then(() =>
     {
-      page = "updates.html";
-    }
-
-    if (page)
-    {
-      browser.tabs.create({url: browser.extension.getURL(page)});
-
-      // For new users and users that have already seen this updates page we
-      // want to avoid showing it again for subsequent updates.
-      Prefs.last_updates_page_displayed = updatesVersion;
-    }
-  }
-
-  initNotifications();
+      if (!Prefs.suppress_first_run_page)
+      {
+        // Always show the first run page if a data corruption was detected
+        // (either through failure of reading from or writing to storage.local).
+        // The first run page notifies the user about the data corruption.
+        let url;
+        if (firstRun || dataCorrupted)
+          url = "firstRun.html";
+        else
+          url = "updates.html";
+        browser.tabs.create({url});
+      }
+    });
+  }
 }
 
 Promise.all([
   FilterNotifier.once("load"),
-  Prefs.untilLoaded.catch(() => { exports.dataCorrupted = true; })
+  Prefs.untilLoaded.catch(() => { dataCorrupted = true; })
 ]).then(detectFirstRun)
   .then(getSubscriptions)
-  .then(finishInitialization);
-
-/**
- * Indicates whether the default filter subscriptions have been added
- * again because there weren't any subscriptions even though this wasn't
+  .then(addSubscriptionsAndNotifyUser)
+  // We have to require the "uninstall" module on demand,
+  // as the "uninstall" module in turn requires this module.
+  .then(() => { require("./uninstall").setUninstallURL(); })
+  .then(initNotifications);
+
+/**
+ * Gets a value indicating whether the default filter subscriptions have been
+ * added again because there weren't any subscriptions even though this wasn't
  * the first run.
  *
- * @type {boolean}
- */
-exports.reinitialized = false;
-
-/**
- * Indicates whether a data corruption was detcted.
- *
- * @type {boolean}
- */
-exports.dataCorrupted = false;
+ * @return {boolean}
+ */
+exports.isReinitialized = () => reinitialized;
+
+/**
+ * Gets a value indicating whether a data corruption was detected.
+ *
+ * @return {boolean}
+ */
+exports.isDataCorrupted = () => dataCorrupted;
 
 /**
  * Sets a callback that is called with an array of subscriptions to be added
  * during initialization. The callback must return an array of subscriptions
  * that will effectively be added.
  *
  * @param {function} callback
  */
 exports.setSubscriptionsCallback = callback =>
 {
   subscriptionsCallback = callback;
 };