Infobar system refactor.

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

Index: chrome/browser/ui/android/infobars/confirm_infobar.cc
===================================================================
--- chrome/browser/ui/android/infobars/confirm_infobar.cc	(revision 226624)
+++ chrome/browser/ui/android/infobars/confirm_infobar.cc	(working copy)
@@ -15,16 +15,16 @@
 // ConfirmInfoBarDelegate -----------------------------------------------------
 
 // static
-InfoBar* ConfirmInfoBarDelegate::CreateInfoBar(InfoBarService* owner) {
-  return new ConfirmInfoBar(owner, this);
+scoped_ptr<InfoBar> ConfirmInfoBarDelegate::CreateInfoBar(
+    scoped_ptr<ConfirmInfoBarDelegate> delegate) {
+  return scoped_ptr<InfoBar>(new ConfirmInfoBar(delegate.Pass()));
 }
 
 
 // ConfirmInfoBar -------------------------------------------------------------
 
-ConfirmInfoBar::ConfirmInfoBar(InfoBarService* owner, InfoBarDelegate* delegate)
-    : InfoBarAndroid(owner, delegate),
-      delegate_(delegate->AsConfirmInfoBarDelegate()),
+ConfirmInfoBar::ConfirmInfoBar(scoped_ptr<ConfirmInfoBarDelegate> delegate)
+    : InfoBarAndroid(delegate.PassAs<InfoBarDelegate>()),
       java_confirm_delegate_() {
 }
 
@@ -42,7 +42,7 @@
           env, GetTextFor(ConfirmInfoBarDelegate::BUTTON_CANCEL));
   base::android::ScopedJavaLocalRef<jstring> message_text =
       base::android::ConvertUTF16ToJavaString(
-          env, delegate_->GetMessageText());
+          env, GetDelegate()->GetMessageText());
 
   return Java_ConfirmInfoBarDelegate_showConfirmInfoBar(
       env, java_confirm_delegate_.obj(), reinterpret_cast<jint>(this),
@@ -52,17 +52,28 @@
 
 void ConfirmInfoBar::ProcessButton(int action,
                                    const std::string& action_value) {
+  // TODO(pkasting): The subsequent check isn't necessary if Android infobar
+  // buttons become unclickable once the infobar is unowned.

[Miguel Garcia, 2013/10/07 15:19:00]

just to make sure I understand correctly. Are you suggesting that we intercept
the clear_owner method on infobar_service and do some jni to disable the
buttons? That would be less racy than this check right? 

Right now there is still the chance of owner being ok here but by the time we
are in say the delegate->Accept() implementation it might be null already.

[Peter Kasting, 2013/10/07 20:19:24]

I'm not actually suggesting you do anything in particular.  I was merely writing
this in case the implementation actually did this now or in the future, so that
we wouldn't have code we didn't need.

For reference, GTK basically disables the buttons (specifically, their
callbacks) when the infobar is unowned, while views does what we're doing here. 
Anything is fine.
 
How?  That would imply the owner can get changed on a different thread than this
code is executing on.  That shouldn't happen.

+  if (!owner())
+    return;  // We're closing; don't call anything, it might access the owner.
+
   DCHECK((action == InfoBarAndroid::ACTION_OK) ||
       (action == InfoBarAndroid::ACTION_CANCEL));
+  ConfirmInfoBarDelegate* delegate = GetDelegate();
   if ((action == InfoBarAndroid::ACTION_OK) ?
-      delegate_->Accept() : delegate_->Cancel())
+      delegate->Accept() : delegate->Cancel())
     CloseInfoBar();
 }
 
+ConfirmInfoBarDelegate* ConfirmInfoBar::GetDelegate() {
+  return delegate()->AsConfirmInfoBarDelegate();
+}
+
 string16 ConfirmInfoBar::GetTextFor(
     ConfirmInfoBarDelegate::InfoBarButton button) {
-  return (delegate_->GetButtons() & button) ?
-      delegate_->GetButtonLabel(button) : string16();
+  ConfirmInfoBarDelegate* delegate = GetDelegate();
+  return (delegate->GetButtons() & button) ?
+      delegate->GetButtonLabel(button) : string16();
 }