[Android] Prompt with infobar on filename conflict

chrome/browser/ui/android/infobars/download_overwrite_infobar.cc

Index: chrome/browser/ui/android/infobars/download_overwrite_infobar.cc
diff --git a/chrome/browser/ui/android/infobars/download_overwrite_infobar.cc b/chrome/browser/ui/android/infobars/download_overwrite_infobar.cc
new file mode 100644
index 0000000000000000000000000000000000000000..a5da96935be3dc757dc2a598a4f7a2de0325099f
--- /dev/null
+++ b/chrome/browser/ui/android/infobars/download_overwrite_infobar.cc
@@ -0,0 +1,72 @@
+// Copyright 2015 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.
+
+#include "chrome/browser/ui/android/infobars/download_overwrite_infobar.h"
+
+#include "base/android/jni_android.h"
+#include "base/android/jni_array.h"
+#include "base/android/jni_string.h"
+#include "base/android/jni_weak_ref.h"
+#include "base/files/file_path.h"
+#include "chrome/browser/android/download/download_overwrite_infobar_delegate.h"
+#include "jni/DownloadOverwriteInfoBar_jni.h"
+
+using chrome::android::DownloadOverwriteInfoBarDelegate;
+
+// static
+scoped_ptr<infobars::InfoBar> DownloadOverwriteInfoBar::CreateInfoBar(
+    scoped_ptr<DownloadOverwriteInfoBarDelegate> delegate) {
+  return make_scoped_ptr(new DownloadOverwriteInfoBar(delegate.Pass()));
+}
+
+DownloadOverwriteInfoBar::~DownloadOverwriteInfoBar() {
+}
+
+DownloadOverwriteInfoBar::DownloadOverwriteInfoBar(
+    scoped_ptr<DownloadOverwriteInfoBarDelegate> delegate)
+    : InfoBarAndroid(delegate.Pass()) {
+}
+
+base::android::ScopedJavaLocalRef<jobject>
+DownloadOverwriteInfoBar::CreateRenderInfoBar(JNIEnv* env) {
+  DownloadOverwriteInfoBarDelegate* delegate = GetDelegate();
+
+  ScopedJavaLocalRef<jstring> j_file_name =
+      base::android::ConvertUTF8ToJavaString(env, delegate->GetFileName());
+  ScopedJavaLocalRef<jstring> j_dir_name =
+      base::android::ConvertUTF8ToJavaString(env, delegate->GetDirName());
+  ScopedJavaLocalRef<jstring> j_dir_full_path =
+      base::android::ConvertUTF8ToJavaString(env, delegate->GetDirFullPath());
+  base::android::ScopedJavaLocalRef<jobject> java_infobar(
+      Java_DownloadOverwriteInfoBar_createInfoBar(
+          env, reinterpret_cast<intptr_t>(this), j_file_name.obj(),
+          j_dir_name.obj(), j_dir_full_path.obj()));
+  return java_infobar;
+}
+
+void DownloadOverwriteInfoBar::ProcessButton(int action,
+                                             const std::string& action_value) {
+  if (!owner())
+    return;  // We're closing; don't call anything, it might access the owner.
+
+  DownloadOverwriteInfoBarDelegate* delegate = GetDelegate();
+  if (action == InfoBarAndroid::ACTION_OVERWRITE)
+    delegate->OverwriteExistingFile();
+  else if (action == InfoBarAndroid::ACTION_CREATE_NEW_FILE)
+    delegate->CreateNewFile();
+  else
+    DCHECK(false);

[Peter Kasting, 2015/03/02 21:05:15]

You can shorten both this code and the code in
DownloadOverwriteInfoBarDelegateForAndroidDownloadManager by collapsing the
OverwriteExistingFile() and CreateNewFile() functions into a single function
that takes a bool, e.g. OnUserAction(bool overwrite_existing_file).  Then the
implementation of that function (for Android at least) is just:

  ChromeDownloadDelegate::EnqueueDownloadManagerRequest(
      base::android::AttachCurrentThread(), chrome_download_delegate_,
      overwrite_existing_file, download_info_);

And the code for this block can be:

  DCHECK((action == InfoBarAndroid::ACTION_OVERWRITE) ||
         (action == InfoBarAndroid::ACTION_CREATE_NEW_FILE));
  GetDelegate()->OnUserAction(action == InfoBarAndroid::ACTION_OVERWRITE);

[Changwan Ryu, 2015/03/04 05:15:04]

Hmm... I understand that it will shorten code length in the
AndroidDownloadManager code flow, but I feel that it is not as clear as the
current implementation.

For instance, it is not very clear that the user action was 'create new file'
when overwrite_existing_file was false, so I'll need to add additional javadoc
to describe it. So I prefer how it's currently implemented.

+
+  RemoveSelf();
+}
+
+DownloadOverwriteInfoBarDelegate* DownloadOverwriteInfoBar::GetDelegate() {
+  return static_cast<DownloadOverwriteInfoBarDelegate*>(delegate());
+}
+
+// Native JNI methods ---------------------------------------------------------
+
+bool RegisterDownloadOverwriteInfoBarDelegate(JNIEnv* env) {
+  return RegisterNativesImpl(env);
+}