Download later: Don't show time picker when on WIFI.

Only show download later dialog when on metered network.

Also refactor some code to use new network API.

Bug: 1078454
Change-Id: I7217592b3dc79c3b2afb2e1c4a492d6e4a965d64
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2261106
Commit-Queue: Xing Liu <xingliu@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#782244}

chrome/browser/download/android/download_dialog_bridge.cc

@@ -12,6 +12,8 @@
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/common/pref_names.h"
 #include "components/prefs/pref_service.h"
+#include "net/base/network_change_notifier.h"
+#include "services/network/public/cpp/network_connection_tracker.h"
 #include "ui/android/window_android.h"
 
 // -----------------------------------------------------------------------------
@@ -71,12 +73,19 @@ void DownloadDialogBridge::ShowDialog(gfx::NativeWindow native_window,
 
   is_dialog_showing_ = true;
 
+  // Only show download later dialog when on cellular network.
+  auto connection_type = network::mojom::ConnectionType(
+      net::NetworkChangeNotifier::GetConnectionType());
+  bool supports_later_dialog =
+      network::NetworkConnectionTracker::IsConnectionCellular(connection_type);
+
   JNIEnv* env = base::android::AttachCurrentThread();
   Java_DownloadDialogBridge_showDialog(
       env, java_obj_, native_window->GetJavaObject(),
       static_cast<long>(total_bytes), static_cast<int>(dialog_type),
       base::android::ConvertUTF8ToJavaString(env,
-                                             suggested_path.AsUTF8Unsafe()));
+                                             suggested_path.AsUTF8Unsafe()),
+      supports_later_dialog);
 }
 
 void DownloadDialogBridge::OnComplete(

chrome/browser/download/android/java/src/org/chromium/chrome/browser/download/DownloadDialogBridge.java

@@ -89,7 +89,8 @@ public class DownloadDialogBridge implements DownloadLocationDialogController,
 
     @CalledByNative
     private void showDialog(WindowAndroid windowAndroid, long totalBytes,
-            @DownloadLocationDialogType int dialogType, String suggestedPath) {
+            @DownloadLocationDialogType int dialogType, String suggestedPath,
+            boolean supportsLaterDialog) {
         Activity activity = windowAndroid.getActivity().get();
         if (activity == null) {
             onCancel();
@@ -98,11 +99,13 @@ public class DownloadDialogBridge implements DownloadLocationDialogController,
 
         ModalDialogManager modalDialogManager =
                 ((ModalDialogManagerHolder) activity).getModalDialogManager();
-        showDialog(activity, modalDialogManager, totalBytes, dialogType, suggestedPath);
+        showDialog(activity, modalDialogManager, totalBytes, dialogType, suggestedPath,
+                supportsLaterDialog);
     }
 
     void showDialog(Activity activity, ModalDialogManager modalDialogManager, long totalBytes,
-            @DownloadLocationDialogType int dialogType, String suggestedPath) {
+            @DownloadLocationDialogType int dialogType, String suggestedPath,
+            boolean supportsLaterDialog) {
         mActivity = activity;
         mModalDialogManager = modalDialogManager;
         mTotalBytes = totalBytes;
@@ -113,7 +116,7 @@ public class DownloadDialogBridge implements DownloadLocationDialogController,
         mDownloadLaterTime = INVALID_START_TIME;
 
         // Download later dialogs flow.
-        if (ChromeFeatureList.isEnabled(ChromeFeatureList.DOWNLOAD_LATER)) {
+        if (ChromeFeatureList.isEnabled(ChromeFeatureList.DOWNLOAD_LATER) && supportsLaterDialog) {
             PropertyModel model =
                     new PropertyModel.Builder(DownloadLaterDialogProperties.ALL_KEYS)
                             .with(DownloadLaterDialogProperties.DOWNLOAD_TIME_INITIAL_SELECTION,

chrome/browser/download/android/java/src/org/chromium/chrome/browser/download/DownloadDialogBridgeUnitTest.java

@@ -102,8 +102,29 @@ public class DownloadDialogBridgeUnitTest {
                 .showDialog(any(), any(), eq(TOTAL_BYTES), eq(LOCATION_DIALOG_TYPE),
                         eq(SUGGESTED_PATH));
 
-        mBridge.showDialog(
-                mActivity, mModalDialogManager, TOTAL_BYTES, LOCATION_DIALOG_TYPE, SUGGESTED_PATH);
+        mBridge.showDialog(mActivity, mModalDialogManager, TOTAL_BYTES, LOCATION_DIALOG_TYPE,
+                SUGGESTED_PATH, true);
+        verify(mLocationDialog)
+                .showDialog(any(), any(), eq(TOTAL_BYTES), eq(LOCATION_DIALOG_TYPE),
+                        eq(SUGGESTED_PATH));
+        verify(mNativeMock)
+                .onComplete(anyLong(), any(), eq(NEW_SUGGESTED_PATH), eq(false),
+                        eq(INVALID_START_TIME));
+    }
+
+    @Test
+    @Features.EnableFeatures({ChromeFeatureList.DOWNLOAD_LATER})
+    public void testShowDialog_notShowOnWifi() {
+        doAnswer(invocation -> {
+            mBridge.onDownloadLocationDialogComplete(NEW_SUGGESTED_PATH);
+            return null;
+        })
+                .when(mLocationDialog)
+                .showDialog(any(), any(), eq(TOTAL_BYTES), eq(LOCATION_DIALOG_TYPE),
+                        eq(SUGGESTED_PATH));
+
+        mBridge.showDialog(mActivity, mModalDialogManager, TOTAL_BYTES, LOCATION_DIALOG_TYPE,
+                SUGGESTED_PATH, false /*isOnMeteredNetwork*/);
         verify(mLocationDialog)
                 .showDialog(any(), any(), eq(TOTAL_BYTES), eq(LOCATION_DIALOG_TYPE),
                         eq(SUGGESTED_PATH));
@@ -129,8 +150,8 @@ public class DownloadDialogBridgeUnitTest {
                 .showDialog(any(), any(), eq(TOTAL_BYTES), eq(LOCATION_DIALOG_TYPE),
                         eq(SUGGESTED_PATH));
 
-        mBridge.showDialog(
-                mActivity, mModalDialogManager, TOTAL_BYTES, LOCATION_DIALOG_TYPE, SUGGESTED_PATH);
+        mBridge.showDialog(mActivity, mModalDialogManager, TOTAL_BYTES, LOCATION_DIALOG_TYPE,
+                SUGGESTED_PATH, true);
         verify(mNativeMock).onCanceled(anyLong(), any());
     }
 
@@ -151,8 +172,8 @@ public class DownloadDialogBridgeUnitTest {
                 .showDialog(any(), any(), eq(TOTAL_BYTES), eq(LOCATION_DIALOG_TYPE),
                         eq(SUGGESTED_PATH));
 
-        mBridge.showDialog(
-                mActivity, mModalDialogManager, TOTAL_BYTES, LOCATION_DIALOG_TYPE, SUGGESTED_PATH);
+        mBridge.showDialog(mActivity, mModalDialogManager, TOTAL_BYTES, LOCATION_DIALOG_TYPE,
+                SUGGESTED_PATH, true);
         verify(mDownloadLaterDialog).showDialog(any(), any(), any());
         verify(mLocationDialog)
                 .showDialog(any(), any(), eq(TOTAL_BYTES), eq(LOCATION_DIALOG_TYPE),
@@ -180,8 +201,8 @@ public class DownloadDialogBridgeUnitTest {
                 .showDialog(any(), any(), eq(TOTAL_BYTES), eq(LOCATION_DIALOG_TYPE),
                         eq(SUGGESTED_PATH));
 
-        mBridge.showDialog(
-                mActivity, mModalDialogManager, TOTAL_BYTES, LOCATION_DIALOG_TYPE, SUGGESTED_PATH);
+        mBridge.showDialog(mActivity, mModalDialogManager, TOTAL_BYTES, LOCATION_DIALOG_TYPE,
+                SUGGESTED_PATH, true);
         verify(mDownloadLaterDialog).showDialog(any(), any(), any());
         verify(mLocationDialog)
                 .showDialog(any(), any(), eq(TOTAL_BYTES), eq(LOCATION_DIALOG_TYPE),
@@ -216,8 +237,8 @@ public class DownloadDialogBridgeUnitTest {
                 .when(mDateTimePickerDialog)
                 .showDialog(any(), any(), any());
 
-        mBridge.showDialog(
-                mActivity, mModalDialogManager, TOTAL_BYTES, LOCATION_DIALOG_TYPE, SUGGESTED_PATH);
+        mBridge.showDialog(mActivity, mModalDialogManager, TOTAL_BYTES, LOCATION_DIALOG_TYPE,
+                SUGGESTED_PATH, true);
         verify(mDownloadLaterDialog).showDialog(any(), any(), any());
         verify(mLocationDialog)
                 .showDialog(any(), any(), eq(TOTAL_BYTES), eq(LOCATION_DIALOG_TYPE),
@@ -238,8 +259,8 @@ public class DownloadDialogBridgeUnitTest {
                 .when(mDownloadLaterDialog)
                 .showDialog(any(), any(), any());
 
-        mBridge.showDialog(
-                mActivity, mModalDialogManager, TOTAL_BYTES, LOCATION_DIALOG_TYPE, SUGGESTED_PATH);
+        mBridge.showDialog(mActivity, mModalDialogManager, TOTAL_BYTES, LOCATION_DIALOG_TYPE,
+                SUGGESTED_PATH, true);
         verify(mDownloadLaterDialog).showDialog(any(), any(), any());
         verify(mLocationDialog, times(0))
                 .showDialog(any(), any(), anyLong(), anyInt(), anyString());

components/download/public/common/auto_resumption_handler.cc

@@ -16,6 +16,7 @@
 #include "base/time/time.h"
 #include "components/download/public/common/download_item.h"
 #include "components/download/public/task/task_scheduler.h"
+#include "services/network/public/cpp/network_connection_tracker.h"
 #include "url/gurl.h"
 
 namespace download {
@@ -48,23 +49,6 @@ const int64_t kWindowStartTimeSeconds = 0;
 // The window end time before which the system should fire the task.
 const int64_t kWindowEndTimeSeconds = 24 * 60 * 60;
 
-bool IsMetered(network::mojom::ConnectionType type) {
-  switch (type) {
-    case network::mojom::ConnectionType::CONNECTION_2G:
-    case network::mojom::ConnectionType::CONNECTION_3G:
-    case network::mojom::ConnectionType::CONNECTION_4G:
-      return true;
-    case network::mojom::ConnectionType::CONNECTION_ETHERNET:
-    case network::mojom::ConnectionType::CONNECTION_WIFI:
-    case network::mojom::ConnectionType::CONNECTION_UNKNOWN:
-    case network::mojom::ConnectionType::CONNECTION_NONE:
-    case network::mojom::ConnectionType::CONNECTION_BLUETOOTH:
-      return false;
-  }
-  NOTREACHED();
-  return false;
-}
-
 bool IsConnected(network::mojom::ConnectionType type) {
   switch (type) {
     case network::mojom::ConnectionType::CONNECTION_UNKNOWN:
@@ -134,7 +118,8 @@ void AutoResumptionHandler::SetResumableDownloads(
 }
 
 bool AutoResumptionHandler::IsActiveNetworkMetered() const {
-  return IsMetered(network_listener_->GetConnectionType());
+  return network::NetworkConnectionTracker::IsConnectionCellular(
+      network_listener_->GetConnectionType());
 }
 
 void AutoResumptionHandler::OnNetworkChanged(