Infobar system refactor.

Infobar system refactor.

This changes the ownership model of infobars so that InfoBars are long-lived and
own their InfoBarDelegates directly.  The InfoBarService pseudo-owns the
InfoBars (instead of deleting them directly, it tells them when they're unowned
and expects them to delete themselves).

This fixes leaks when infobars are closed while not visible (e.g. in a
background tab) and in general makes the system clearer and easier to reason
about.

BUG=62154
TEST=none
R=erg@chromium.org, sail@chromium.org, sky@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=238283

[Peter Kasting, 2013-09-04 00:41:58]

Android is not yet working, but I'm in contact with the relevant Android folks
about that.

In the meantime, I think this is ready to go to review.  sail, I'm sending this
to you for the initial overall review pass, since you've presumably already
looked through most of the cross-platform stuff while writing the Mac stuff
recently.  Let me know if there are particular files or directories you can't
review.

erg: GTK OWNERS.

[Elliot Glaysher, 2013-09-04 17:56:13]

On 2013/09/04 00:41:58, Peter Kasting wrote:
> Android is not yet working, but I'm in contact with the relevant Android folks
> about that.
> 
> In the meantime, I think this is ready to go to review.  sail, I'm sending
this
> to you for the initial overall review pass, since you've presumably already
> looked through most of the cross-platform stuff while writing the Mac stuff
> recently.  Let me know if there are particular files or directories you can't
> review.
> 
> erg: GTK OWNERS.

Whoa. Is this THE patch? The one that everything has been building up to for
years?

[sail, 2013-09-04 18:59:09]

