[Downloads location] Implement pre-download error-handling.
Handle errors that occur before downloading, including the name being
repeated/too long, the location being missing/full, etc. Some of the
changes include:
- Creating DownloadsLocationDialogType to indicate which type of error-
handling dialog to display.
- Re-routing DownloadTargetDeterminer to loop back and double check in
the case that it was a user-confirmed "new" filename/location.
Screenshots:
Rename workflow:
https://drive.google.com/file/d/1S4Pw8zXy-uZvotx3L8eNZgl2QMzaETfX/view
Not enough space option:
https://drive.google.com/file/d/1JiCVo5K4kEcGVtZQhdJ4sSrqEQNagdgB/view
Bug: 792775, 825929
Change-Id: Ic981343eede2b10a84d383ad429f5ddcbccfd38c
Reviewed-on: https://chromium-review.googlesource.com/982530
Commit-Queue: Joy Ming <jming@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Reviewed-by: Xing Liu <xingliu@chromium.org>
Reviewed-by: Shakti Sahu <shaktisahu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549674}diff --git a/chrome/BUILD.gn b/chrome/BUILD.gn
index 51bba141..4304cc82 100644
--- a/chrome/BUILD.gn
+++ b/chrome/BUILD.gn
@@ -1785,6 +1785,7 @@
java_cpp_enum("download_enum_javagen") {
sources = [
+ "browser/download/download_location_dialog_type.h",
"browser/download/download_prompt_status.h",
]
}
diff --git a/chrome/android/java/res/layout/download_location_dialog.xml b/chrome/android/java/res/layout/download_location_dialog.xml
index 1daae20..cd37ca6 100644
--- a/chrome/android/java/res/layout/download_location_dialog.xml
+++ b/chrome/android/java/res/layout/download_location_dialog.xml
@@ -3,60 +3,76 @@
Use of this source code is governed by a BSD-style license that can be
found in the LICENSE file. -->
-<LinearLayout
+<ScrollView
xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:tools="http://schemas.android.com/apk/res-auto"
- android:orientation="vertical"
android:layout_width="match_parent"
- android:layout_height="match_parent"
- style="@style/AlertDialogContent">
+ android:layout_height="wrap_content" >
<LinearLayout
+ android:orientation="vertical"
android:layout_width="match_parent"
android:layout_height="wrap_content"
- android:orientation="horizontal">
+ style="@style/AlertDialogContent">
- <org.chromium.chrome.browser.widget.TintedImageView
- android:layout_width="wrap_content"
- android:layout_height="wrap_content"
- tools:srcCompat="@drawable/ic_drive_file_24dp"
- android:tint="@color/black_alpha_87"
- style="@style/ListItemStartIcon" />
-
- <org.chromium.chrome.browser.widget.AlertDialogEditText
- android:id="@+id/file_name"
+ <TextView
android:layout_width="match_parent"
android:layout_height="wrap_content"
- android:textAppearance="@style/BlackTitle1" />
+ android:id="@+id/subtitle"
+ android:textAppearance="@style/BlackBody"
+ android:visibility="gone"
+ android:layout_marginBottom="16dp" />
+
+ <LinearLayout
+ android:layout_width="match_parent"
+ android:layout_height="wrap_content"
+ android:orientation="horizontal">
+
+ <org.chromium.chrome.browser.widget.TintedImageView
+ android:layout_width="wrap_content"
+ android:layout_height="wrap_content"
+ tools:srcCompat="@drawable/ic_drive_file_24dp"
+ android:tint="@color/black_alpha_87"
+ style="@style/ListItemStartIcon" />
+
+ <org.chromium.chrome.browser.widget.AlertDialogEditText
+ android:id="@+id/file_name"
+ android:layout_width="match_parent"
+ android:layout_height="wrap_content"
+ android:textAppearance="@style/BlackTitle1"
+ android:singleLine="true" />
+
+ </LinearLayout>
+
+ <LinearLayout
+ android:layout_width="match_parent"
+ android:layout_height="wrap_content"
+ android:orientation="horizontal">
+
+ <org.chromium.chrome.browser.widget.TintedImageView
+ android:layout_width="wrap_content"
+ android:layout_height="wrap_content"
+ android:src="@drawable/ic_folder_blue_24dp"
+ android:tint="@color/black_alpha_87"
+ style="@style/ListItemStartIcon" />
+
+ <Spinner
+ android:id="@+id/file_location"
+ android:layout_width="match_parent"
+ android:layout_height="wrap_content"
+ style="@android:style/Widget.EditText"/>
+
+ </LinearLayout>
+
+ <CheckBox
+ android:id="@+id/show_again_checkbox"
+ android:layout_width="match_parent"
+ android:layout_height="wrap_content"
+ android:text="@string/download_location_dialog_checkbox"
+ android:layout_marginTop="16dp" />
+
</LinearLayout>
- <LinearLayout
- android:layout_width="match_parent"
- android:layout_height="wrap_content"
- android:orientation="horizontal">
+</ScrollView>
- <org.chromium.chrome.browser.widget.TintedImageView
- android:layout_width="wrap_content"
- android:layout_height="wrap_content"
- android:src="@drawable/ic_folder_blue_24dp"
- android:tint="@color/black_alpha_87"
- style="@style/ListItemStartIcon" />
-
- <Spinner
- android:id="@+id/file_location"
- android:layout_width="match_parent"
- android:layout_height="wrap_content"
- style="@android:style/Widget.EditText"/>
-
- </LinearLayout>
-
- <CheckBox
- android:id="@+id/show_again_checkbox"
- android:layout_width="match_parent"
- android:layout_height="wrap_content"
- android:text="@string/download_location_dialog_checkbox"
- android:layout_marginTop="16dp" />
-
-
-</LinearLayout>
\ No newline at end of file
diff --git a/chrome/android/java/res/values-v17/styles.xml b/chrome/android/java/res/values-v17/styles.xml
index dbbec87..9e8ac85 100644
--- a/chrome/android/java/res/values-v17/styles.xml
+++ b/chrome/android/java/res/values-v17/styles.xml
@@ -122,6 +122,7 @@
<item name="android:windowMinWidthMinor">100%</item>
<item name="buttonBarStyle">@style/ModalDialogButtonBarStyle</item>
<item name="buttonBarButtonStyle">@style/ModalDialogButtonStyle</item>
+ <item name="android:windowSoftInputMode">adjustResize|stateHidden</item>
</style>
<style name="ModalDialogButtonBarStyle" parent="Widget.AppCompat.ButtonBar.AlertDialog">
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadLocationDialog.java b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadLocationDialog.java
index af049e7e..315dcff 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadLocationDialog.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadLocationDialog.java
@@ -4,10 +4,14 @@
package org.chromium.chrome.browser.download;
+import static org.chromium.chrome.browser.preferences.download.DownloadDirectoryAdapter.NO_SELECTED_ITEM_ID;
+
import android.content.Context;
import android.view.LayoutInflater;
+import android.view.View;
import android.widget.CheckBox;
import android.widget.Spinner;
+import android.widget.TextView;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.modaldialog.ModalDialogView;
@@ -34,23 +38,54 @@
*
* @param controller Controller that listens to the events from the dialog.
* @param context Context from which the dialog emerged.
+ * @param dialogType Type of dialog that should be displayed, dictates title/subtitle.
* @param suggestedPath The path that was automatically generated, used as a starting point.
* @return A {@link DownloadLocationDialog} with the given properties.
*/
- public static DownloadLocationDialog create(
- Controller controller, Context context, File suggestedPath) {
+ public static DownloadLocationDialog create(Controller controller, Context context,
+ @DownloadLocationDialogType int dialogType, File suggestedPath) {
Params params = new Params();
- params.title = context.getString(R.string.download_location_dialog_title);
params.positiveButtonTextId = R.string.duplicate_download_infobar_download_button;
params.negativeButtonTextId = R.string.cancel;
params.customView =
LayoutInflater.from(context).inflate(R.layout.download_location_dialog, null);
- return new DownloadLocationDialog(controller, context, suggestedPath, params);
+ params.title = context.getString(R.string.download_location_dialog_title);
+ TextView subtitleText = params.customView.findViewById(R.id.subtitle);
+ subtitleText.setVisibility(
+ dialogType == DownloadLocationDialogType.DEFAULT ? View.GONE : View.VISIBLE);
+
+ switch (dialogType) {
+ case DownloadLocationDialogType.LOCATION_FULL:
+ params.title = context.getString(R.string.download_location_not_enough_space);
+ subtitleText.setText(R.string.download_location_download_to_default_folder);
+ break;
+
+ case DownloadLocationDialogType.LOCATION_NOT_FOUND:
+ params.title = context.getString(R.string.download_location_no_sd_card);
+ subtitleText.setText(R.string.download_location_download_to_default_folder);
+ break;
+
+ case DownloadLocationDialogType.NAME_CONFLICT:
+ params.title = context.getString(R.string.download_location_download_again);
+ subtitleText.setText(R.string.download_location_name_exists);
+ break;
+
+ case DownloadLocationDialogType.NAME_TOO_LONG:
+ params.title = context.getString(R.string.download_location_rename_file);
+ subtitleText.setText(R.string.download_location_name_too_long);
+ break;
+
+ case DownloadLocationDialogType.DEFAULT:
+ default:
+ break;
+ }
+
+ return new DownloadLocationDialog(controller, context, dialogType, suggestedPath, params);
}
- private DownloadLocationDialog(
- Controller controller, Context context, File suggestedPath, Params params) {
+ private DownloadLocationDialog(Controller controller, Context context,
+ @DownloadLocationDialogType int dialogType, File suggestedPath, Params params) {
super(controller, params);
mDirectoryAdapter = new DownloadDirectoryAdapter(context);
@@ -61,7 +96,13 @@
mFileLocation = (Spinner) params.customView.findViewById(R.id.file_location);
mFileLocation.setAdapter(mDirectoryAdapter);
- mFileLocation.setSelection(mDirectoryAdapter.getSelectedItemId());
+ int selectedItemId = mDirectoryAdapter.getSelectedItemId();
+ if (selectedItemId == NO_SELECTED_ITEM_ID
+ || dialogType == DownloadLocationDialogType.LOCATION_FULL
+ || dialogType == DownloadLocationDialogType.LOCATION_NOT_FOUND) {
+ selectedItemId = mDirectoryAdapter.getFirstSelectableItemId();
+ }
+ mFileLocation.setSelection(selectedItemId);
// Automatically check "don't show again" the first time the user is seeing the dialog.
mDontShowAgain = (CheckBox) params.customView.findViewById(R.id.show_again_checkbox);
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadLocationDialogBridge.java b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadLocationDialogBridge.java
index 4707a08..4b1948a 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadLocationDialogBridge.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadLocationDialogBridge.java
@@ -9,7 +9,7 @@
import org.chromium.chrome.browser.modaldialog.ModalDialogManager;
import org.chromium.chrome.browser.modaldialog.ModalDialogView;
import org.chromium.chrome.browser.preferences.PrefServiceBridge;
-import org.chromium.content_public.browser.WebContents;
+import org.chromium.ui.base.WindowAndroid;
import java.io.File;
@@ -37,10 +37,9 @@
}
@CalledByNative
- public void showDialog(WebContents webContents, String suggestedPath) {
- // TODO(jming): Remove WebContents requirement.
- ChromeActivity activity =
- (ChromeActivity) webContents.getTopLevelNativeWindow().getActivity().get();
+ public void showDialog(WindowAndroid windowAndroid, @DownloadLocationDialogType int dialogType,
+ String suggestedPath) {
+ ChromeActivity activity = (ChromeActivity) windowAndroid.getActivity().get();
// If the activity has gone away, just clean up the native pointer.
if (activity == null) {
onCancel();
@@ -50,7 +49,8 @@
mModalDialogManager = activity.getModalDialogManager();
if (mLocationDialog != null) return;
- mLocationDialog = DownloadLocationDialog.create(this, activity, new File(suggestedPath));
+ mLocationDialog =
+ DownloadLocationDialog.create(this, activity, dialogType, new File(suggestedPath));
mModalDialogManager.showDialog(mLocationDialog, ModalDialogManager.APP_MODAL);
}
@@ -98,10 +98,15 @@
// Update native with new path.
if (mNativeDownloadLocationDialogBridge != 0) {
+ PrefServiceBridge.getInstance().setDownloadAndSaveFileDefaultDirectory(
+ fileLocation.getAbsolutePath());
+
File filePath = new File(fileLocation, fileName);
nativeOnComplete(mNativeDownloadLocationDialogBridge, filePath.getAbsolutePath());
}
+ // TODO(jming): Right now this doesn't stay checked if a second error is displayed.
+ // Figure out if this needs to be fixed (depending on if we want it pre-checked anyways).
// Update preference to show prompt based on whether checkbox is checked.
if (dontShowAgain) {
PrefServiceBridge.getInstance().setPromptForDownloadAndroid(
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java
index dbf0a76..f3f6a1d 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java
@@ -88,6 +88,10 @@
R.string.file_size_downloaded_gb
};
+ private static final int[] BYTES_AVAILABLE_STRINGS = {
+ R.string.download_manager_ui_space_free_kb, R.string.download_manager_ui_space_free_mb,
+ R.string.download_manager_ui_space_free_gb};
+
private static final String TAG = "download";
private static final String DEFAULT_MIME_TYPE = "*/*";
@@ -875,6 +879,18 @@
}
/**
+ * Format the number of available bytes into KB, MB, or GB and return the corresponding string
+ * resource. Uses deafult format "20 KB available."
+ *
+ * @param context Context to use.
+ * @param bytes Number of bytes needed to display.
+ * @return The formatted string to be displayed.
+ */
+ public static String getStringForAvailableBytes(Context context, long bytes) {
+ return getStringForBytes(context, BYTES_AVAILABLE_STRINGS, bytes);
+ }
+
+ /**
* Format the number of bytes into KB, or MB, or GB and return the corresponding string
* resource.
* @param context Context to use.
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/download/ui/SpaceDisplay.java b/chrome/android/java/src/org/chromium/chrome/browser/download/ui/SpaceDisplay.java
index 87973c3..bde64142 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/download/ui/SpaceDisplay.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/download/ui/SpaceDisplay.java
@@ -43,11 +43,6 @@
R.string.download_manager_ui_space_used_mb,
R.string.download_manager_ui_space_used_gb
};
- private static final int[] FREE_STRINGS = {
- R.string.download_manager_ui_space_free_kb,
- R.string.download_manager_ui_space_free_mb,
- R.string.download_manager_ui_space_free_gb
- };
private static final int[] OTHER_STRINGS = {
R.string.download_manager_ui_space_other_kb,
R.string.download_manager_ui_space_other_mb,
@@ -190,7 +185,7 @@
mSpaceUsedByDownloadsTextView.setText(
DownloadUtils.getStringForBytes(context, USED_STRINGS, bytesUsedByDownloads));
- String spaceFree = DownloadUtils.getStringForBytes(context, FREE_STRINGS, mFreeBytes);
+ String spaceFree = DownloadUtils.getStringForAvailableBytes(context, mFreeBytes);
String spaceUsedByOtherApps =
DownloadUtils.getStringForBytes(context, OTHER_STRINGS, bytesUsedByOtherApps);
mSpaceFreeAndOtherAppsTextView.setText(
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/preferences/download/DownloadDirectoryAdapter.java b/chrome/android/java/src/org/chromium/chrome/browser/preferences/download/DownloadDirectoryAdapter.java
index f0f444d..d35e686 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/preferences/download/DownloadDirectoryAdapter.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/preferences/download/DownloadDirectoryAdapter.java
@@ -9,7 +9,7 @@
import android.os.Environment;
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
-import android.text.format.Formatter;
+import android.support.v4.widget.TextViewCompat;
import android.view.LayoutInflater;
import android.view.View;
import android.view.ViewGroup;
@@ -17,6 +17,7 @@
import android.widget.TextView;
import org.chromium.chrome.R;
+import org.chromium.chrome.browser.download.DownloadUtils;
import org.chromium.chrome.browser.preferences.PrefServiceBridge;
import org.chromium.chrome.browser.widget.TintedImageView;
@@ -29,15 +30,17 @@
* download location.
*/
public class DownloadDirectoryAdapter extends ArrayAdapter<Object> {
+ public static int NO_SELECTED_ITEM_ID = -1;
+
/**
* Denotes a given option for directory selection; includes name, location, and space.
*/
public static class DirectoryOption {
- private String mName;
- private File mLocation;
- private String mAvailableSpace;
+ private final String mName;
+ private final File mLocation;
+ private final long mAvailableSpace;
- DirectoryOption(String directoryName, File directoryLocation, String availableSpace) {
+ DirectoryOption(String directoryName, File directoryLocation, long availableSpace) {
mName = directoryName;
mLocation = directoryLocation;
mAvailableSpace = availableSpace;
@@ -60,7 +63,7 @@
/**
* @return Amount of available space in this directory option.
*/
- String getAvailableSpace() {
+ long getAvailableSpace() {
return mAvailableSpace;
}
}
@@ -136,7 +139,16 @@
titleText.setText(directoryOption.getName());
TextView summaryText = (TextView) view.findViewById(R.id.description);
- summaryText.setText(directoryOption.getAvailableSpace());
+ if (isEnabled(position)) {
+ TextViewCompat.setTextAppearance(titleText, R.style.BlackTitle1);
+ TextViewCompat.setTextAppearance(summaryText, R.style.BlackBody);
+ summaryText.setText(DownloadUtils.getStringForAvailableBytes(
+ mContext, directoryOption.getAvailableSpace()));
+ } else {
+ TextViewCompat.setTextAppearance(titleText, R.style.BlackDisabledText1);
+ TextViewCompat.setTextAppearance(summaryText, R.style.BlackDisabledText3);
+ summaryText.setText(mContext.getText(R.string.download_location_not_enough_space));
+ }
TintedImageView imageView = (TintedImageView) view.findViewById(R.id.icon_view);
imageView.setVisibility(View.GONE);
@@ -144,16 +156,45 @@
return view;
}
+ @Override
+ public boolean isEnabled(int position) {
+ DirectoryOption directoryOption = (DirectoryOption) getItem(position);
+ return directoryOption != null && directoryOption.getAvailableSpace() != 0;
+ }
+
/**
- * @return Id of the directory option that matches the default download location.
+ * @return ID of the directory option that matches the default download location or
+ * NO_SELECTED_ITEM_ID if no item matches the default path.
*/
public int getSelectedItemId() {
- String location = PrefServiceBridge.getInstance().getDownloadDefaultDirectory();
+ String defaultLocation = PrefServiceBridge.getInstance().getDownloadDefaultDirectory();
for (int i = 0; i < getCount(); i++) {
DirectoryOption option = (DirectoryOption) getItem(i);
if (option == null) continue;
- if (location.equals(option.getLocation().getAbsolutePath())) return i;
+ if (defaultLocation.equals(option.getLocation().getAbsolutePath())) return i;
}
+ return NO_SELECTED_ITEM_ID;
+ }
+
+ /**
+ * In the case that there is no selected item ID/the selected item ID is invalid (ie. there is
+ * not enough space), select either the default or the next valid item ID. Set the default to be
+ * this item and return the ID.
+ *
+ * @return ID of the first valid, selectable item and the new default location.
+ */
+ public int getFirstSelectableItemId() {
+ for (int i = 0; i < getCount(); i++) {
+ DirectoryOption option = (DirectoryOption) getItem(i);
+ if (option == null) continue;
+ if (option.getAvailableSpace() > 0) {
+ PrefServiceBridge.getInstance().setDownloadAndSaveFileDefaultDirectory(
+ option.getLocation().getAbsolutePath());
+ return i;
+ }
+ }
+
+ // TODO(jming): Update behavior with UX suggestions.
throw new AssertionError("No selected item ID.");
}
@@ -167,13 +208,13 @@
File directoryLocation =
Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS);
mCanonicalOptions.add(new DirectoryOption(mContext.getString(R.string.menu_downloads),
- directoryLocation, getAvailableBytesString(directoryLocation)));
+ directoryLocation, directoryLocation.getUsableSpace()));
}
private void setAdditionalDirectoryOptions() {
mAdditionalOptions.clear();
- // TODO(jming): Is there any way to do this for API < 19????
+ // TODO(jming): Is there any way to do this for API < 19?
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.KITKAT) return;
File[] externalDirs = mContext.getExternalFilesDirs(Environment.DIRECTORY_DOWNLOADS);
@@ -197,14 +238,9 @@
org.chromium.chrome.R.string.downloads_location_sd_card_number,
numOtherAdditionalDirectories + 1)
: mContext.getString(org.chromium.chrome.R.string.downloads_location_sd_card);
- String availableBytes = getAvailableBytesString(dir);
- mAdditionalOptions.add(new DirectoryOption(directoryName, dir, availableBytes));
+ mAdditionalOptions.add(new DirectoryOption(directoryName, dir, dir.getUsableSpace()));
numOtherAdditionalDirectories++;
}
}
-
- private String getAvailableBytesString(File file) {
- return Formatter.formatFileSize(mContext, file.getUsableSpace());
- }
}
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/preferences/download/DownloadPreferences.java b/chrome/android/java/src/org/chromium/chrome/browser/preferences/download/DownloadPreferences.java
index 32e8b3c..570db6e4 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/preferences/download/DownloadPreferences.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/preferences/download/DownloadPreferences.java
@@ -4,6 +4,8 @@
package org.chromium.chrome.browser.preferences.download;
+import static org.chromium.chrome.browser.preferences.download.DownloadDirectoryAdapter.NO_SELECTED_ITEM_ID;
+
import android.os.Bundle;
import android.preference.Preference;
import android.preference.PreferenceFragment;
@@ -25,6 +27,7 @@
private static final String PREF_LOCATION_PROMPT_ENABLED = "location_prompt_enabled";
private SpinnerPreference mLocationChangePref;
+ private DownloadDirectoryAdapter mDirectoryAdapter;
private ChromeSwitchPreference mLocationPromptEnabledPref;
@Override
@@ -39,22 +42,26 @@
mLocationPromptEnabledPref.setOnPreferenceChangeListener(this);
mLocationChangePref = (SpinnerPreference) findPreference(PREF_LOCATION_CHANGE);
- DownloadDirectoryAdapter directoryAdapter = new DownloadDirectoryAdapter(getActivity());
- mLocationChangePref.setAdapter(directoryAdapter, directoryAdapter.getSelectedItemId());
+ mLocationChangePref.setOnPreferenceChangeListener(this);
+ mDirectoryAdapter = new DownloadDirectoryAdapter(getActivity());
+ int selectedItemId = mDirectoryAdapter.getSelectedItemId();
+ if (selectedItemId == NO_SELECTED_ITEM_ID) {
+ selectedItemId = mDirectoryAdapter.getFirstSelectableItemId();
+ }
+ mLocationChangePref.setAdapter(mDirectoryAdapter, selectedItemId);
- updateSummaries();
+ updateData();
}
@Override
public void onResume() {
super.onResume();
- updateSummaries();
+ updateData();
}
- private void updateSummaries() {
+ private void updateData() {
if (mLocationChangePref != null) {
- mLocationChangePref.setSummary(
- PrefServiceBridge.getInstance().getDownloadDefaultDirectory());
+ mDirectoryAdapter.notifyDataSetChanged();
}
if (mLocationPromptEnabledPref != null) {
@@ -83,9 +90,11 @@
DownloadPromptStatus.DONT_SHOW);
}
} else if (PREF_LOCATION_CHANGE.equals(preference.getKey())) {
+ DownloadDirectoryAdapter.DirectoryOption option =
+ (DownloadDirectoryAdapter.DirectoryOption) newValue;
PrefServiceBridge.getInstance().setDownloadAndSaveFileDefaultDirectory(
- (String) newValue);
- updateSummaries();
+ option.getLocation().getAbsolutePath());
+ updateData();
}
return true;
}
diff --git a/chrome/android/java/strings/android_chrome_strings.grd b/chrome/android/java/strings/android_chrome_strings.grd
index 7b819a03..584b27f 100644
--- a/chrome/android/java/strings/android_chrome_strings.grd
+++ b/chrome/android/java/strings/android_chrome_strings.grd
@@ -1204,6 +1204,27 @@
<message name="IDS_DOWNLOAD_LOCATION_DIALOG_CHECKBOX" desc="Label for the checkbox that allows the user to indicate if they do not want the download location selection dialog to appear every time they initiate a download.">
Don‘t show again
</message>
+ <message name="IDS_DOWNLOAD_LOCATION_DOWNLOAD_AGAIN" desc="Title for download location dialog in the case that there exists a file with the same name as the one they are trying to download, confirming the user wants to download a file with the same name.">
+ Download file again?
+ </message>
+ <message name="IDS_DOWNLOAD_LOCATION_NAME_EXISTS" desc="Subtitle for download location dialog in the case that the file needs to be renamed because another file has the same name.">
+ File name already exists
+ </message>
+ <message name="IDS_DOWNLOAD_LOCATION_RENAME_FILE" desc="Title for download location dialog in the case that the file needs to be renamed.">
+ Rename file
+ </message>
+ <message name="IDS_DOWNLOAD_LOCATION_NAME_TOO_LONG" desc="Subtitle for download location dialog in the case that the file needs to be renamed because the name is too long.">
+ File name too long
+ </message>
+ <message name="IDS_DOWNLOAD_LOCATION_NOT_ENOUGH_SPACE" desc="Text on download location dialog in the case that there is not enough space in the chosen location.">
+ Not enough space
+ </message>
+ <message name="IDS_DOWNLOAD_LOCATION_DOWNLOAD_TO_DEFAULT_FOLDER" desc="Subtitle for download location dialog in the case that the download location the user had originally selected is full or not found. We automatically revert back to the default download location and ask the user to confirm that this is ok.">
+ Download to default folder?
+ </message>
+ <message name="IDS_DOWNLOAD_LOCATION_NO_SD_CARD" desc="Title for download location dialog in the case that the SD card is not found.">
+ No SD card found
+ </message>
<!-- About Chrome preferences -->
<message name="IDS_PREFS_ABOUT_CHROME" desc="Title for the About Chrome page. [CHAR-LIMIT=32]">
diff --git a/chrome/android/java/strings/android_chrome_strings_grd/IDS_DOWNLOAD_LOCATION_DOWNLOAD_AGAIN.png.sha1 b/chrome/android/java/strings/android_chrome_strings_grd/IDS_DOWNLOAD_LOCATION_DOWNLOAD_AGAIN.png.sha1
new file mode 100644
index 0000000..4905dd2
--- /dev/null
+++ b/chrome/android/java/strings/android_chrome_strings_grd/IDS_DOWNLOAD_LOCATION_DOWNLOAD_AGAIN.png.sha1
@@ -0,0 +1 @@
+996451fc5e924ea7e0e0a750e815798ef1610001
\ No newline at end of file
diff --git a/chrome/android/java/strings/android_chrome_strings_grd/IDS_DOWNLOAD_LOCATION_DOWNLOAD_TO_DEFAULT_FOLDER.png.sha1 b/chrome/android/java/strings/android_chrome_strings_grd/IDS_DOWNLOAD_LOCATION_DOWNLOAD_TO_DEFAULT_FOLDER.png.sha1
new file mode 100644
index 0000000..baa4153
--- /dev/null
+++ b/chrome/android/java/strings/android_chrome_strings_grd/IDS_DOWNLOAD_LOCATION_DOWNLOAD_TO_DEFAULT_FOLDER.png.sha1
@@ -0,0 +1 @@
+73e660d7f0b82f0404c3f91911ae619275a3f00f
\ No newline at end of file
diff --git a/chrome/android/java/strings/android_chrome_strings_grd/IDS_DOWNLOAD_LOCATION_NAME_EXISTS.png.sha1 b/chrome/android/java/strings/android_chrome_strings_grd/IDS_DOWNLOAD_LOCATION_NAME_EXISTS.png.sha1
new file mode 100644
index 0000000..4905dd2
--- /dev/null
+++ b/chrome/android/java/strings/android_chrome_strings_grd/IDS_DOWNLOAD_LOCATION_NAME_EXISTS.png.sha1
@@ -0,0 +1 @@
+996451fc5e924ea7e0e0a750e815798ef1610001
\ No newline at end of file
diff --git a/chrome/android/java/strings/android_chrome_strings_grd/IDS_DOWNLOAD_LOCATION_NAME_TOO_LONG.png.sha1 b/chrome/android/java/strings/android_chrome_strings_grd/IDS_DOWNLOAD_LOCATION_NAME_TOO_LONG.png.sha1
new file mode 100644
index 0000000..822922858
--- /dev/null
+++ b/chrome/android/java/strings/android_chrome_strings_grd/IDS_DOWNLOAD_LOCATION_NAME_TOO_LONG.png.sha1
@@ -0,0 +1 @@
+228aff612bcbaf525281368e2fbfb695fcdda98c
\ No newline at end of file
diff --git a/chrome/android/java/strings/android_chrome_strings_grd/IDS_DOWNLOAD_LOCATION_NOT_ENOUGH_SPACE.png.sha1 b/chrome/android/java/strings/android_chrome_strings_grd/IDS_DOWNLOAD_LOCATION_NOT_ENOUGH_SPACE.png.sha1
new file mode 100644
index 0000000..f3144d4
--- /dev/null
+++ b/chrome/android/java/strings/android_chrome_strings_grd/IDS_DOWNLOAD_LOCATION_NOT_ENOUGH_SPACE.png.sha1
@@ -0,0 +1 @@
+84cce574e1120a5c2a6360fe440447dc06b77a5f
\ No newline at end of file
diff --git a/chrome/android/java/strings/android_chrome_strings_grd/IDS_DOWNLOAD_LOCATION_NO_SD_CARD.png.sha1 b/chrome/android/java/strings/android_chrome_strings_grd/IDS_DOWNLOAD_LOCATION_NO_SD_CARD.png.sha1
new file mode 100644
index 0000000..baa4153
--- /dev/null
+++ b/chrome/android/java/strings/android_chrome_strings_grd/IDS_DOWNLOAD_LOCATION_NO_SD_CARD.png.sha1
@@ -0,0 +1 @@
+73e660d7f0b82f0404c3f91911ae619275a3f00f
\ No newline at end of file
diff --git a/chrome/android/java/strings/android_chrome_strings_grd/IDS_DOWNLOAD_LOCATION_RENAME_FILE.png.sha1 b/chrome/android/java/strings/android_chrome_strings_grd/IDS_DOWNLOAD_LOCATION_RENAME_FILE.png.sha1
new file mode 100644
index 0000000..822922858
--- /dev/null
+++ b/chrome/android/java/strings/android_chrome_strings_grd/IDS_DOWNLOAD_LOCATION_RENAME_FILE.png.sha1
@@ -0,0 +1 @@
+228aff612bcbaf525281368e2fbfb695fcdda98c
\ No newline at end of file
diff --git a/chrome/browser/BUILD.gn b/chrome/browser/BUILD.gn
index ab747b4..53d88d4 100644
--- a/chrome/browser/BUILD.gn
+++ b/chrome/browser/BUILD.gn
@@ -392,6 +392,7 @@
"download/download_history.h",
"download/download_item_model.cc",
"download/download_item_model.h",
+ "download/download_location_dialog_type.h",
"download/download_path_reservation_tracker.cc",
"download/download_path_reservation_tracker.h",
"download/download_permission_request.cc",
@@ -1988,8 +1989,9 @@
"android/download/download_controller.h",
"android/download/download_controller_base.cc",
"android/download/download_controller_base.h",
- "android/download/download_location_dialog_bridge.cc",
"android/download/download_location_dialog_bridge.h",
+ "android/download/download_location_dialog_bridge_impl.cc",
+ "android/download/download_location_dialog_bridge_impl.h",
"android/download/download_manager_service.cc",
"android/download/download_manager_service.h",
"android/download/duplicate_download_infobar_delegate.cc",
diff --git a/chrome/browser/android/download/download_location_dialog_bridge.h b/chrome/browser/android/download/download_location_dialog_bridge.h
index eeba67c..0349005 100644
--- a/chrome/browser/android/download/download_location_dialog_bridge.h
+++ b/chrome/browser/android/download/download_location_dialog_bridge.h
@@ -7,38 +7,28 @@
#include "base/android/jni_android.h"
#include "base/android/scoped_java_ref.h"
-#include "base/callback.h"
-#include "chrome/browser/download/download_confirmation_result.h"
+#include "chrome/browser/download/download_location_dialog_type.h"
#include "chrome/browser/download/download_target_determiner_delegate.h"
#include "ui/gfx/native_widget_types.h"
-namespace content {
-class WebContents;
-} // namespace content
-
class DownloadLocationDialogBridge {
public:
- DownloadLocationDialogBridge();
- ~DownloadLocationDialogBridge();
+ virtual ~DownloadLocationDialogBridge() = default;
- void ShowDialog(
- content::WebContents* web_contents,
+ virtual void ShowDialog(
+ gfx::NativeWindow native_window,
+ DownloadLocationDialogType dialog_type,
const base::FilePath& suggested_path,
- const DownloadTargetDeterminerDelegate::ConfirmationCallback& callback);
+ const DownloadTargetDeterminerDelegate::ConfirmationCallback&
+ callback) = 0;
- void OnComplete(JNIEnv* env,
- const base::android::JavaParamRef<jobject>& obj,
- const base::android::JavaParamRef<jstring>& returned_path);
+ virtual void OnComplete(
+ JNIEnv* env,
+ const base::android::JavaParamRef<jobject>& obj,
+ const base::android::JavaParamRef<jstring>& returned_path) = 0;
- void OnCanceled(JNIEnv* env, const base::android::JavaParamRef<jobject>& obj);
-
- private:
- jboolean is_dialog_showing_;
- base::android::ScopedJavaGlobalRef<jobject> java_obj_;
- DownloadTargetDeterminerDelegate::ConfirmationCallback
- dialog_complete_callback_;
-
- DISALLOW_COPY_AND_ASSIGN(DownloadLocationDialogBridge);
+ virtual void OnCanceled(JNIEnv* env,
+ const base::android::JavaParamRef<jobject>& obj) = 0;
};
#endif // CHROME_BROWSER_ANDROID_DOWNLOAD_DOWNLOAD_LOCATION_DIALOG_BRIDGE_H_
diff --git a/chrome/browser/android/download/download_location_dialog_bridge.cc b/chrome/browser/android/download/download_location_dialog_bridge_impl.cc
similarity index 74%
rename from chrome/browser/android/download/download_location_dialog_bridge.cc
rename to chrome/browser/android/download/download_location_dialog_bridge_impl.cc
index bb2cb01..af1dc0d 100644
--- a/chrome/browser/android/download/download_location_dialog_bridge.cc
+++ b/chrome/browser/android/download/download_location_dialog_bridge_impl.cc
@@ -2,16 +2,16 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#include "chrome/browser/android/download/download_location_dialog_bridge.h"
+#include "chrome/browser/android/download/download_location_dialog_bridge_impl.h"
#include "base/android/jni_android.h"
#include "base/android/jni_string.h"
#include "chrome/browser/android/download/download_controller.h"
#include "chrome/browser/android/download/download_manager_service.h"
-#include "content/public/browser/web_contents.h"
#include "jni/DownloadLocationDialogBridge_jni.h"
+#include "ui/android/window_android.h"
-DownloadLocationDialogBridge::DownloadLocationDialogBridge()
+DownloadLocationDialogBridgeImpl::DownloadLocationDialogBridgeImpl()
: is_dialog_showing_(false) {
JNIEnv* env = base::android::AttachCurrentThread();
java_obj_.Reset(env, Java_DownloadLocationDialogBridge_create(
@@ -20,18 +20,25 @@
DCHECK(!java_obj_.is_null());
}
-DownloadLocationDialogBridge::~DownloadLocationDialogBridge() {
+DownloadLocationDialogBridgeImpl::~DownloadLocationDialogBridgeImpl() {
JNIEnv* env = base::android::AttachCurrentThread();
Java_DownloadLocationDialogBridge_destroy(env, java_obj_);
}
-void DownloadLocationDialogBridge::ShowDialog(
- content::WebContents* web_contents,
+void DownloadLocationDialogBridgeImpl::ShowDialog(
+ gfx::NativeWindow native_window,
+ DownloadLocationDialogType dialog_type,
const base::FilePath& suggested_path,
const DownloadTargetDeterminerDelegate::ConfirmationCallback& callback) {
- if (!web_contents)
+ if (!native_window)
return;
+ // This shouldn't happen, but if it does, cancel download.
+ if (dialog_type == DownloadLocationDialogType::NO_DIALOG) {
+ NOTREACHED();
+ callback.Run(DownloadConfirmationResult::CANCELED, base::FilePath());
+ }
+
// If dialog is showing, run the callback to continue without confirmation.
if (is_dialog_showing_) {
if (!callback.is_null()) {
@@ -46,12 +53,13 @@
JNIEnv* env = base::android::AttachCurrentThread();
Java_DownloadLocationDialogBridge_showDialog(
- env, java_obj_, web_contents->GetJavaWebContents(),
+ env, java_obj_, native_window->GetJavaObject(),
+ static_cast<int>(dialog_type),
base::android::ConvertUTF8ToJavaString(env,
suggested_path.AsUTF8Unsafe()));
}
-void DownloadLocationDialogBridge::OnComplete(
+void DownloadLocationDialogBridgeImpl::OnComplete(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj,
const base::android::JavaParamRef<jstring>& returned_path) {
@@ -60,14 +68,14 @@
if (!dialog_complete_callback_.is_null()) {
base::ResetAndReturn(&dialog_complete_callback_)
- .Run(DownloadConfirmationResult::CONFIRMED,
+ .Run(DownloadConfirmationResult::CONFIRMED_WITH_DIALOG,
base::FilePath(FILE_PATH_LITERAL(path_string)));
}
is_dialog_showing_ = false;
}
-void DownloadLocationDialogBridge::OnCanceled(
+void DownloadLocationDialogBridgeImpl::OnCanceled(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj) {
if (!dialog_complete_callback_.is_null()) {
diff --git a/chrome/browser/android/download/download_location_dialog_bridge_impl.h b/chrome/browser/android/download/download_location_dialog_bridge_impl.h
new file mode 100644
index 0000000..ede0423
--- /dev/null
+++ b/chrome/browser/android/download/download_location_dialog_bridge_impl.h
@@ -0,0 +1,46 @@
+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_ANDROID_DOWNLOAD_DOWNLOAD_LOCATION_DIALOG_BRIDGE_IMPL_H_
+#define CHROME_BROWSER_ANDROID_DOWNLOAD_DOWNLOAD_LOCATION_DIALOG_BRIDGE_IMPL_H_
+
+#include "base/android/jni_android.h"
+#include "base/android/scoped_java_ref.h"
+#include "base/callback.h"
+#include "chrome/browser/android/download/download_location_dialog_bridge.h"
+#include "chrome/browser/download/download_confirmation_result.h"
+#include "chrome/browser/download/download_location_dialog_type.h"
+#include "chrome/browser/download/download_target_determiner_delegate.h"
+#include "ui/gfx/native_widget_types.h"
+
+class DownloadLocationDialogBridgeImpl : public DownloadLocationDialogBridge {
+ public:
+ DownloadLocationDialogBridgeImpl();
+ ~DownloadLocationDialogBridgeImpl() override;
+
+ // DownloadLocationDialogBridge implementation.
+ void ShowDialog(gfx::NativeWindow native_window,
+ DownloadLocationDialogType dialog_type,
+ const base::FilePath& suggested_path,
+ const DownloadTargetDeterminerDelegate::ConfirmationCallback&
+ callback) override;
+
+ void OnComplete(
+ JNIEnv* env,
+ const base::android::JavaParamRef<jobject>& obj,
+ const base::android::JavaParamRef<jstring>& returned_path) override;
+
+ void OnCanceled(JNIEnv* env,
+ const base::android::JavaParamRef<jobject>& obj) override;
+
+ private:
+ jboolean is_dialog_showing_;
+ base::android::ScopedJavaGlobalRef<jobject> java_obj_;
+ DownloadTargetDeterminerDelegate::ConfirmationCallback
+ dialog_complete_callback_;
+
+ DISALLOW_COPY_AND_ASSIGN(DownloadLocationDialogBridgeImpl);
+};
+
+#endif // CHROME_BROWSER_ANDROID_DOWNLOAD_DOWNLOAD_LOCATION_DIALOG_BRIDGE_IMPL_H_
diff --git a/chrome/browser/download/chrome_download_manager_delegate.cc b/chrome/browser/download/chrome_download_manager_delegate.cc
index 469adb1..76c41d2 100644
--- a/chrome/browser/download/chrome_download_manager_delegate.cc
+++ b/chrome/browser/download/chrome_download_manager_delegate.cc
@@ -28,6 +28,7 @@
#include "chrome/browser/download/download_file_picker.h"
#include "chrome/browser/download/download_history.h"
#include "chrome/browser/download/download_item_model.h"
+#include "chrome/browser/download/download_location_dialog_type.h"
#include "chrome/browser/download/download_path_reservation_tracker.h"
#include "chrome/browser/download/download_prefs.h"
#include "chrome/browser/download/download_request_limiter.h"
@@ -44,6 +45,7 @@
#include "chrome/browser/ui/scoped_tabbed_browser_displayer.h"
#include "chrome/common/buildflags.h"
#include "chrome/common/chrome_constants.h"
+#include "chrome/common/chrome_features.h"
#include "chrome/common/pdf_uma.h"
#include "chrome/common/pref_names.h"
#include "chrome/common/safe_browsing/file_type_policies.h"
@@ -66,8 +68,10 @@
#include "ui/base/l10n/l10n_util.h"
#if defined(OS_ANDROID)
+#include "base/android/path_utils.h"
#include "chrome/browser/android/download/chrome_duplicate_download_infobar_delegate.h"
#include "chrome/browser/android/download/download_controller.h"
+#include "chrome/browser/android/download/download_location_dialog_bridge_impl.h"
#include "chrome/browser/android/download/download_manager_service.h"
#include "chrome/browser/infobars/infobar_service.h"
#endif
@@ -274,7 +278,7 @@
download_metadata_cache_.reset(new download::InProgressCacheImpl(
metadata_cache_file, disk_access_task_runner_));
#if defined(OS_ANDROID)
- location_dialog_bridge_.reset(new DownloadLocationDialogBridge);
+ location_dialog_bridge_.reset(new DownloadLocationDialogBridgeImpl);
#endif
}
@@ -294,6 +298,13 @@
}
}
+#if defined(OS_ANDROID)
+void ChromeDownloadManagerDelegate::SetDownloadLocationDialogBridgeForTesting(
+ DownloadLocationDialogBridge* bridge) {
+ location_dialog_bridge_.reset(bridge);
+}
+#endif
+
void ChromeDownloadManagerDelegate::Shutdown() {
download_prefs_.reset();
weak_ptr_factory_.InvalidateWeakPtrs();
@@ -776,63 +787,121 @@
#if defined(OS_ANDROID)
content::WebContents* web_contents =
content::DownloadItemUtils::GetWebContents(download);
- switch (reason) {
- case DownloadConfirmationReason::NONE:
- NOTREACHED();
- return;
-
- case DownloadConfirmationReason::TARGET_PATH_NOT_WRITEABLE:
- DownloadManagerService::OnDownloadCanceled(
- download, DownloadController::CANCEL_REASON_NO_EXTERNAL_STORAGE);
- callback.Run(DownloadConfirmationResult::CANCELED, base::FilePath());
- return;
-
- case DownloadConfirmationReason::PREFERENCE:
- if (web_contents) {
- location_dialog_bridge_->ShowDialog(web_contents, suggested_path,
- callback);
- } else {
- // For now, if there are no WebContents, continue anyways.
- callback.Run(DownloadConfirmationResult::CONTINUE_WITHOUT_CONFIRMATION,
- suggested_path);
- }
- return;
-
- case DownloadConfirmationReason::NAME_TOO_LONG:
- case DownloadConfirmationReason::TARGET_NO_SPACE:
- // These are errors. But rather than cancel the download we are going to
- // continue with the current path so that the download will get
- // interrupted again.
- //
- // Ideally we'd allow the user to try another location, but on Android,
- // the user doesn't have much of a choice (currently). So we skip the
- // prompt and try the same location.
-
- case DownloadConfirmationReason::SAVE_AS:
+ if (base::FeatureList::IsEnabled(features::kDownloadsLocationChange)) {
+ if (reason == DownloadConfirmationReason::SAVE_AS) {
+ // If this is a 'Save As' download, just run without confirmation.
callback.Run(DownloadConfirmationResult::CONTINUE_WITHOUT_CONFIRMATION,
suggested_path);
- return;
-
- case DownloadConfirmationReason::TARGET_CONFLICT:
- if (web_contents) {
- android::ChromeDuplicateDownloadInfoBarDelegate::Create(
- InfoBarService::FromWebContents(web_contents), download,
- suggested_path, callback);
+ } else if (!web_contents ||
+ reason == DownloadConfirmationReason::UNEXPECTED) {
+ // If there are no web_contents and there are no errors (ie. location
+ // dialog is only being requested because of a user preference), continue.
+ if (reason == DownloadConfirmationReason::PREFERENCE) {
+ callback.Run(DownloadConfirmationResult::CONTINUE_WITHOUT_CONFIRMATION,
+ suggested_path);
return;
}
- FALLTHROUGH;
- // If we cannot reserve the path and the WebContent is already gone, there
- // is no way to prompt user for an infobar. This could happen after chrome
- // gets killed, and user tries to resume a download while another app has
- // created the target file (not the temporary .crdownload file).
- case DownloadConfirmationReason::UNEXPECTED:
+ // If we cannot reserve the path and the WebContent is already gone, there
+ // is no way to prompt user for a dialog. This could happen after chrome
+ // gets killed, and user tries to resume a download while another app has
+ // created the target file (not the temporary .crdownload file).
DownloadManagerService::OnDownloadCanceled(
download,
DownloadController::CANCEL_REASON_CANNOT_DETERMINE_DOWNLOAD_TARGET);
callback.Run(DownloadConfirmationResult::CANCELED, base::FilePath());
+ } else if (reason == DownloadConfirmationReason::TARGET_CONFLICT) {
+ // If there is a file that already has the same name, try to generate a
+ // unique name for the new download (ie. "image (1).png" vs "image.png").
+ base::FilePath download_dir;
+ if (!base::android::GetDownloadsDirectory(&download_dir)) {
+ callback.Run(DownloadConfirmationResult::CANCELED, base::FilePath());
+ return;
+ }
+ gfx::NativeWindow native_window = web_contents->GetTopLevelNativeWindow();
+ DownloadPathReservationTracker::GetReservedPath(
+ download, suggested_path, download_dir, true,
+ DownloadPathReservationTracker::UNIQUIFY,
+ base::BindRepeating(
+ &ChromeDownloadManagerDelegate::GenerateUniqueFileNameDone,
+ weak_ptr_factory_.GetWeakPtr(), native_window, callback));
return;
+ } else {
+ // Figure out type of dialog and display.
+ DownloadLocationDialogType dialog_type =
+ DownloadLocationDialogType::DEFAULT;
+ switch (reason) {
+ case DownloadConfirmationReason::TARGET_NO_SPACE:
+ dialog_type = DownloadLocationDialogType::LOCATION_FULL;
+ break;
+
+ case DownloadConfirmationReason::TARGET_PATH_NOT_WRITEABLE:
+ dialog_type = DownloadLocationDialogType::LOCATION_NOT_FOUND;
+ break;
+
+ case DownloadConfirmationReason::NAME_TOO_LONG:
+ dialog_type = DownloadLocationDialogType::NAME_TOO_LONG;
+ break;
+
+ case DownloadConfirmationReason::PREFERENCE:
+ default:
+ break;
+ }
+
+ gfx::NativeWindow native_window = web_contents->GetTopLevelNativeWindow();
+ location_dialog_bridge_->ShowDialog(native_window, dialog_type,
+ suggested_path, callback);
+ }
+ } else {
+ switch (reason) {
+ case DownloadConfirmationReason::NONE:
+ NOTREACHED();
+ return;
+
+ case DownloadConfirmationReason::TARGET_PATH_NOT_WRITEABLE:
+ DownloadManagerService::OnDownloadCanceled(
+ download, DownloadController::CANCEL_REASON_NO_EXTERNAL_STORAGE);
+ callback.Run(DownloadConfirmationResult::CANCELED, base::FilePath());
+ return;
+
+ case DownloadConfirmationReason::PREFERENCE:
+ case DownloadConfirmationReason::NAME_TOO_LONG:
+ case DownloadConfirmationReason::TARGET_NO_SPACE:
+ // These are errors. But rather than cancel the download we are going to
+ // continue with the current path so that the download will get
+ // interrupted again.
+ //
+ // Ideally we'd allow the user to try another location, but on Android,
+ // the user doesn't have much of a choice (currently). So we skip the
+ // prompt and try the same location.
+
+ case DownloadConfirmationReason::SAVE_AS:
+ callback.Run(DownloadConfirmationResult::CONTINUE_WITHOUT_CONFIRMATION,
+ suggested_path);
+ return;
+
+ case DownloadConfirmationReason::TARGET_CONFLICT:
+ if (web_contents) {
+ android::ChromeDuplicateDownloadInfoBarDelegate::Create(
+ InfoBarService::FromWebContents(web_contents), download,
+ suggested_path, callback);
+ return;
+ }
+ FALLTHROUGH;
+
+ // If we cannot reserve the path and the WebContent is already gone, there
+ // is no way to prompt user for an infobar. This could happen after chrome
+ // gets killed, and user tries to resume a download while another app has
+ // created the target file (not the temporary .crdownload file).
+ case DownloadConfirmationReason::UNEXPECTED:
+ DownloadManagerService::OnDownloadCanceled(
+ download,
+ DownloadController::CANCEL_REASON_CANNOT_DETERMINE_DOWNLOAD_TARGET);
+ callback.Run(DownloadConfirmationResult::CANCELED, base::FilePath());
+ return;
+ }
}
+
#else // !OS_ANDROID
// Desktop Chrome displays a file picker for all confirmation needs. We can do
// better.
@@ -840,6 +909,26 @@
#endif // !OS_ANDROID
}
+#if defined(OS_ANDROID)
+void ChromeDownloadManagerDelegate::GenerateUniqueFileNameDone(
+ gfx::NativeWindow native_window,
+ const DownloadTargetDeterminerDelegate::ConfirmationCallback& callback,
+ PathValidationResult result,
+ const base::FilePath& target_path) {
+ // After a new, unique filename has been generated, display the error dialog
+ // with the filename automatically set to be the unique filename.
+ DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+ if (result == PathValidationResult::SUCCESS) {
+ location_dialog_bridge_->ShowDialog(
+ native_window, DownloadLocationDialogType::NAME_CONFLICT, target_path,
+ callback);
+ } else {
+ // If the name generation failed, fail the download.
+ callback.Run(DownloadConfirmationResult::FAILED, base::FilePath());
+ }
+}
+#endif
+
void ChromeDownloadManagerDelegate::DetermineLocalPath(
DownloadItem* download,
const base::FilePath& virtual_path,
diff --git a/chrome/browser/download/chrome_download_manager_delegate.h b/chrome/browser/download/chrome_download_manager_delegate.h
index dbe1bbcf..4db0502 100644
--- a/chrome/browser/download/chrome_download_manager_delegate.h
+++ b/chrome/browser/download/chrome_download_manager_delegate.h
@@ -29,6 +29,7 @@
#include "content/public/browser/notification_observer.h"
#include "content/public/browser/notification_registrar.h"
#include "extensions/buildflags/buildflags.h"
+#include "ui/gfx/native_widget_types.h"
#if defined(OS_ANDROID)
#include "chrome/browser/android/download/download_location_dialog_bridge.h"
@@ -59,6 +60,10 @@
static void DisableSafeBrowsing(download::DownloadItem* item);
void SetDownloadManager(content::DownloadManager* dm);
+#if defined(OS_ANDROID)
+ void SetDownloadLocationDialogBridgeForTesting(
+ DownloadLocationDialogBridge* bridge);
+#endif
// Callbacks passed to GetNextId() will not be called until the returned
// callback is called.
@@ -198,6 +203,16 @@
bool storage_permission_granted,
bool allow);
+#if defined(OS_ANDROID)
+ // Called after a unique file name is generated in the case that there is a
+ // TARGET_CONFLICT and the new file name should be displayed to the user.
+ void GenerateUniqueFileNameDone(
+ gfx::NativeWindow native_window,
+ const DownloadTargetDeterminerDelegate::ConfirmationCallback& callback,
+ PathValidationResult result,
+ const base::FilePath& target_path);
+#endif
+
Profile* profile_;
std::unique_ptr<download::InProgressCache> download_metadata_cache_;
diff --git a/chrome/browser/download/chrome_download_manager_delegate_unittest.cc b/chrome/browser/download/chrome_download_manager_delegate_unittest.cc
index e61ae4db..7197867 100644
--- a/chrome/browser/download/chrome_download_manager_delegate_unittest.cc
+++ b/chrome/browser/download/chrome_download_manager_delegate_unittest.cc
@@ -15,6 +15,7 @@
#include "base/path_service.h"
#include "base/run_loop.h"
#include "base/single_thread_task_runner.h"
+#include "base/test/scoped_feature_list.h"
#include "base/test/scoped_path_override.h"
#include "base/threading/thread_task_runner_handle.h"
#include "build/build_config.h"
@@ -24,6 +25,7 @@
#include "chrome/browser/download/download_target_info.h"
#include "chrome/browser/safe_browsing/download_protection/download_protection_util.h"
#include "chrome/common/buildflags.h"
+#include "chrome/common/chrome_features.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/common/pref_names.h"
@@ -980,9 +982,56 @@
int infobar_count_ = 0;
};
+class TestDownloadLocationDialogBridge : public DownloadLocationDialogBridge {
+ public:
+ TestDownloadLocationDialogBridge() {}
+
+ // DownloadLocationDialogBridge implementation.
+ void ShowDialog(gfx::NativeWindow native_window,
+ DownloadLocationDialogType dialog_type,
+ const base::FilePath& suggested_path,
+ const DownloadTargetDeterminerDelegate::ConfirmationCallback&
+ callback) override {
+ dialog_shown_count_++;
+ dialog_type_ = dialog_type;
+ callback.Run(DownloadConfirmationResult::CANCELED, base::FilePath());
+ }
+
+ void OnComplete(
+ JNIEnv* env,
+ const base::android::JavaParamRef<jobject>& obj,
+ const base::android::JavaParamRef<jstring>& returned_path) override {}
+
+ void OnCanceled(JNIEnv* env,
+ const base::android::JavaParamRef<jobject>& obj) override {}
+
+ // Returns the number of times ShowDialog has been called.
+ int GetDialogShownCount() { return dialog_shown_count_; }
+
+ // Returns the type of the last dialog that was called to be shown.
+ DownloadLocationDialogType GetDialogType() { return dialog_type_; }
+
+ // Resets the stored information.
+ void ResetStoredVariables() {
+ dialog_shown_count_ = 0;
+ dialog_type_ = DownloadLocationDialogType::NO_DIALOG;
+ }
+
+ private:
+ int dialog_shown_count_;
+ DownloadLocationDialogType dialog_type_;
+ DownloadTargetDeterminerDelegate::ConfirmationCallback
+ dialog_complete_callback_;
+
+ DISALLOW_COPY_AND_ASSIGN(TestDownloadLocationDialogBridge);
+};
+
} // namespace
TEST_F(ChromeDownloadManagerDelegateTest, RequestConfirmation_Android) {
+ EXPECT_FALSE(
+ base::FeatureList::IsEnabled(features::kDownloadsLocationChange));
+
enum class WebContents { AVAILABLE, NONE };
enum class ExpectPath { FULL, EMPTY };
enum class ExpectInfoBar { YES, NO };
@@ -1009,11 +1058,6 @@
DownloadConfirmationResult::CONTINUE_WITHOUT_CONFIRMATION,
WebContents::AVAILABLE, ExpectInfoBar::NO, ExpectPath::FULL},
- // TODO(jming): Fix test when download location storage is complete.
- // {DownloadConfirmationReason::PREFERENCE,
- // DownloadConfirmationResult::CONTINUE_WITHOUT_CONFIRMATION,
- // WebContents::AVAILABLE, ExpectInfoBar::NO, ExpectPath::FULL},
-
// This case results in an infobar. The logic above dismisses the infobar
// and counts it for testing. The functionality of the infobar is not
// tested here other than that dimssing the infobar is treated as a user
@@ -1071,4 +1115,124 @@
infobar_counter.CheckAndResetInfobarCount());
}
}
+
+TEST_F(ChromeDownloadManagerDelegateTest,
+ RequestConfirmation_Android_WithLocationChangeEnabled) {
+ DeleteContents();
+ SetContents(CreateTestWebContents());
+
+ base::test::ScopedFeatureList scoped_list;
+ scoped_list.InitAndEnableFeature(features::kDownloadsLocationChange);
+ EXPECT_TRUE(base::FeatureList::IsEnabled(features::kDownloadsLocationChange));
+
+ enum class WebContents { AVAILABLE, NONE };
+ enum class ExpectPath { FULL, EMPTY };
+ struct {
+ DownloadConfirmationReason confirmation_reason;
+ DownloadConfirmationResult expected_result;
+ WebContents web_contents;
+ DownloadLocationDialogType dialog_type;
+ ExpectPath path;
+ } kTestCases[] = {
+ // SAVE_AS
+ {DownloadConfirmationReason::SAVE_AS,
+ DownloadConfirmationResult::CONTINUE_WITHOUT_CONFIRMATION,
+ WebContents::AVAILABLE, DownloadLocationDialogType::NO_DIALOG,
+ ExpectPath::FULL},
+ {DownloadConfirmationReason::SAVE_AS,
+ DownloadConfirmationResult::CONTINUE_WITHOUT_CONFIRMATION,
+ WebContents::NONE, DownloadLocationDialogType::NO_DIALOG,
+ ExpectPath::FULL},
+
+ // !web_contents
+ {DownloadConfirmationReason::PREFERENCE,
+ DownloadConfirmationResult::CONTINUE_WITHOUT_CONFIRMATION,
+ WebContents::NONE, DownloadLocationDialogType::NO_DIALOG,
+ ExpectPath::FULL},
+ {DownloadConfirmationReason::TARGET_CONFLICT,
+ DownloadConfirmationResult::CANCELED, WebContents::NONE,
+ DownloadLocationDialogType::NO_DIALOG, ExpectPath::EMPTY},
+ {DownloadConfirmationReason::TARGET_NO_SPACE,
+ DownloadConfirmationResult::CANCELED, WebContents::NONE,
+ DownloadLocationDialogType::NO_DIALOG, ExpectPath::EMPTY},
+ {DownloadConfirmationReason::TARGET_PATH_NOT_WRITEABLE,
+ DownloadConfirmationResult::CANCELED, WebContents::NONE,
+ DownloadLocationDialogType::NO_DIALOG, ExpectPath::EMPTY},
+ {DownloadConfirmationReason::NAME_TOO_LONG,
+ DownloadConfirmationResult::CANCELED, WebContents::NONE,
+ DownloadLocationDialogType::NO_DIALOG, ExpectPath::EMPTY},
+
+ // UNEXPECTED
+ {DownloadConfirmationReason::UNEXPECTED,
+ DownloadConfirmationResult::CANCELED, WebContents::AVAILABLE,
+ DownloadLocationDialogType::NO_DIALOG, ExpectPath::EMPTY},
+ {DownloadConfirmationReason::UNEXPECTED,
+ DownloadConfirmationResult::CANCELED, WebContents::NONE,
+ DownloadLocationDialogType::NO_DIALOG, ExpectPath::EMPTY},
+
+ // TARGET_CONFLICT
+ {DownloadConfirmationReason::TARGET_CONFLICT,
+ DownloadConfirmationResult::CANCELED, WebContents::AVAILABLE,
+ DownloadLocationDialogType::NAME_CONFLICT, ExpectPath::EMPTY},
+
+ // Other error dialogs
+ {DownloadConfirmationReason::TARGET_NO_SPACE,
+ DownloadConfirmationResult::CANCELED, WebContents::AVAILABLE,
+ DownloadLocationDialogType::LOCATION_FULL, ExpectPath::EMPTY},
+ {DownloadConfirmationReason::TARGET_PATH_NOT_WRITEABLE,
+ DownloadConfirmationResult::CANCELED, WebContents::AVAILABLE,
+ DownloadLocationDialogType::LOCATION_NOT_FOUND, ExpectPath::EMPTY},
+ {DownloadConfirmationReason::NAME_TOO_LONG,
+ DownloadConfirmationResult::CANCELED, WebContents::AVAILABLE,
+ DownloadLocationDialogType::NAME_TOO_LONG, ExpectPath::EMPTY},
+ };
+
+ EXPECT_CALL(*delegate(), RequestConfirmation(_, _, _, _))
+ .WillRepeatedly(Invoke(
+ delegate(),
+ &TestChromeDownloadManagerDelegate::RequestConfirmationConcrete));
+ base::FilePath fake_path = GetPathInDownloadDir(FILE_PATH_LITERAL("foo.txt"));
+ GURL url("http://example.com");
+ TestDownloadLocationDialogBridge* location_dialog_bridge =
+ new TestDownloadLocationDialogBridge();
+ delegate()->SetDownloadLocationDialogBridgeForTesting(
+ static_cast<DownloadLocationDialogBridge*>(location_dialog_bridge));
+
+ for (const auto& test_case : kTestCases) {
+ std::unique_ptr<download::MockDownloadItem> download_item =
+ CreateActiveDownloadItem(1);
+ content::DownloadItemUtils::AttachInfo(
+ download_item.get(), profile(),
+ test_case.web_contents == WebContents::AVAILABLE ? web_contents()
+ : nullptr);
+ EXPECT_CALL(*download_item, GetURL()).WillRepeatedly(ReturnRef(url));
+ location_dialog_bridge->ResetStoredVariables();
+
+ base::RunLoop loop;
+ const auto callback = base::BindRepeating(
+ [](const base::RepeatingClosure& quit_closure,
+ DownloadConfirmationResult expected_result,
+ const base::FilePath& expected_path,
+ DownloadConfirmationResult actual_result,
+ const base::FilePath& actual_path) {
+ EXPECT_EQ(expected_result, actual_result);
+ EXPECT_EQ(expected_path, actual_path);
+ quit_closure.Run();
+ },
+ loop.QuitClosure(), test_case.expected_result,
+ test_case.path == ExpectPath::FULL ? fake_path : base::FilePath());
+ delegate()->RequestConfirmation(download_item.get(), fake_path,
+ test_case.confirmation_reason, callback);
+ loop.Run();
+
+ EXPECT_EQ(
+ test_case.dialog_type != DownloadLocationDialogType::NO_DIALOG ? 1 : 0,
+ location_dialog_bridge->GetDialogShownCount());
+ EXPECT_EQ(test_case.dialog_type, location_dialog_bridge->GetDialogType());
+
+ EXPECT_CALL(*download_item, GetState())
+ .WillRepeatedly(Return(DownloadItem::COMPLETE));
+ download_item->NotifyObserversDownloadUpdated();
+ }
+}
#endif // OS_ANDROID
diff --git a/chrome/browser/download/download_confirmation_result.h b/chrome/browser/download/download_confirmation_result.h
index b09a0a3..7b92014 100644
--- a/chrome/browser/download/download_confirmation_result.h
+++ b/chrome/browser/download/download_confirmation_result.h
@@ -5,6 +5,8 @@
#ifndef CHROME_BROWSER_DOWNLOAD_DOWNLOAD_CONFIRMATION_RESULT_H_
#define CHROME_BROWSER_DOWNLOAD_DOWNLOAD_CONFIRMATION_RESULT_H_
+#include "build/build_config.h"
+
// Result of RequestConfirmation delegate method for
// DownloadTargetDeterminerDelegate.
enum class DownloadConfirmationResult {
@@ -22,7 +24,15 @@
// delegate should use this value instead of CONFIRMED if the user was not
// presented with some UI that explicitly called out the filename being
// downloaded.
- CONTINUE_WITHOUT_CONFIRMATION
+ CONTINUE_WITHOUT_CONFIRMATION,
+
+#if defined(OS_ANDROID)
+ // After the user confirmed the file path on the dialog, we still need to
+ // check to make sure the path is valid. This is only used in the Android
+ // case because there is no equivalent DownloadLocationPicker to handle
+ // all of the error cases.
+ CONFIRMED_WITH_DIALOG
+#endif
};
#endif // CHROME_BROWSER_DOWNLOAD_DOWNLOAD_CONFIRMATION_RESULT_H_
diff --git a/chrome/browser/download/download_location_dialog_type.h b/chrome/browser/download/download_location_dialog_type.h
new file mode 100644
index 0000000..b590c39
--- /dev/null
+++ b/chrome/browser/download/download_location_dialog_type.h
@@ -0,0 +1,20 @@
+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_DOWNLOAD_DOWNLOAD_LOCATION_DIALOG_TYPE_H_
+#define CHROME_BROWSER_DOWNLOAD_DOWNLOAD_LOCATION_DIALOG_TYPE_H_
+
+// The type of download location dialog that should by shown by Android.
+// A Java counterpart will be generated for this enum.
+// GENERATED_JAVA_ENUM_PACKAGE: org.chromium.chrome.browser.download
+enum class DownloadLocationDialogType {
+ NO_DIALOG, // No dialog.
+ DEFAULT, // Dialog without any error states.
+ LOCATION_FULL, // Error dialog, default location is full.
+ LOCATION_NOT_FOUND, // Error dialog, default location is not found.
+ NAME_CONFLICT, // Error dialog, there is already a file with that name.
+ NAME_TOO_LONG, // Error dialog, the file name is too long.
+};
+
+#endif // CHROME_BROWSER_DOWNLOAD_DOWNLOAD_LOCATION_DIALOG_TYPE_H_
diff --git a/chrome/browser/download/download_target_determiner.cc b/chrome/browser/download/download_target_determiner.cc
index 80e4fdc..cbc50ff 100644
--- a/chrome/browser/download/download_target_determiner.cc
+++ b/chrome/browser/download/download_target_determiner.cc
@@ -102,6 +102,9 @@
danger_level_(DownloadFileType::NOT_DANGEROUS),
virtual_path_(initial_virtual_path),
is_filetype_handled_safely_(false),
+#if defined(OS_ANDROID)
+ is_checking_dialog_confirmed_path_(false),
+#endif
download_(download),
is_resumption_(download_->GetLastReason() !=
download::DOWNLOAD_INTERRUPT_REASON_NONE &&
@@ -408,14 +411,29 @@
// Avoid prompting for a download if it isn't in-progress. The user will be
// prompted once the download is resumed and headers are available.
- if (confirmation_reason_ != DownloadConfirmationReason::NONE &&
- download_->GetState() == DownloadItem::IN_PROGRESS) {
- delegate_->RequestConfirmation(
- download_, virtual_path_, confirmation_reason_,
- base::Bind(&DownloadTargetDeterminer::RequestConfirmationDone,
- weak_ptr_factory_.GetWeakPtr()));
- return QUIT_DOLOOP;
+ if (download_->GetState() == DownloadItem::IN_PROGRESS) {
+#if defined(OS_ANDROID)
+ // If we were looping back to check the user-confirmed path from the
+ // dialog, and there were no additional errors, continue.
+ if (is_checking_dialog_confirmed_path_ &&
+ (confirmation_reason_ == DownloadConfirmationReason::PREFERENCE ||
+ confirmation_reason_ == DownloadConfirmationReason::NONE)) {
+ is_checking_dialog_confirmed_path_ = false;
+ return CONTINUE;
+ }
+#endif
+
+ // If there is a non-neutral confirmation reason, prompt the user.
+ if (confirmation_reason_ != DownloadConfirmationReason::NONE) {
+ delegate_->RequestConfirmation(
+ download_, virtual_path_, confirmation_reason_,
+ base::BindRepeating(
+ &DownloadTargetDeterminer::RequestConfirmationDone,
+ weak_ptr_factory_.GetWeakPtr()));
+ return QUIT_DOLOOP;
+ }
}
+
return CONTINUE;
}
@@ -425,6 +443,9 @@
DCHECK_CURRENTLY_ON(BrowserThread::UI);
DCHECK(!download_->IsTransient());
DVLOG(20) << "User selected path:" << virtual_path.AsUTF8Unsafe();
+#if defined(OS_ANDROID)
+ is_checking_dialog_confirmed_path_ = false;
+#endif
if (result == DownloadConfirmationResult::CANCELED) {
ScheduleCallbackAndDeleteSelf(
download::DOWNLOAD_INTERRUPT_REASON_USER_CANCELED);
@@ -440,6 +461,16 @@
confirmation_reason_ = DownloadConfirmationReason::NONE;
virtual_path_ = virtual_path;
+
+#if defined(OS_ANDROID)
+ if (result == DownloadConfirmationResult::CONFIRMED_WITH_DIALOG) {
+ // Double check the user-selected path is valid by looping back.
+ is_checking_dialog_confirmed_path_ = true;
+ confirmation_reason_ = DownloadConfirmationReason::NONE;
+ next_state_ = STATE_RESERVE_VIRTUAL_PATH;
+ }
+#endif
+
download_prefs_->SetSaveFilePath(virtual_path_.DirName());
DoLoop();
}
diff --git a/chrome/browser/download/download_target_determiner.h b/chrome/browser/download/download_target_determiner.h
index 133117c..d3f49be1 100644
--- a/chrome/browser/download/download_target_determiner.h
+++ b/chrome/browser/download/download_target_determiner.h
@@ -323,6 +323,9 @@
base::FilePath intermediate_path_;
std::string mime_type_;
bool is_filetype_handled_safely_;
+#if defined(OS_ANDROID)
+ bool is_checking_dialog_confirmed_path_;
+#endif
download::DownloadItem* download_;
const bool is_resumption_;
diff --git a/ui/android/java/res/values-v17/styles.xml b/ui/android/java/res/values-v17/styles.xml
index 5694b47..8e4bcef 100644
--- a/ui/android/java/res/values-v17/styles.xml
+++ b/ui/android/java/res/values-v17/styles.xml
@@ -75,6 +75,10 @@
<item name="android:textColor">@color/black_alpha_38</item>
<item name="android:textSize">@dimen/text_size_small</item>
</style>
+ <style name="BlackDisabledText3" tools:ignore="UnusedResources">
+ <item name="android:textColor">@color/black_alpha_38</item>
+ <item name="android:textSize">@dimen/text_size_medium</item>
+ </style>
<style name="BlackBodyDefault" tools:ignore="UnusedResources">
<item name="android:textColor">@color/black_alpha_87</item>
<item name="android:textSize">@dimen/text_size_medium</item>
