Implement instant apps infobar.

chrome/browser/android/instantapps/instant_apps_infobar_delegate.cc

Index: chrome/browser/android/instantapps/instant_apps_infobar_delegate.cc
diff --git a/chrome/browser/android/instantapps/instant_apps_infobar_delegate.cc b/chrome/browser/android/instantapps/instant_apps_infobar_delegate.cc
new file mode 100644
index 0000000000000000000000000000000000000000..14c8e1ec2fbdc9069f8b1f3c89fb391a6c2735cf
--- /dev/null
+++ b/chrome/browser/android/instantapps/instant_apps_infobar_delegate.cc
@@ -0,0 +1,60 @@
+// 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/android/instantapps/instant_apps_infobar_delegate.h"
+
+#include "base/android/jni_android.h"
+#include "base/android/jni_string.h"
+#include "base/memory/ptr_util.h"
+#include "chrome/browser/infobars/infobar_service.h"
+#include "chrome/browser/ui/android/infobars/instant_apps_infobar.h"
+#include "components/infobars/core/infobar_delegate.h"
+#include "content/public/browser/web_contents.h"
+#include "jni/InstantAppsInfoBarDelegate_jni.h"
+
+
+InstantAppsInfoBarDelegate::InstantAppsInfoBarDelegate(
+    const base::android::ScopedJavaGlobalRef<jobject>& jdata) : data_(jdata) {

[Peter Kasting, 2016/08/23 07:59:22]

Don't expose a public constructor.  Infobar delegate constructors should be
private.

Instead expose a public Create() function that takes an InfoBarService* and any
necessary args, and make that function create the delegate and add it to the
infobar service.  Basically, what Launch() is doing, except drop the first two
(unused) args and move the computation of the InfoBarService* to the caller
side.

[Maria, 2016/08/23 21:30:27]

Done.

+  JNIEnv* env = base::android::AttachCurrentThread();
+  java_delegate_.Reset(Java_InstantAppsInfoBarDelegate_create(

[Peter Kasting, 2016/08/23 07:59:22]

Do we really need both a Java infobar and a Java infobar delegate?  Seems like
most Android infobars get by with just the former.

And it seems like usually, when the Java side wants to access the C++ delegate,
the Java infobar has a pointer to the C++ infobar, and from there it can get at
the infobar delegate.  I'm uneasy about having an object that has a direct
pointer to this delegate.

[Maria, 2016/08/23 21:30:27]

Many Java infobars actually do have java infobar delegates, see:

AppBannerInofBarDelegateAndroid, DataReductionPromoInfoBarDelegate,
GeneratePassowrdSavedInfoBarDelegate, PermissionUpdateInfoBarDelegate

I view the delegate as the controller for the infobar where the view is the
infobar class itself.

The pointer is needed when making calls from java to native. This CL currently
doesn't do it, but it ostensibly might. I am just putting in the basics for now.
And it seems like the other delegates do this. What's the reason you think
Java-side shouldn't have access to the delegate?

[Peter Kasting, 2016/08/23 22:10:57]

Are you sure it will need this in the future?  If not, don't add it until you
know you need it.
You can always get at the infobar delegate from the infobar, but not the other
way around.  And it seems like the Java side should have a pointer to the
infobar anyway.  Isn't that what the Java_InfoBar_setNativeInfoBar call in the
InfoBarAndroid code sets up?

So I would assume you don't also need a direct delegate pointer.

[Maria, 2016/08/23 23:55:59]

Done.

+      env,
+      reinterpret_cast<intptr_t>(this)));
+}
+
+InstantAppsInfoBarDelegate::~InstantAppsInfoBarDelegate() {}
+
+infobars::InfoBarDelegate::InfoBarIdentifier
+InstantAppsInfoBarDelegate::GetIdentifier() const {
+  return INSTANT_APPS_INFOBAR_DELEGATE_ANDROID;
+}
+
+base::string16 InstantAppsInfoBarDelegate::GetMessageText() const {
+  // Message is set in InstantAppInfobar.java

[Peter Kasting, 2016/08/23 07:59:22]

So why are we overriding this method?

[Maria, 2016/08/23 21:30:27]

Because ConfirmInfoBar only provides a pure virtual implementation.

+  return base::string16();
+}
+
+bool InstantAppsInfoBarDelegate::Accept() {
+  JNIEnv* env = base::android::AttachCurrentThread();
+  Java_InstantAppsInfoBarDelegate_openInstantApp(env, java_delegate_.obj());
+  return true;
+}
+
+void Launch(JNIEnv* env,
+            const base::android::JavaParamRef<jclass>& clazz,
+            const base::android::JavaParamRef<jobject>& jweb_contents,
+            const base::android::JavaParamRef<jobject>& jdata) {
+  content::WebContents* web_contents =
+        content::WebContents::FromJavaWebContents(jweb_contents);
+  InfoBarService* infobar_service =
+        InfoBarService::FromWebContents(web_contents);
+  std::unique_ptr<InstantAppsInfoBarDelegate> delegate(
+      new InstantAppsInfoBarDelegate(
+          base::android::ScopedJavaGlobalRef<jobject>(jdata)));

[Peter Kasting, 2016/08/23 07:59:22]

AndroidDownloadManagerOverwriteInfoBarDelegate just passes this in as a jobject
by value instead of constructing a ScopedJavaGlobalRef here.  The scoped ref
doesn't seem to buy much anyway given that the infobar delegate constructs its
own, so this ref lives only for the life of the function call.  Seems like we
should just nuke it.

[Maria, 2016/08/23 21:30:27]

No, this data is used by InstantAppsInfoBar::CreateRenderInfoBar(). If we don't
wrap this into ScopedJavaGlobalRef, the java object can be garbage collected in
java heap, but we need to keep it around to pass to the infobar.

[Peter Kasting, 2016/08/23 22:10:57]

Right, and that's why the delegate wraps this in a scoped ref.  I wasn't
proposing removing that one.  I was proposing removing the scoped ref we create
_here_ just to pass to the constructor which then refs it (again) in the member
object.

[Maria, 2016/08/23 23:55:59]

Done.

+  infobar_service->AddInfoBar(base::WrapUnique(

[Peter Kasting, 2016/08/23 07:59:22]

Nit: Use MakeUnique() rather than WrapUnique(new()).

[Maria, 2016/08/23 21:30:27]

Done.

+      new InstantAppsInfoBar(std::move(delegate))));
+}
+
+bool RegisterInstantAppsInfoBarDelegate(JNIEnv* env) {
+  return RegisterNativesImpl(env);
+}