Change InfoBar-related notifications to be sourced from a TabContentsWrapper, not a TabContents. ...

Change InfoBar-related notifications to be sourced from a TabContentsWrapper, not a TabContents.  Also change the notifications' Details to be more like what they will ultimately become once TCW owns the InfoBars, by adding a bool to the REMOVED notification for animation state, and changing the "add"-related args from InfoBarDelegate* to InfoBar*.  (I can't yet change the removed infobar delegate args similarly because TabContentsWrapper current has no way to map back from an InfoBarDelegate* to a corresponding Infobar*.)

BUG=62154
TEST=none
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=87696

[Peter Kasting, 2011-05-31 21:47:16]

rohitrao: Cocoa bits
erg: Rest

[rohitrao (ping after 24h), 2011-05-31 22:04:24]

On 2011/05/31 21:47:16, Peter Kasting wrote:
> rohitrao: Cocoa bits
> erg: Rest

The memory management seems a little wonky, because there could be multiple
observers for INFOBAR_ADDED, but one of the observers takes ownership of the
Details pointer (and, on Mac, deletes it when done).  What happens if there are
multiple observers and the InfoBarController goes first?

[Elliot Glaysher, 2011-06-01 00:50:11]

On 2011/05/31 21:47:16, Peter Kasting wrote:
> rohitrao: Cocoa bits
> erg: Rest

LGTM

[Peter Kasting, 2011-06-01 20:46:02]

On 2011/05/31 22:04:24, rohitrao wrote:
> The memory management seems a little wonky, because there could be multiple
> observers for INFOBAR_ADDED, but one of the observers takes ownership of the
> Details pointer (and, on Mac, deletes it when done).  What happens if there
are
> multiple observers and the InfoBarController goes first?

There aren't multiple observers.  Besides, the goal is to scrap all the Mac
infobar code and switch to the cross-platform implementation, at which point
there won't be this silliness of "we create an InfoBar object we don't actually
care about and then delete it immediately".  This is just transition code so I
can change how the TCW works without breaking Mac for now.

[rohitrao (ping after 24h), 2011-06-01 20:58:41]

Ok, didn't realize this was just transition code.

There are probably some cocoa infobar unittests that will need to change as
well.

http://codereview.chromium.org/7006010/diff/2004/chrome/browser/tab_contents/...
File chrome/browser/tab_contents/infobar.h (right):

http://codereview.chromium.org/7006010/diff/2004/chrome/browser/tab_contents/...
chrome/browser/tab_contents/infobar.h:124: #include
"chrome/browser/ui/cocoa/infobars/infobar.h"
You won't be able to include this file in straight-up C++ code unless you remove
all the Obj-C from chrome/browser/ui/cocoa/infobars/infobar.h.

http://codereview.chromium.org/7006010/diff/2004/chrome/browser/ui/cocoa/info...
File chrome/browser/ui/cocoa/infobars/infobar_container_controller.mm (right):

http://codereview.chromium.org/7006010/diff/2004/chrome/browser/ui/cocoa/info...
chrome/browser/ui/cocoa/infobars/infobar_container_controller.mm:136:
CreateInfoBar(currentTabContents_));
Extra ) at the end.

[Peter Kasting, 2011-06-01 22:42:45]

Rohit, if you wanted another look, this snapshot should hopefully address the
previous issues.

[rohitrao (ping after 24h), 2011-06-02 15:20:59]

LGTM

[Peter Kasting, 2011-06-02 18:33:56]

On 2011/06/01 20:46:02, Peter Kasting wrote:
> On 2011/05/31 22:04:24, rohitrao wrote:
> > The memory management seems a little wonky, because there could be multiple
> > observers for INFOBAR_ADDED, but one of the observers takes ownership of the
> > Details pointer (and, on Mac, deletes it when done).  What happens if there
> are
> > multiple observers and the InfoBarController goes first?
> 
> There aren't multiple observers.

Well, I was wrong.  geolocation_browsertest.cc is another observer here.

I've now changed the Mac code to be even more gruesome; addInfobar now takes an
arg telling it whether to delete the InfoBar synchronously or asynchronously.  I
did this so that direct unittests (as opposed to ones using notifications) don't
have to spin a message loop to avoid leaks, but can still delete immediately.

Take one more look?

[rohitrao (ping after 24h), 2011-06-02 18:58:25]

LGTM with a couple style fixes.

Agreed on the gruesomeness, but ok since it's all about to be rewritten anyways.

http://codereview.chromium.org/7006010/diff/21007/chrome/browser/ui/cocoa/inf...
File chrome/browser/ui/cocoa/infobars/infobar_container_controller.h (right):

http://codereview.chromium.org/7006010/diff/21007/chrome/browser/ui/cocoa/inf...
chrome/browser/ui/cocoa/infobars/infobar_container_controller.h:116:
deleteImmediately:(BOOL)delete_now;
thisKindOfCase for obj-c arguments.  (CamelCase?)

And since you can't get at least four leading spaces on all the lines here, we
give up on lining up the colons and just do 4 leading spaces for the other two
args:

- (void)addInfoBar:(InfoBar*)infobar
    animate:(BOOL)animate
    deleteImmediately:(BOOL)deleteNow;

http://codereview.chromium.org/7006010/diff/21007/chrome/browser/ui/cocoa/inf...
File chrome/browser/ui/cocoa/infobars/infobar_container_controller.mm (right):

http://codereview.chromium.org/7006010/diff/21007/chrome/browser/ui/cocoa/inf...
chrome/browser/ui/cocoa/infobars/infobar_container_controller.mm:194:
deleteImmediately:(BOOL)delete_now {
Same rules for indentation and variable names.

[Ben Goodger (Google), 2011-06-02 21:26:57]

Owner LGTM.