Add a dialog UI for Data Use accounting

chrome/android/java/src/org/chromium/chrome/browser/datausage/DataUseTabUIManager.java

Index: chrome/android/java/src/org/chromium/chrome/browser/datausage/DataUseTabUIManager.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/datausage/DataUseTabUIManager.java b/chrome/android/java/src/org/chromium/chrome/browser/datausage/DataUseTabUIManager.java
index 711877acc15cc88d73a40bba0e73b4d03a4abdb3..c2b122eb2f4173162c37036e34c1423fe323f3fc 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/datausage/DataUseTabUIManager.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/datausage/DataUseTabUIManager.java
@@ -4,15 +4,33 @@
 
 package org.chromium.chrome.browser.datausage;
 
+import android.content.Context;
+import android.content.DialogInterface;
+import android.content.DialogInterface.OnClickListener;
+import android.preference.PreferenceManager;
+import android.support.v7.app.AlertDialog;
+import android.text.TextUtils;
+import android.view.View;
+import android.widget.CheckBox;
+import android.widget.CompoundButton;
+import android.widget.CompoundButton.OnCheckedChangeListener;
+
+import org.chromium.chrome.R;
+import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.profiles.Profile;
 import org.chromium.chrome.browser.sessions.SessionTabHelper;
+import org.chromium.chrome.browser.tab.InterceptNavigationDelegateImpl.OverrideUrlLoadingResult;
 import org.chromium.chrome.browser.tab.Tab;
