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 229 matching lines @@
       Synchronizer.execute(subscription);
   }
 
   // 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(() =>

[Thomas Greiner, 2018/04/27 14:50:13]

Personally, I'd love to do that check before running any of our initialization
code but I understand that this would require further discussion so for the sake
of the hotfix feel free to ignore this comment.

[Sebastian Noack, 2018/04/27 16:01:04]

I guess we could add a dummy preference, and try to read/write in order to
detect the data corruption before the actual initialization, but this seems to
make things more complicated, and potentially less robust since theoretically at
least reading/writing the dummy preference might succeed while the IO operations
here still might fail.

[Thomas Greiner, 2018/04/27 16:27:33]

Agreed that more thought needs to be put into it so if you agree I'll create a
separate ticket for that for later.

[Sebastian Noack, 2018/04/27 16:33:22]

Well, if you have any specific idea that would account for my concerns, sure.

     {
       dataCorrupted = true;
     }).then(() =>
     {
       if (!Prefs.suppress_first_run_page)
       {
         // Always show the first run page if a data corruption was detected
-        // (either though failure of reading from or writing to storage.local).

[Thomas Greiner, 2018/04/27 14:50:13]

Typo: Replace "though" with "through".

[Sebastian Noack, 2018/04/27 16:01:04]

Done.

+        // (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(() => { dataCorrupted = true; })
 ]).then(detectFirstRun)
   .then(getSubscriptions)
   .then(addSubscriptionsAndNotifyUser)
-  .then(() => { require("./uninstall").setUninstallURL() })

[Sebastian Noack, 2018/04/26 18:25:35]

I found an issue with the circular dependency of the uninstall and
subscriptionInit modules. Without this change, I get "isDataCorrupted is not a
function" in the background page log, causing the uninstall page to not show.

[Thomas Greiner, 2018/04/27 14:50:13]

Acknowledged. Mind adding an inline comment for that?

[Sebastian Noack, 2018/04/27 16:01:04]

Done.

+  // 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.
  *
  * @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;
 };