Bug 1306784 - Don't set a default panel during fixup migration if all panels are disabled r=sebastian a=ritu
authorAndrzej Hunt <ahunt@mozilla.com>
Tue, 04 Oct 2016 09:12:37 -0700
changeset 350626 f4ae62f16cc7f076762727a17a55b73d43057f8d
parent 350625 23487c75faf7d5fc53202c2cd1628c7134489587
child 350627 99caef326a26a027c42fdf6ac8a28a4d7d76e927
push id1230
push userjlund@mozilla.com
push dateMon, 31 Oct 2016 18:13:35 +0000
treeherdermozilla-release@5e06e3766db2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssebastian, ritu
bugs1306784
milestone50.0
Bug 1306784 - Don't set a default panel during fixup migration if all panels are disabled r=sebastian a=ritu The fixup migration assumes that there must be a default panel, and sets the history panel as default if not. This assumption is wrong in the case where no panels are visible: in this case there is no default. Before this commit, that migration will make the history panel visible for all users who had no visible panels, with this commit that migration will retain the all-panels-hidden state. Note: it's impossible for us to recover the all-panels-hidden state for clients that have already run the migration, however we can at least prevent this issue from happening for users who have not yet migrated (i.e. in release). MozReview-Commit-ID: 9JlmltPW2Ly
mobile/android/base/java/org/mozilla/gecko/home/HomeConfigPrefsBackend.java
--- a/mobile/android/base/java/org/mozilla/gecko/home/HomeConfigPrefsBackend.java
+++ b/mobile/android/base/java/org/mozilla/gecko/home/HomeConfigPrefsBackend.java
@@ -214,27 +214,42 @@ public class HomeConfigPrefsBackend impl
     /**
      * Iterate over all homepanels to verify that there is at least one default panel. If there is
      * no default panel, set History as the default panel. (This is only relevant for two botched
      * migrations where the history panel should have been made the default panel, but wasn't.)
      */
     private static void ensureDefaultPanelForV5orV8(Context context, JSONArray jsonPanels) throws JSONException {
         int historyIndex = -1;
 
+        // If all panels are disabled, there is no default panel - this is the only valid state
+        // that has no default. We can use this flag to track whether any visible panels have been
+        // found.
+        boolean enabledPanelsFound = false;
+
         for (int i = 0; i < jsonPanels.length(); i++) {
             final PanelConfig panelConfig = new PanelConfig(jsonPanels.getJSONObject(i));
             if (panelConfig.isDefault()) {
                 return;
             }
 
+            if (!panelConfig.isDisabled()) {
+                enabledPanelsFound = true;
+            }
+
             if (panelConfig.getType() == PanelType.COMBINED_HISTORY) {
                 historyIndex = i;
             }
         }
 
+        if (!enabledPanelsFound) {
+            // No panels are enabled, hence there can be no default (see noEnabledPanelsFound declaration
+            // for more information).
+            return;
+        }
+
         // Make the History panel default. We can't modify existing PanelConfigs, so make a new one.
         final PanelConfig historyPanelConfig = createBuiltinPanelConfig(context, PanelType.COMBINED_HISTORY, EnumSet.of(PanelConfig.Flags.DEFAULT_PANEL));
         jsonPanels.put(historyIndex, historyPanelConfig.toJSON());
     }
 
     /**
      * Removes a panel from the home panel config.
      * If the removed panel was set as the default home panel, we provide a replacement for it.
@@ -247,46 +262,48 @@ public class HomeConfigPrefsBackend impl
      * @param alwaysUnhide If true, the replacement panel will always be unhidden,
      *                     otherwise only if we turn it into the new default panel.
      * @return new array of updated JSON panels
      * @throws JSONException
      */
     private static JSONArray removePanel(Context context, JSONArray jsonPanels,
                                          PanelType panelToRemove, PanelType replacementPanel, boolean alwaysUnhide) throws JSONException {
         boolean wasDefault = false;
+        boolean wasDisabled = false;
         int replacementPanelIndex = -1;
         boolean replacementWasDefault = false;
 
         // JSONArrary doesn't provide remove() for API < 19, therefore we need to manually copy all
         // the items we don't want deleted into a new array.
         final JSONArray newJSONPanels = new JSONArray();
 
         for (int i = 0; i < jsonPanels.length(); i++) {
             final JSONObject panelJSON = jsonPanels.getJSONObject(i);
             final PanelConfig panelConfig = new PanelConfig(panelJSON);
 
             if (panelConfig.getType() == panelToRemove) {
                 // If this panel was the default we'll need to assign a new default:
                 wasDefault = panelConfig.isDefault();
+                wasDisabled = panelConfig.isDisabled();
             } else {
                 if (panelConfig.getType() == replacementPanel) {
                     replacementPanelIndex = newJSONPanels.length();
                     if (panelConfig.isDefault()) {
                         replacementWasDefault = true;
                     }
                 }
 
                 newJSONPanels.put(panelJSON);
             }
         }
 
         // Unless alwaysUnhide is true, we make the replacement panel visible only if it is going
         // to be the new default panel, since a hidden default panel doesn't make sense.
         // This is to allow preserving the behaviour of the original reading list migration function.
-        if (wasDefault || alwaysUnhide) {
+        if ((wasDefault || alwaysUnhide) && !wasDisabled) {
             final JSONObject replacementPanelConfig;
             if (wasDefault) {
                 // If the removed panel was the default, the replacement has to be made the new default
                 replacementPanelConfig = createBuiltinPanelConfig(context, replacementPanel, EnumSet.of(PanelConfig.Flags.DEFAULT_PANEL)).toJSON();
             } else {
                 final EnumSet<HomeConfig.PanelConfig.Flags> flags;
                 if (replacementWasDefault) {
                     // However if the replacement panel was already default, we need to preserve it's default status