Ensure that a browser action is never passed to a function which expects a page action.

Ensure that a browser action is never passed to a function which expects a page action.

(Changed review name, because the solution has changed)

BUG=53041
TEST=On a mac, install a browser action and a page action, right click on each icon, select "Inspect Popup".

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=57048

[Sam Kerner (Chrome), 2010-08-19 23:55:40]

Scott,
    I think a NOTREACHED() that you added should be reachable.  Let me know if
it really shouldn't be.

Sam

[Scott Hess - ex-Googler, 2010-08-20 00:00:23]

LGTM.

I think you're right, but I was faithfully replicating older code, warts and all
:-).  Looping in Andy in case he wants to contest my point, though I would say
you don't need to wait for his response if this is causing problems.

[Bons, 2010-08-20 16:24:20]

http://codereview.chromium.org/3176021/diff/1/2
File chrome/browser/cocoa/location_bar/location_bar_view_mac.mm (right):

http://codereview.chromium.org/3176021/diff/1/2#newcode290
chrome/browser/cocoa/location_bar/location_bar_view_mac.mm:290: // We will get
here if |page_action| is actually a browser action.
Could you put in the comment how that could happen?

[Sam Kerner (Chrome), 2010-08-23 03:15:04]

After looking at all callers to the function I was changing, I think it is
simpler to not pass a browser action to a function that is expecting a page
action.  This only required updating one function.  Please re-review.

Sam

http://codereview.chromium.org/3176021/diff/1/2
File chrome/browser/cocoa/location_bar/location_bar_view_mac.mm (right):

http://codereview.chromium.org/3176021/diff/1/2#newcode290
chrome/browser/cocoa/location_bar/location_bar_view_mac.mm:290: // We will get
here if |page_action| is actually a browser action.
On 2010/08/20 16:24:20, Andrew Bonventre (Bons) wrote:
> Could you put in the comment how that could happen?

Done.

[Bons, 2010-08-23 03:38:24]

LGTM