Discourage creation of Java infobars without InfoBarDelegates

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

Index: chrome/browser/ui/android/infobars/simple_confirm_infobar_builder.cc
diff --git a/chrome/browser/ui/android/infobars/simple_confirm_infobar_builder.cc b/chrome/browser/ui/android/infobars/simple_confirm_infobar_builder.cc
new file mode 100644
index 0000000000000000000000000000000000000000..3b074033901592a3553ad057a3140551d1750985
--- /dev/null
+++ b/chrome/browser/ui/android/infobars/simple_confirm_infobar_builder.cc
@@ -0,0 +1,167 @@
+// Copyright 2016 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/simple_confirm_infobar_builder.h"
+
+#include "base/android/jni_android.h"
+#include "base/android/jni_string.h"
+#include "base/android/scoped_java_ref.h"
+#include "base/memory/scoped_ptr.h"
+#include "chrome/browser/android/tab_android.h"
+#include "chrome/browser/infobars/infobar_service.h"
+#include "components/infobars/core/confirm_infobar_delegate.h"
+#include "components/infobars/core/infobar.h"
+#include "jni/SimpleConfirmInfoBarBuilder_jni.h"
+#include "ui/gfx/android/java_bitmap.h"
+#include "ui/gfx/image/image.h"
+#include "ui/gfx/vector_icons_public.h"
+
+namespace {
+
+// Delegate for a simple ConfirmInfoBar triggered via JNI.
+class SimpleConfirmInfoBarDelegate : public ConfirmInfoBarDelegate {
+ public:
+  SimpleConfirmInfoBarDelegate(
+      const JavaParamRef<jobject>& j_listener,
+      infobars::InfoBarDelegate::InfoBarIdentifier infobar_identifier,
+      const gfx::Image& bitmap,
+      const base::string16& message_str,
+      const base::string16& primary_str,
+      const base::string16& secondary_str,
+      bool auto_expire);
+
+  ~SimpleConfirmInfoBarDelegate() override;
+
+  // InfoBarDelegate overrides:

[Peter Kasting, 2016/01/16 01:00:56]

Nit: You don't directly subclass this class, so these should be combined with
the ConfirmInfoBarDelegate overrides below.

[gone, 2016/01/19 23:43:03]

Done.

+  infobars::InfoBarDelegate::InfoBarIdentifier GetIdentifier() const override;
+  gfx::Image GetIcon() const override;
+  bool ShouldExpire(const NavigationDetails& details) const override;
+  void InfoBarDismissed() override;
+
+  // ConfirmInfoBarDelegate overrides:

[Peter Kasting, 2016/01/16 01:00:56]

Nit: No need for " overrides"

[gone, 2016/01/19 23:43:03]

Done.

+  base::string16 GetMessageText() const override;
+  int GetButtons() const override;
+  base::string16 GetButtonLabel(InfoBarButton button) const override;
+  bool Accept() override;
+  bool Cancel() override;
+
+ private:
+  base::android::ScopedJavaGlobalRef<jobject> java_listener_;
+  infobars::InfoBarDelegate::InfoBarIdentifier identifier_;
+  gfx::Image icon_bitmap_;
+  base::string16 message_str_;
+  base::string16 primary_str_;
+  base::string16 secondary_str_;
+  bool auto_expire_;
+};

[Peter Kasting, 2016/01/16 01:00:56]

Nit: DISALLOW_COPY_AND_ASSIGN

[gone, 2016/01/19 23:43:03]

Argh, I always miss that one when I stay in Java too long.  Thanks!

+
+SimpleConfirmInfoBarDelegate::SimpleConfirmInfoBarDelegate(
+    const JavaParamRef<jobject>& j_listener,
+    infobars::InfoBarDelegate::InfoBarIdentifier identifier,
+    const gfx::Image& bitmap,
+    const base::string16& message_str,
+    const base::string16& primary_str,
+    const base::string16& secondary_str,
+    bool auto_expire)
+    : identifier_(identifier),
+      icon_bitmap_(bitmap),
+      message_str_(message_str),
+      primary_str_(primary_str),
+      secondary_str_(secondary_str),
+      auto_expire_(auto_expire) {
+  java_listener_.Reset(j_listener);
+}
+
+SimpleConfirmInfoBarDelegate::~SimpleConfirmInfoBarDelegate() {
+}
+
+infobars::InfoBarDelegate::InfoBarIdentifier
+SimpleConfirmInfoBarDelegate::GetIdentifier() const {
+  return identifier_;
+}
+
+gfx::Image SimpleConfirmInfoBarDelegate::GetIcon() const {
+  return icon_bitmap_.IsEmpty() ? ConfirmInfoBarDelegate::GetIcon()
+                                : icon_bitmap_;
+}
+
+bool SimpleConfirmInfoBarDelegate::ShouldExpire(
+    const NavigationDetails& details) const {
+  return auto_expire_ && ConfirmInfoBarDelegate::ShouldExpire(details);
+}
+
+void SimpleConfirmInfoBarDelegate::InfoBarDismissed() {
+  Java_SimpleConfirmInfoBarBuilder_onInfoBarDismissed(
+      base::android::AttachCurrentThread(), java_listener_.obj());
+}
+
+base::string16 SimpleConfirmInfoBarDelegate::GetMessageText() const {
+  return message_str_;
+}
+
+int SimpleConfirmInfoBarDelegate::GetButtons() const {
+  int buttons = 0;
+  if (primary_str_.size() > 0) buttons |= BUTTON_OK;

[Peter Kasting, 2016/01/16 01:00:56]

Nit: Chromium style avoids placing even short conditional bodies on the same
line.

Prefer !empty() to "size() > 0".

Here's a way of implementing this that avoids these issues and it shorter to
boot:

  return (primary_str_.empty() ? 0 : BUTTON_OK) |
      (secondary_str_.empty() ? 0 : BUTTON_CANCEL);

[gone, 2016/01/19 23:43:03]

Nice; switched over

+  if (secondary_str_.size() > 0) buttons |= BUTTON_CANCEL;
+  return buttons;
+}
+
+base::string16
+SimpleConfirmInfoBarDelegate::GetButtonLabel(InfoBarButton button) const {
+  return button == BUTTON_OK ? primary_str_ : secondary_str_;
+}
+
+bool SimpleConfirmInfoBarDelegate::Accept() {
+  Java_SimpleConfirmInfoBarBuilder_onInfoBarButtonClicked(
+      base::android::AttachCurrentThread(), java_listener_.obj(), true);
+  return true;
+}
+
+bool SimpleConfirmInfoBarDelegate::Cancel() {
+  Java_SimpleConfirmInfoBarBuilder_onInfoBarButtonClicked(
+      base::android::AttachCurrentThread(), java_listener_.obj(), false);
+  return true;
+}
+
+}  // anonymous namespace
+
+

