Skip to content

Commit b828621

Browse files
author
Xing Liu
committed
Download location: Fix issue for location dialog prompt pref.
When users choose not to prompt the download location dialog. We still prompt the dialog. This is caused by two reasons: 1. The pref is persisted when user click the download button. Instead, the pref should also be persisted when users click on the checkbox. 2. When exisiting file results in file name conflict, we didn't check the pref. [email protected] (cherry picked from commit f865da8) Bug: 845311 Change-Id: I8a3bf53cbb9edcd5905003b5739400243977e55a Reviewed-on: https://chromium-review.googlesource.com/1074378 Commit-Queue: Xing Liu <[email protected]> Reviewed-by: Min Qin <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#562558} Reviewed-on: https://chromium-review.googlesource.com/1080037 Reviewed-by: Xing Liu <[email protected]> Cr-Commit-Position: refs/branch-heads/3440@{#51} Cr-Branched-From: 010ddcf-refs/heads/master@{#561733}
1 parent b4de322 commit b828621

File tree

3 files changed

+28
-4
lines changed

3 files changed

+28
-4
lines changed

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
import android.view.LayoutInflater;
1111
import android.view.View;
1212
import android.widget.CheckBox;
13+
import android.widget.CompoundButton;
14+
import android.widget.CompoundButton.OnCheckedChangeListener;
1315
import android.widget.Spinner;
1416
import android.widget.TextView;
1517

@@ -26,7 +28,7 @@
2628
/**
2729
* Dialog that is displayed to ask user where they want to download the file.
2830
*/
29-
public class DownloadLocationDialog extends ModalDialogView {
31+
public class DownloadLocationDialog extends ModalDialogView implements OnCheckedChangeListener {
3032
private DownloadDirectoryAdapter mDirectoryAdapter;
3133

3234
private AlertDialogEditText mFileName;
@@ -109,6 +111,15 @@ private DownloadLocationDialog(Controller controller, Context context,
109111
boolean isInitial = PrefServiceBridge.getInstance().getPromptForDownloadAndroid()
110112
== DownloadPromptStatus.SHOW_INITIAL;
111113
mDontShowAgain.setChecked(isInitial);
114+
mDontShowAgain.setOnCheckedChangeListener(this);
115+
}
116+
117+
// CompoundButton.OnCheckedChangeListener implementation.
118+
@Override
119+
public void onCheckedChanged(CompoundButton buttonView, boolean isChecked) {
120+
int newStatus =
121+
isChecked ? DownloadPromptStatus.DONT_SHOW : DownloadPromptStatus.SHOW_PREFERENCE;
122+
PrefServiceBridge.getInstance().setPromptForDownloadAndroid(newStatus);
112123
}
113124

114125
// Helper methods available to DownloadLocationDialogBridge.

chrome/browser/download/chrome_download_manager_delegate.cc

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -957,9 +957,17 @@ void ChromeDownloadManagerDelegate::GenerateUniqueFileNameDone(
957957
// with the filename automatically set to be the unique filename.
958958
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
959959
if (result == PathValidationResult::SUCCESS) {
960-
ChooseDownloadLocation(
961-
native_window, DownloadLocationDialogType::NAME_CONFLICT, target_path,
962-
base::BindOnce(&OnDownloadLocationDetermined, callback));
960+
if (download_prefs_ && download_prefs_->PromptForDownload()) {
961+
ChooseDownloadLocation(
962+
native_window, DownloadLocationDialogType::NAME_CONFLICT, target_path,
963+
base::BindOnce(&OnDownloadLocationDetermined, callback));
964+
return;
965+
}
966+
967+
// If user chose not to show download location dialog, uses current unique
968+
// target path.
969+
callback.Run(DownloadConfirmationResult::CONTINUE_WITHOUT_CONFIRMATION,
970+
target_path);
963971
} else {
964972
// If the name generation failed, fail the download.
965973
callback.Run(DownloadConfirmationResult::FAILED, base::FilePath());

chrome/browser/download/chrome_download_manager_delegate_unittest.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
#endif
5454

5555
#if defined(OS_ANDROID)
56+
#include "chrome/browser/download/download_prompt_status.h"
5657
#include "chrome/browser/infobars/infobar_service.h"
5758
#include "components/infobars/core/infobar.h"
5859
#include "components/infobars/core/infobar_delegate.h"
@@ -1195,6 +1196,10 @@ TEST_F(ChromeDownloadManagerDelegateTest,
11951196
scoped_list.InitAndEnableFeature(features::kDownloadsLocationChange);
11961197
EXPECT_TRUE(base::FeatureList::IsEnabled(features::kDownloadsLocationChange));
11971198

1199+
profile()->GetTestingPrefService()->SetInteger(
1200+
prefs::kPromptForDownloadAndroid,
1201+
static_cast<int>(DownloadPromptStatus::SHOW_PREFERENCE));
1202+
11981203
enum class WebContents { AVAILABLE, NONE };
11991204
enum class ExpectPath { FULL, EMPTY };
12001205
struct {

0 commit comments

Comments
 (0)