+import org.chromium.content_public.browser.LoadUrlParams;
+import org.chromium.content_public.common.Referrer;
 
 /**
  * Entry point to manage all UI details for measuring data use within a Tab.
  */
 public class DataUseTabUIManager {
 
+    private static final String SHARED_PREF_DATA_USE_DIALOG_OPT_OUT = "data_use_dialog_opt_out";
+
     /**
      * Returns true if data use tracking has started within a Tab.
      *
@@ -35,6 +53,100 @@ public class DataUseTabUIManager {
                 SessionTabHelper.sessionIdForTab(tab.getWebContents()), tab.getProfile());
     }
 
+    /**
+     * Returns whether a navigation should be paused to show a dialog telling the user that data use

[newt (away), 2015/11/06 20:45:44]

This is a wonderfully clear description, by the way. Thank you :)

[megjablon, 2015/11/06 23:41:40]

:)

+     * tracking has ended within a Tab. If the navigation should be paused, shows a dialog with the
+     * option to cancel the navigation or continue.
+     *
+     * @param activity current activity.

[newt (away), 2015/11/06 20:40:36]

We usually capitalize the first letter in the @param description, e.g. @param
activity Current activity.  It's not a strict rule, but I think it's easier to
read.

[megjablon, 2015/11/06 23:41:40]

Agreed.

+     * @param tab the tab to see if tracking has ended in.
+     * @param url URL that is pending.
+     * @param pageTransitionType the type of transition. see
+     *            {@link org.chromium.content.browser.PageTransition} for valid values.
+     * @param referrerUrl URL for the referrer.
+     * @return true if the URL loading should be overriden.
+     */
+    public static OverrideUrlLoadingResult shouldOverrideUrlLoading(ChromeActivity activity,

[newt (away), 2015/11/06 20:40:36]

s/ChromeActivity/Activity/ since you're not depending on anything that's
specific to ChromeActivity

[megjablon, 2015/11/06 23:41:40]

Done.

+            final Tab tab, final String url, final int pageTransitionType,
+            final String referrerUrl) {
+        if (!getOptedOutOfDataUseDialog(activity) && hasDataUseTrackingEnded(tab)) {
+            startDataUseDialogIntent(activity, tab, url, pageTransitionType, referrerUrl);
+            return OverrideUrlLoadingResult.OVERRIDE_WITH_EXTERNAL_INTENT;

[newt (away), 2015/11/06 20:40:36]

Why "EXTERNAL_INTENT"? An external intent is something like launching the
YouTube app instead of navigating to youtube.com. In this case, we're showing a
dialog, which is not an external intent at all.

Returning OVERRIDE_WITH_EXTERNAL_INTENT will cause the
InterceptNavigationDelegateImpl to try to launch an external intent, which
doesn't make any sense here.

I would just make this method return a boolean. (and then you don't need to move
OverrideUrlLoadingResult into InterceptNavigationDelegateImpl)

[megjablon, 2015/11/06 23:41:40]

Done.

+        }
+        return OverrideUrlLoadingResult.NO_OVERRIDE;
+    }
+
+    /**
+     * Shows a dialog with the option to cancel the navigation or continue. Also allows the user to
+     * opt out of seeing this dialog again.
+     *
+     * @param activity current activity.
+     * @param tab the tab loading the url.
+     * @param url URL that is pending.
+     * @param pageTransitionType the type of transition. see
+     *            {@link org.chromium.content.browser.PageTransition} for valid values.
+     * @param referrerUrl URL for the referrer.
+     */
+    private static void startDataUseDialogIntent(final ChromeActivity activity, final Tab tab,

[newt (away), 2015/11/06 20:40:36]

This doesn't involve intents. I'd remove "Intent" from the method name.

[newt (away), 2015/11/06 20:43:13]

probably just "showDataUseDialog()"

[megjablon, 2015/11/06 23:41:40]

Done.

+            final String url, final int pageTransitionType, final String referrerUrl) {
+        View checkBoxView = View.inflate(activity, R.layout.data_use_dialog, null);
+        CheckBox checkBox = (CheckBox) checkBoxView.findViewById(R.id.data_use_checkbox);
+        checkBox.setOnCheckedChangeListener(new OnCheckedChangeListener() {
+            @Override
+            public void onCheckedChanged(CompoundButton buttonView, boolean isChecked) {
+                setOptedOutOfDataUseDialog(activity, isChecked);

[newt (away), 2015/11/06 20:40:36]

I think we should call setOptedOutOfDataUseDialog() when the user clicks "OK" on
the dialog, rather than when they check the checkbox.

[megjablon, 2015/11/06 23:41:40]

Done. What is the standard for if the user clicks outside of the dialog? Should
I setCancelable(false) so the user has to choose one of the buttons, or is that
a valid means of exiting and I should set the checkbox state?

[newt (away), 2015/11/07 00:43:34]

If they tap outside the dialog, or press back to exit the dialog, I don't think
we should save the checkbox state. I think the current code is good.

+            }
+        });
+        new AlertDialog.Builder(activity, R.style.AlertDialogTheme)
+                .setTitle(R.string.data_use_tracking_ended_title)
+                .setMessage(R.string.data_use_tracking_ended_message)
+                .setView(checkBoxView)
+                .setPositiveButton(R.string.data_use_tracking_ended_continue,
+                        new OnClickListener() {
+                            @Override
+                            public void onClick(DialogInterface dialog, int which) {
+                                LoadUrlParams loadUrlParams = new LoadUrlParams(url,
+                                        pageTransitionType);
+                                if (!TextUtils.isEmpty(referrerUrl)) {
+                                    Referrer referrer = new Referrer(referrerUrl,
+                                            Referrer.REFERRER_POLICY_ALWAYS);
+                                    loadUrlParams.setReferrer(referrer);
+                                }
+                                tab.loadUrl(loadUrlParams);
+                            }
+                        })
+                .setNegativeButton(R.string.cancel, new OnClickListener() {
+                    @Override
+                    public void onClick(DialogInterface dialog, int which) {
+                        // Do nothing.
+                    }
+                })
+                .show();
+    }
+
+    /**
+     * Returns true if the user has opted out of seeing the data use dialog.
+     *
+     * @param context an Android context.
+     * @return true if the user has opted out of seeing the data use dialog.
+     */
+    public static boolean getOptedOutOfDataUseDialog(Context context) {
+        return PreferenceManager.getDefaultSharedPreferences(context).getBoolean(
+                SHARED_PREF_DATA_USE_DIALOG_OPT_OUT, false);
+    }
+
+    /**
+     * Sets whether the user has opted out of seeing the data use dialog.
+     *
+     * @param context an Android context.
+     * @param optedOut whether the user has opted out of seeing the data use dialog.
+     */
+    private static void setOptedOutOfDataUseDialog(Context context, boolean optedOut) {
+        PreferenceManager.getDefaultSharedPreferences(context).edit()
+                .putBoolean(SHARED_PREF_DATA_USE_DIALOG_OPT_OUT, optedOut)
+                .apply();
+    }
+
     private static native boolean nativeHasDataUseTrackingStarted(int tabId, Profile profile);
     private static native boolean nativeHasDataUseTrackingEnded(int tabId, Profile profile);
 }