Issue 3136 - Only show about:feedback to release build users

mobile/android/base/BrowserApp.java

 /* -*- Mode: Java; c-basic-offset: 4; tab-width: 4; indent-tabs-mode: nil; -*-
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko;
 
 import org.mozilla.gecko.AppConstants.Versions;
 import org.mozilla.gecko.DynamicToolbar.PinReason;
 import org.mozilla.gecko.DynamicToolbar.VisibilityTransition;
@@ skipping 3519 matching lines @@
         }
 
         // Check to see how many times the app has been launched.
         final String keyName = getPackageName() + ".feedback_launch_count";
         final StrictMode.ThreadPolicy savedPolicy = StrictMode.allowThreadDiskReads();
 
         // Faster on main thread with an async apply().
         try {
             SharedPreferences settings = getPreferences(Activity.MODE_PRIVATE);
             int launchCount = settings.getInt(keyName, 0);
-            if (launchCount < FEEDBACK_LAUNCH_COUNT) {
+            // In Adblock Browser, we only want to increment the feedback page
+            // launch count for release builds that were installed from the
+            // Play store, see https://issues.adblockplus.org/ticket/2967 and
+            // https://issues.adblockplus.org/ticket/3136.
+            if (launchCount < FEEDBACK_LAUNCH_COUNT &&

[René Jeschke, 2015/10/01 14:38:11]

Is it really necessary to check `wasInstalledFromPlayStore()` on _every_ startup
(when it's a release build and until FEEDBACK_LAUNCH_COUNT is reached)?

The `wasInstalledFromPlayStore()` isn't that resource hungry, but still it
shouldn't be called without a reason IMHO.

So, I would prefer having both checks performed in this if: 
if (launchCount == FEEDBACK_LAUNCH_COUNT) {
->
if (launchCount == FEEDBACK_LAUNCH_COUNT && AppConstants.ABB_RELEASE_BUILD &&
wasInstalledFromPlayStore()) {

[Felix Dahlke, 2015/10/01 15:30:53]

I've changed this mainly to ensure that users that migrated from a beta build to
a release build, or even from a non-Play build to a Play one will still end up
seeing about:feedback, as long as they are on a Play release build. If we run
into performance issues with the Play store check, I'd prefer to optimise this
rather than to end up never showing about:feedback to people that didn't
initially install a Play release build.

[René Jeschke, 2015/10/02 08:22:34]

I wasn't really concerned about performance issues, it just felt pretty
unnecessary ... I didn't think about that issue you mentioned, though, so it
really makes sense to leave it like this.

+                AppConstants.ABB_RELEASE_BUILD && wasInstalledFromPlayStore()) {
                 // Increment the launch count and store the new value.
                 launchCount++;
                 settings.edit().putInt(keyName, launchCount).apply();
 
                 // If we've reached our magic number, show the feedback page.
-                if (launchCount == FEEDBACK_LAUNCH_COUNT && wasInstalledFromPlayStore()) {
+                if (launchCount == FEEDBACK_LAUNCH_COUNT) {
                     GeckoAppShell.sendEventToGecko(GeckoEvent.createBroadcastEvent("Feedback:Show", null));
                 }
             }
         } finally {
             StrictMode.setThreadPolicy(savedPolicy);
         }
     }
 
     private boolean wasInstalledFromPlayStore() {
         final String installer = this.getPackageManager().getInstallerPackageName(this.getPackageName());
@@ skipping 240 matching lines @@
                                          dispatcher,
                                          osLocale,
                                          appLocale,
                                          previousSession);
     }
 
     public static interface Refreshable {
         public void refresh();
     }
 }