Commit f865da82 authored by Xing Liu's avatar Xing Liu Committed by Commit Bot

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.

Bug: 845311
Change-Id: I8a3bf53cbb9edcd5905003b5739400243977e55a
Reviewed-on: https://chromium-review.googlesource.com/1074378
Commit-Queue: Xing Liu <xingliu@chromium.org>
Reviewed-by: default avatarMin Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562558}
parent d9bbf493
......@@ -10,6 +10,8 @@ import android.content.Context;
import android.view.LayoutInflater;
import android.view.View;
import android.widget.CheckBox;
import android.widget.CompoundButton;
import android.widget.CompoundButton.OnCheckedChangeListener;
import android.widget.Spinner;
import android.widget.TextView;
......@@ -26,7 +28,7 @@ import javax.annotation.Nullable;
/**
* Dialog that is displayed to ask user where they want to download the file.
*/
public class DownloadLocationDialog extends ModalDialogView {
public class DownloadLocationDialog extends ModalDialogView implements OnCheckedChangeListener {
private DownloadDirectoryAdapter mDirectoryAdapter;
private AlertDialogEditText mFileName;
......@@ -109,6 +111,15 @@ public class DownloadLocationDialog extends ModalDialogView {
boolean isInitial = PrefServiceBridge.getInstance().getPromptForDownloadAndroid()
== DownloadPromptStatus.SHOW_INITIAL;
mDontShowAgain.setChecked(isInitial);
mDontShowAgain.setOnCheckedChangeListener(this);
}
// CompoundButton.OnCheckedChangeListener implementation.
@Override
public void onCheckedChanged(CompoundButton buttonView, boolean isChecked) {
int newStatus =
isChecked ? DownloadPromptStatus.DONT_SHOW : DownloadPromptStatus.SHOW_PREFERENCE;
PrefServiceBridge.getInstance().setPromptForDownloadAndroid(newStatus);
}
// Helper methods available to DownloadLocationDialogBridge.
......
......@@ -957,9 +957,17 @@ void ChromeDownloadManagerDelegate::GenerateUniqueFileNameDone(
// with the filename automatically set to be the unique filename.
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
if (result == PathValidationResult::SUCCESS) {
ChooseDownloadLocation(
native_window, DownloadLocationDialogType::NAME_CONFLICT, target_path,
base::BindOnce(&OnDownloadLocationDetermined, callback));
if (download_prefs_ && download_prefs_->PromptForDownload()) {
ChooseDownloadLocation(
native_window, DownloadLocationDialogType::NAME_CONFLICT, target_path,
base::BindOnce(&OnDownloadLocationDetermined, callback));
return;
}
// If user chose not to show download location dialog, uses current unique
// target path.
callback.Run(DownloadConfirmationResult::CONTINUE_WITHOUT_CONFIRMATION,
target_path);
} else {
// If the name generation failed, fail the download.
callback.Run(DownloadConfirmationResult::FAILED, base::FilePath());
......
......@@ -53,6 +53,7 @@
#endif
#if defined(OS_ANDROID)
#include "chrome/browser/download/download_prompt_status.h"
#include "chrome/browser/infobars/infobar_service.h"
#include "components/infobars/core/infobar.h"
#include "components/infobars/core/infobar_delegate.h"
......@@ -1195,6 +1196,10 @@ TEST_F(ChromeDownloadManagerDelegateTest,
scoped_list.InitAndEnableFeature(features::kDownloadsLocationChange);
EXPECT_TRUE(base::FeatureList::IsEnabled(features::kDownloadsLocationChange));
profile()->GetTestingPrefService()->SetInteger(
prefs::kPromptForDownloadAndroid,
static_cast<int>(DownloadPromptStatus::SHOW_PREFERENCE));
enum class WebContents { AVAILABLE, NONE };
enum class ExpectPath { FULL, EMPTY };
struct {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment