Eliminate the LinkInfoBar[Delegate] classes entirely.

Eliminate the LinkInfoBar[Delegate] classes entirely.

And switch to the existing AlternateNavInfoBar[Delegate] classes.

BUG=164772
R=pkasting@chromium.org
TBR=ben@chromium.org
NOTRY=True # ios_dbg_simulator seems to be broken in lkgm?

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

[tfarina, 2012-12-29 15:13:35]

https://codereview.chromium.org/11721003/diff/4023/chrome/browser/api/infobar...
File chrome/browser/api/infobars/infobar_delegate.cc (right):

https://codereview.chromium.org/11721003/diff/4023/chrome/browser/api/infobar...
chrome/browser/api/infobars/infobar_delegate.cc:75: MediaStreamInfoBarDelegate*
InfoBarDelegate::AsMediaStreamInfoBarDelegate() {
This seems to be a bug in rietveld. In my local file I just have one
MediaStreamInfoBarDelegate definition.

[Peter Kasting, 2013-01-02 19:32:52]

We should get rid of everything that we used to do for LinkInfobars that we
didn't do for other specific non-Confirm infobar delegates.

So in other words, if Mac code has custom controllers and mock objects and tests
for link infobars, but it didn't have them for alternate nav infobars, or
autofill cc infobars, or insecure content infobars, or whatever, then just nuke
all those controllers/tests/etc.  Similarly, I'd like to kill the downcast
conversion function if possible.

We want to make the alternate nav infobar feel like a peer of the other one-off
infobars, rather than feeling like a peer of ConfirmInfoBarDelegate.

https://codereview.chromium.org/11721003/diff/9027/chrome/browser/alternate_n...
File chrome/browser/alternate_nav_infobar_delegate.h (right):

https://codereview.chromium.org/11721003/diff/9027/chrome/browser/alternate_n...
chrome/browser/alternate_nav_infobar_delegate.h:1: // Copyright 2012 The
Chromium Authors. All rights reserved.
These should live in chrome/browser/infobars.

[tfarina, 2013-01-02 21:01:24]

On 2013/01/02 19:32:52, Peter Kasting wrote:
> We should get rid of everything that we used to do for LinkInfobars that we
> didn't do for other specific non-Confirm infobar delegates.
> 
Done.

> So in other words, if Mac code has custom controllers and mock objects and
tests
> for link infobars, but it didn't have them for alternate nav infobars, or
> autofill cc infobars, or insecure content infobars, or whatever, then just
nuke
> all those controllers/tests/etc.

Done.

>  Similarly, I'd like to kill the downcast
> conversion function if possible.

Can I kill the downcast in a follow up CL?

Please, take another look.

https://codereview.chromium.org/11721003/diff/9027/chrome/browser/alternate_n...
File chrome/browser/alternate_nav_infobar_delegate.h (right):

https://codereview.chromium.org/11721003/diff/9027/chrome/browser/alternate_n...
chrome/browser/alternate_nav_infobar_delegate.h:1: // Copyright 2012 The
Chromium Authors. All rights reserved.
On 2013/01/02 19:32:52, Peter Kasting wrote:
> These should live in chrome/browser/infobars.

Done.

[tfarina, 2013-01-03 18:41:47]

Peter, friendly ping??

[Peter Kasting, 2013-01-03 18:43:22]

On 2013/01/03 18:41:47, tfarina wrote:
> Peter, friendly ping??

I don't do code reviews every day anymore (so I can get anything else done at
all).  Have patience, I'll try to get to you on Friday.

[Peter Kasting, 2013-01-04 01:09:07]

LGTM

https://codereview.chromium.org/11721003/diff/22001/chrome/browser/api/infoba...
File chrome/browser/api/infobars/infobar_delegate.cc (right):

