[Android] Prompt with infobar on filename conflict

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

+// Copyright 2014 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 "chrome/browser/android/download_overwrite_infobar_delegate.h"
+#include "jni/DownloadOverwriteInfoBarDelegate_jni.h"
+
+using base::android::ConvertUTF8ToJavaString;

[Peter Kasting, 2014/10/24 01:32:23]

Nit: I believe at least this statement can be omitted without making the lines
below overflow the 80-column limit.

In general, avoid using directives unless they significantly improve the rest of
the file.

[Changwan Ryu, 2014/10/27 06:40:04]

Done.

+using chrome::android::DownloadOverwriteInfoBarDelegate;
+
+// static
+scoped_ptr<infobars::InfoBar> DownloadOverwriteInfoBar::CreateInfoBar(
+    scoped_ptr<DownloadOverwriteInfoBarDelegate> delegate) {
+  return scoped_ptr<infobars::InfoBar>(

[Peter Kasting, 2014/10/24 01:32:22]

Nit: Can use make_scoped_ptr, I think?

[Changwan Ryu, 2014/10/27 06:40:04]

Done.

+      new DownloadOverwriteInfoBar(delegate.Pass()));
+}
+
+DownloadOverwriteInfoBar::~DownloadOverwriteInfoBar() {
+}
+
+DownloadOverwriteInfoBar::DownloadOverwriteInfoBar(
+    scoped_ptr<DownloadOverwriteInfoBarDelegate> delegate)
+    : InfoBarAndroid(delegate.PassAs<infobars::InfoBarDelegate>()),

[Peter Kasting, 2014/10/24 01:32:22]

Nit: Can just use Pass() now

[Changwan Ryu, 2014/10/27 06:40:04]

Done.

+      java_delegate_() {
+}
+
+ScopedJavaLocalRef<jobject> DownloadOverwriteInfoBar::CreateRenderInfoBar(
+    JNIEnv* env) {
+  java_delegate_.Reset(Java_DownloadOverwriteInfoBarDelegate_create(env));
+  DownloadOverwriteInfoBarDelegate* delegate = GetDelegate();
+  ScopedJavaLocalRef<jstring> j_file_name =
+       ConvertUTF8ToJavaString(env, delegate->GetFileName());
+  ScopedJavaLocalRef<jstring> j_dir_name =
+       ConvertUTF8ToJavaString(env, delegate->GetDirName());
+  ScopedJavaLocalRef<jstring> j_dir_full_path =
+       ConvertUTF8ToJavaString(env, delegate->GetDirFullPath());
+  return Java_DownloadOverwriteInfoBarDelegate_showDownloadOverwriteInfoBar(
+      env, java_delegate_.obj(), reinterpret_cast<intptr_t>(this),
+      j_file_name.obj(), j_dir_name.obj(), j_dir_full_path.obj());
+}
+
+void DownloadOverwriteInfoBar::ProcessButton(
+    int action, const std::string& action_value) {

[Peter Kasting, 2014/10/24 01:32:22]

Nit: More typical wrapping:

void DownloadOverwriteInfoBar::ProcessButton(int action,
                                             const std::string& action_value) {

[Changwan Ryu, 2014/10/27 06:40:04]

Done.

+  if (!owner())
+    return;  // We're closing; don't call anything, it might access the owner.
+
+  DownloadOverwriteInfoBarDelegate* delegate = GetDelegate();
+  if (action == InfoBarAndroid::ACTION_OK) {

[Peter Kasting, 2014/10/24 01:32:22]

Nit: {} unnecessary

[Changwan Ryu, 2014/10/27 06:40:04]

Done.

+    delegate->Accept();
+  } else if (action == InfoBarAndroid::ACTION_CANCEL) {
+    delegate->Cancel();
+  } else {
+    DCHECK_EQ(InfoBarAndroid::ACTION_NONE, action);

[Peter Kasting, 2014/10/24 01:32:22]

When is this arm reached?  On clicking the close button?

[Changwan Ryu, 2014/10/27 06:40:04]

Actually it shouldn't be reached. Changed as DCHECK(false).

+  }
+
+  RemoveSelf();
+}
+
+DownloadOverwriteInfoBarDelegate* DownloadOverwriteInfoBar::GetDelegate() {
+  return delegate()->AsDownloadOverwriteInfoBarDelegate();

[Peter Kasting, 2014/10/24 01:32:23]

Just static-cast here, since you know the type of the infobar delegate.  This
eliminates the need for AsDownloadOverwriteInfoBarDelegate() and thus the need
to add knowledge of this type (even under ifdefs) to cross-platform files like
infobar_delegate.*.

(Since this is only called twice, you could even inline this into the two
callers, but I'm fine either way, whatever you prefer.)

[Changwan Ryu, 2014/10/27 06:40:04]

Fixed to static-cast.

+}
+
+// Native JNI methods ---------------------------------------------------------
+
+bool RegisterDownloadOverwriteInfoBarDelegate(JNIEnv* env) {
+  return RegisterNativesImpl(env);
+}