[Android] Prompt with infobar on filename conflict

chrome/browser/android/download_overwrite_infobar_delegate.h

+// 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.
+
+#ifndef CHROME_BROWSER_ANDROID_DOWNLOAD_OVERWRITE_INFOBAR_DELEGATE_H_
+#define CHROME_BROWSER_ANDROID_DOWNLOAD_OVERWRITE_INFOBAR_DELEGATE_H_
+
+#include "base/callback.h"
+#include "base/files/file_path.h"
+#include "components/infobars/core/infobar_delegate.h"
+
+namespace content {
+class DownloadItem;
+}
+
+namespace chrome {
+namespace android {
+
+// An infobar delegate for DownloadOverwriteInfoBar implemented for
+// use by ChromeDownloadManagerDelegate.

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

Nit: Maybe instead of a comment that's a bunch of class names, say in English
what the purpose of this infobar is and where it appears?

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

Done.

+class DownloadOverwriteInfoBarDelegate : public infobars::InfoBarDelegate {
+ public:
+  // Callback to be invoked when PromptUserForDownloadPath() completes.
+  // Originally defined in download_target_determiner_delegate.h, but
+  // redefined to avoid a template-related compile error.

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

What error?  I'm confused why this needs to be here.

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

Hmm... Removed as the compiler error isn't reproduced.

+  typedef base::Callback<void(const base::FilePath& virtual_path)>
+      FileSelectedCallback;
+  virtual ~DownloadOverwriteInfoBarDelegate();

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

Nit: Blank line above and below this so each type of method gets its own section

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

Nit: "override" rather than "virtual"

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

Done.

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

Done.

+  static infobars::InfoBar* Create(
+      content::DownloadItem* download,
+      const base::FilePath& suggested_path,
+      const FileSelectedCallback& callback);
+
+  // InfoBarDelegate:
+  virtual DownloadOverwriteInfoBarDelegate*

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

Nit: No "virtual" when "override" is present

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

Done.

+      AsDownloadOverwriteInfoBarDelegate() override;
+
+  bool Accept();

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

Nit: Say what Accept() and Cancel() do.

Perhaps you should rename these to be more descriptive, since you're not bound
by the genericism of ConfirmInfoBarDelegate.

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

Done.

+  bool Cancel();
+  std::string GetFileName() const;
+  std::string GetDirName() const;
+  std::string GetDirFullPath() const;
+
+ protected:
+  DownloadOverwriteInfoBarDelegate(

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

Why is this protected rather than private?  No one subclasses this class AFAICT.

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

Done.

+      content::DownloadItem* download,
+      const base::FilePath& suggested_path,
+      const FileSelectedCallback& callback);
+
+ private:
+  int pending_id_;

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

Nit: Document these.

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

Done.

+  base::FilePath suggested_path_;
+  FileSelectedCallback callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(DownloadOverwriteInfoBarDelegate);
+};
+
+}  // namespace android
+}  // namespace chrome
+
+#endif  // CHROME_BROWSER_ANDROID_DOWNLOAD_OVERWRITE_INFOBAR_DELEGATE_H_