https://codereview.chromium.org/11721003/diff/22001/chrome/browser/api/infoba...
chrome/browser/api/infobars/infobar_delegate.cc:75: MediaStreamInfoBarDelegate*
InfoBarDelegate::AsMediaStreamInfoBarDelegate() {
This code is duplicated.

https://codereview.chromium.org/11721003/diff/22001/chrome/browser/automation...
File chrome/browser/automation/testing_automation_provider.cc (right):

https://codereview.chromium.org/11721003/diff/22001/chrome/browser/automation...
chrome/browser/automation/testing_automation_provider.cc:2166: } else if
(infobar->AsAlternateNavInfoBarDelegate()) {
Who actually cares about the values this is setting?

It doesn't seem like any test is going to use them, so can't we just kill this
block entirely?

https://codereview.chromium.org/11721003/diff/22001/chrome/browser/infobars/a...
File chrome/browser/infobars/alternate_nav_infobar_delegate.h (right):

https://codereview.chromium.org/11721003/diff/22001/chrome/browser/infobars/a...
chrome/browser/infobars/alternate_nav_infobar_delegate.h:5: #ifndef
CHROME_BROWSER_INFOBARS_ALTERNATE_NAV_INFOBAR_DELEGATE_H_
I'm torn on this, but I think this delegate should probably live in
chrome/browser/ instead of chrome/browser/infobars/, since the pattern with all
other delegates is that they all live in the same directory as the code that
uses them.

https://codereview.chromium.org/11721003/diff/22001/chrome/browser/infobars/a...
chrome/browser/infobars/alternate_nav_infobar_delegate.h:20: 
Nit: Extra blank line (2 places)

https://codereview.chromium.org/11721003/diff/22001/chrome/browser/infobars/a...
chrome/browser/infobars/alternate_nav_infobar_delegate.h:28: virtual
AlternateNavInfoBarDelegate* AsAlternateNavInfoBarDelegate() OVERRIDE;
Nit: Belongs below the next two functions

https://codereview.chromium.org/11721003/diff/22001/chrome/browser/ui/cocoa/i...
File chrome/browser/ui/cocoa/infobars/infobar_controller.mm (right):

https://codereview.chromium.org/11721003/diff/22001/chrome/browser/ui/cocoa/i...
chrome/browser/ui/cocoa/infobars/infobar_controller.mm:292: @implementation
AlternateNavInfoBarController
Nit: Shouldn't all this sort of thing live in a separate
alternate_nav_info_bar_controller.* file, like we do for e.g. extension or
translate infobars?

(Frankly, the confirm one probably should too...)

[commit-bot: I haz the power, 2013-01-05 11:47:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tfarina@chromium.org/11721003/26001

[commit-bot: I haz the power, 2013-01-05 12:21:26]

Retried try job too often on ios_dbg_simulator for step(s) sync_unit_tests,
unit_tests

[commit-bot: I haz the power, 2013-01-05 17:07:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tfarina@chromium.org/11721003/26001

[commit-bot: I haz the power, 2013-01-05 17:40:57]

Retried try job too often on ios_dbg_simulator for step(s) sync_unit_tests,
unit_tests

[commit-bot: I haz the power, 2013-01-05 17:44:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tfarina@chromium.org/11721003/26001

[commit-bot: I haz the power, 2013-01-05 18:16:27]

Retried try job too often on ios_dbg_simulator for step(s) sync_unit_tests,
unit_tests

[commit-bot: I haz the power, 2013-01-05 19:54:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tfarina@chromium.org/11721003/26001

[commit-bot: I haz the power, 2013-01-05 20:22:49]

Retried try job too often on ios_dbg_simulator for step(s) sync_unit_tests,
unit_tests

[commit-bot: I haz the power, 2013-01-05 21:39:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tfarina@chromium.org/11721003/26001

[commit-bot: I haz the power, 2013-01-05 22:07:50]

Retried try job too often on ios_dbg_simulator for step(s) sync_unit_tests,
unit_tests

[commit-bot: I haz the power, 2013-01-06 11:24:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tfarina@chromium.org/11721003/26001

[commit-bot: I haz the power, 2013-01-06 11:56:03]

Retried try job too often on ios_dbg_simulator for step(s) sync_unit_tests,
unit_tests

[commit-bot: I haz the power, 2013-01-06 22:32:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tfarina@chromium.org/11721003/26001

[commit-bot: I haz the power, 2013-01-06 23:04:18]

Retried try job too often on ios_dbg_simulator for step(s) sync_unit_tests,
unit_tests

[commit-bot: I haz the power, 2013-01-06 23:05:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tfarina@chromium.org/11721003/26001

[commit-bot: I haz the power, 2013-01-06 23:36:23]

Retried try job too often on ios_dbg_simulator for step(s) sync_unit_tests,
unit_tests

[tfarina, 2013-01-07 15:31:32]

https://codereview.chromium.org/11721003/diff/22001/chrome/browser/ui/cocoa/i...
File chrome/browser/ui/cocoa/infobars/infobar_controller.mm (right):

https://codereview.chromium.org/11721003/diff/22001/chrome/browser/ui/cocoa/i...
chrome/browser/ui/cocoa/infobars/infobar_controller.mm:292: @implementation
AlternateNavInfoBarController
On 2013/01/04 01:09:07, Peter Kasting wrote:
> Nit: Shouldn't all this sort of thing live in a separate
> alternate_nav_info_bar_controller.* file, like we do for e.g. extension or
> translate infobars?
> 
> (Frankly, the confirm one probably should too...)

Hi Peter, I started a new patch to address this.

https://codereview.chromium.org/11773025

[tfarina, 2013-01-08 21:43:00]

https://codereview.chromium.org/11721003/diff/22001/chrome/browser/infobars/a...
File chrome/browser/infobars/alternate_nav_infobar_delegate.h (right):

https://codereview.chromium.org/11721003/diff/22001/chrome/browser/infobars/a...
chrome/browser/infobars/alternate_nav_infobar_delegate.h:5: #ifndef
CHROME_BROWSER_INFOBARS_ALTERNATE_NAV_INFOBAR_DELEGATE_H_
On 2013/01/04 01:09:07, Peter Kasting wrote:
> I'm torn on this, but I think this delegate should probably live in
> chrome/browser/ instead of chrome/browser/infobars/, since the pattern with
all
> other delegates is that they all live in the same directory as the code that
> uses them.
I've opened a new CL to address this here:

https://codereview.chromium.org/11825005/

[tfarina, 2013-01-19 00:22:53]

On Wed, Jan 2, 2013 at 5:32 PM,  <pkasting@chromium.org> wrote:
> Similarly, I'd like to kill the downcast conversion function if possible.
>
The patch in https://codereview.chromium.org/12017031/ should address
this hopefuly.

> https://codereview.chromium.org/11721003/

-- 
Thiago