cocoa/* LGTM
CC rsesek who reviewed the cocoa code in crrev.com/23224018.

[Elliot Glaysher, 2013-09-04 20:34:18]

gtk lgtm btw

[sail, 2013-09-04 21:37:08]

browser/infobar/* LGTM!
Rest of the changes look mechanical so LGTM those as well.

[Miguel Garcia, 2013-10-07 15:18:57]

https://chromiumcodereview.appspot.com/22694006/diff/439001/chrome/browser/ui...
File chrome/browser/ui/android/infobars/confirm_infobar.cc (right):

https://chromiumcodereview.appspot.com/22694006/diff/439001/chrome/browser/ui...
chrome/browser/ui/android/infobars/confirm_infobar.cc:56: // buttons become
unclickable once the infobar is unowned.
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.

https://chromiumcodereview.appspot.com/22694006/diff/439001/chrome/browser/ui...
File chrome/browser/ui/android/infobars/infobar_android.cc (right):

https://chromiumcodereview.appspot.com/22694006/diff/439001/chrome/browser/ui...
chrome/browser/ui/android/infobars/infobar_android.cc:66: CloseInfoBar();  //
TODO(pkasting): Is this correct?
This method is being removed in https://codereview.chromium.org/26176007/

Right now, closing the java infobar here is not needed. When the user clicks the
X button in java we first close the java bar, then notify C++ via this method.
This was a mistake really and but it was done like that in an attempt to share
the java implementation by java only infobars and C++ infobars. We are trying to
get rid of the former and have all infobars have a C++ equivalent.

As part of the cleanup we are making it very explicit what is the logic for java
only infobars and C++ infobars (so we can transition the former in a second
step)  https://codereview.chromium.org/26263002/ is another step in that
direction. There I add a RemoveSelf after this so the infobar can be dismissed
properly.

[Peter Kasting, 2013-10-07 20:19:21]

https://chromiumcodereview.appspot.com/22694006/diff/439001/chrome/browser/ui...
File chrome/browser/ui/android/infobars/confirm_infobar.cc (right):

https://chromiumcodereview.appspot.com/22694006/diff/439001/chrome/browser/ui...
chrome/browser/ui/android/infobars/confirm_infobar.cc:56: // buttons become
unclickable once the infobar is unowned.
On 2013/10/07 15:19:00, Miguel Garcia wrote:
> 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?

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.
 
> 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.

How?  That would imply the owner can get changed on a different thread than this
code is executing on.  That shouldn't happen.

https://chromiumcodereview.appspot.com/22694006/diff/439001/chrome/browser/ui...
File chrome/browser/ui/android/infobars/infobar_android.cc (right):

https://chromiumcodereview.appspot.com/22694006/diff/439001/chrome/browser/ui...
chrome/browser/ui/android/infobars/infobar_android.cc:66: CloseInfoBar();  //
TODO(pkasting): Is this correct?
On 2013/10/07 15:19:00, Miguel Garcia wrote:
> This method is being removed in https://codereview.chromium.org/26176007/

Sounds like I should just wait for that to go in and then take another look
here.

[Peter Kasting, 2013-10-31 01:15:31]

Miguel, you want to take another look at the Android portion of this change now
that more of the Android upstreaming has happened?

[sky, 2013-12-03 01:45:50]

I looked at chrome/browser/infobars, those LGTM. I also looked at a couple of
the other files and as those are mechanical rubber stamp LGTM for them.

[Peter Kasting, 2013-12-03 03:16:37]

Committed patchset #30 manually as r238283.

[Philippe, 2013-12-03 14:29:23]

On 2013/12/03 03:16:37, Peter Kasting wrote:
> Committed patchset #30 manually as r238283.

FYI, I hit the crash below on Android when using Telemetry with Chromium test
shell. Reverting the CL locally fixes the issue. I believe the crash (happening
while dereferencing address 0) might be due to web_contents() returning NULL.

 00245ae8  AutoLoginInfoBarDelegate                                             
                                                                                
                                                                                
                                                                                
                                            
/mnt/ssd/pliard/chromium/src/out/Release/../../chrome/browser/ui/auto_login_infobar_delegate.cc:148
          0023c797  AutoLoginInfoBarDelegateAndroid                             
                                                                                
                                                                                
                                                                                
                                                     
/mnt/ssd/pliard/chromium/src/out/Release/../../chrome/browser/ui/android/infobars/auto_login_infobar_delegate_android.cc:27
          0024573f  AutoLoginInfoBarDelegate::Create(InfoBarService*,
AutoLoginInfoBarDelegate::Params const&)                                        
                                                                                
                                                                                
                                                                
/mnt/ssd/pliard/chromium/src/out/Release/../../chrome/browser/ui/auto_login_infobar_delegate.cc:134
          v------>  AutoLoginPrompter::AddInfoBarToWebContents()                
                                                                                
                                                                                
                                                                                
                                                     
/mnt/ssd/pliard/chromium/src/out/Release/../../chrome/browser/ui/auto_login_prompter.cc:148
          00245f85  AutoLoginPrompter::AddInfoBarToWebContents()                
                                                                                
                                                                                
                                                                                
                                                     
/mnt/ssd/pliard/chromium/src/out/Release/../../chrome/browser/ui/auto_login_prompter.cc:140
          00245f95  AutoLoginPrompter::DidStopLoading(content::RenderViewHost*) 
                                                                                
                                                                                
                                                                                
                                                     
/mnt/ssd/pliard/chromium/src/out/Release/../../chrome/browser/ui/auto_login_prompter.cc:131
          01562b9d 
content::WebContentsImpl::SetIsLoading(content::RenderViewHost*, bool,
content::LoadNotificationDetails*)                                              
                                                                                
                                                                                
                                           
/mnt/ssd/pliard/chromium/src/out/Release/../../content/browser/web_contents/web_contents_impl.cc:2543
          0155f3c1 
content::WebContentsImpl::DidStopLoading(content::RenderViewHost*)              
                                                                                
                                                                                
                                                                                
                                 
/mnt/ssd/pliard/chromium/src/out/Release/../../content/browser/web_contents/web_contents_impl.cc:3071
          01533da5  content::RenderViewHostImpl::OnDidStopLoading()             
                                                                                
                                                                                
                                                                                
                                                     
/mnt/ssd/pliard/chromium/src/out/Release/../../content/browser/renderer_host/render_view_host_impl.cc:1586
          0041f4d7  bool IPC::Message::Dispatch<TtsMessageFilter,
TtsMessageFilter>(IPC::Message const*, TtsMessageFilter*, TtsMessageFilter*,
void (TtsMessageFilter::*)())                                                   
                                                                                
                                                                        
/mnt/ssd/pliard/chromium/src/out/Release/../../ipc/ipc_message.h:147
          0153add1  content::RenderViewHostImpl::OnMessageReceived(IPC::Message
const&)                                                                         
                                                                                
                                                                                
                                                      
/mnt/ssd/pliard/chromium/src/out/Release/../../content/browser/renderer_host/render_view_host_impl.cc:1262
          01531701 
content::RenderProcessHostImpl::OnMessageReceived(IPC::Message const&)          
                                                                                
                                                                                
                                                                                
                                 
/mnt/ssd/pliard/chromium/src/out/Release/../../content/browser/renderer_host/render_process_host_impl.cc:1361
          00862abb  IPC::ChannelProxy::Context::OnDispatchMessage(IPC::Message
const&)                                                                         
                                                                                
                                                                                
                                                       
/mnt/ssd/pliard/chromium/src/out/Release/../../ipc/ipc_channel_proxy.cc:260