[Peter Kasting, 2016/01/16 01:00:56]

Nit: Too many blank lines

[gone, 2016/01/19 23:43:03]

Done.

+
+// Native JNI methods ---------------------------------------------------------
+
+void Create(JNIEnv* env,
+            const JavaParamRef<jclass>& j_caller,
+            const JavaParamRef<jobject>& j_tab,
+            jint j_identifier,
+            const JavaParamRef<jobject>& j_icon,
+            const JavaParamRef<jstring>& j_message,
+            const JavaParamRef<jstring>& j_primary,
+            const JavaParamRef<jstring>& j_secondary,
+            jboolean auto_expire,
+            const JavaParamRef<jobject>& j_listener) {
+  infobars::InfoBarDelegate::InfoBarIdentifier infobar_identifier =
+      static_cast<infobars::InfoBarDelegate::InfoBarIdentifier>(j_identifier);
+
+  gfx::Image icon_bitmap;
+  if (j_icon) {
+    icon_bitmap = gfx::Image::CreateFrom1xBitmap(
+        gfx::CreateSkBitmapFromJavaBitmap(gfx::JavaBitmap(j_icon)));
+  }
+
+  InfoBarService* service = InfoBarService::FromWebContents(
+      TabAndroid::GetNativeTab(env, j_tab)->web_contents());
+  service->AddInfoBar(service->CreateConfirmInfoBar(
+      scoped_ptr<ConfirmInfoBarDelegate>(new SimpleConfirmInfoBarDelegate(

[Peter Kasting, 2016/01/16 01:00:56]

Nit: make_scoped_ptr?

[gone, 2016/01/19 23:43:03]

Done.

+          j_listener,
+          infobar_identifier,
+          icon_bitmap,
+          base::android::ConvertJavaStringToUTF16(env, j_message),
+          base::android::ConvertJavaStringToUTF16(env, j_primary),
+          base::android::ConvertJavaStringToUTF16(env, j_secondary),
+          auto_expire))));
+}
+
+bool RegisterSimpleConfirmInfoBarBuilder(JNIEnv* env) {
+  return RegisterNativesImpl(env);
+}