CompactLocationBar

CompactLocationBar 1st step:
 * Added OnMouseXXXTab methods to BrowserExtender to handle on tabs.
 * Changed tab to call above methods.
 * Added GetSelectedTab to TabStrip

Minor changes along with the major changes above
* Removed unnecessary file entries in chrome.gyp
* Fixed a few method's const.
* Removed unnecessary class declarations and includes.

Know issue: keyboard focus is not working. 
In 2nd step, I'm going to eliminate popup and use whatever that FindBar is using to show compact location bar. I'll fix the issue above after this migration. (if it still persists)

BUG=None
Test=None

[brettw, 2009-10-28 16:36:57]

http://codereview.chromium.org/341008/diff/1031/1038
File chrome/browser/views/tabs/tab.cc (right):

http://codereview.chromium.org/341008/diff/1031/1038#newcode165
Line 165: GetBrowserExtender()->OnMouseEnteredToTab(this);
Is this call necessary? I would have thought you'd get entered before a pressed
and this would be unnecessary.

http://codereview.chromium.org/341008/diff/1031/1038#newcode262
Line 262: return
static_cast<BrowserView*>(tab_strip->GetParent())->browser_extender();
Ugh. I guess I don't have a better suggestion for this. Hopefully it won't have
to stay this way for long.

[oshima, 2009-10-28 17:17:56]

http://codereview.chromium.org/341008/diff/1031/1038
File chrome/browser/views/tabs/tab.cc (right):

http://codereview.chromium.org/341008/diff/1031/1038#newcode165
Line 165: GetBrowserExtender()->OnMouseEnteredToTab(this);
On 2009/10/28 16:36:57, brettw wrote:
> Is this call necessary? I would have thought you'd get entered before a
pressed
> and this would be unnecessary.

I should have say this is just a hack to show the compact navbar for the
selected tab.I updated the comment. We can add separate method to Extender to
notify that this is "selected", but I decided  to keep this way as this may (and
will) change. Let me know if you prefer to have separate method.

http://codereview.chromium.org/341008/diff/1031/1038#newcode262
Line 262: return
static_cast<BrowserView*>(tab_strip->GetParent())->browser_extender();
On 2009/10/28 16:36:57, brettw wrote:
> Ugh. I guess I don't have a better suggestion for this. Hopefully it won't
have
> to stay this way for long.

Ah, yes. I forgot to mention this. I agree that this is ugly, and I was
wondering if there is a better way. One alternative is to use View ID (and
probably cache it), or pass reference around to this. Or may be we should add
method to return browser view/extender to delegate?  What do you think?

[brettw, 2009-10-28 17:33:49]

LGTM

http://codereview.chromium.org/341008/diff/1031/1038
File chrome/browser/views/tabs/tab.cc (right):

http://codereview.chromium.org/341008/diff/1031/1038#newcode262
Line 262: return
static_cast<BrowserView*>(tab_strip->GetParent())->browser_extender();
On 2009/10/28 17:17:56, oshima wrote:
> On 2009/10/28 16:36:57, brettw wrote:
> > Ugh. I guess I don't have a better suggestion for this. Hopefully it won't
> have
> > to stay this way for long.
> 
> Ah, yes. I forgot to mention this. I agree that this is ugly, and I was
> wondering if there is a better way. One alternative is to use View ID (and
> probably cache it), or pass reference around to this. Or may be we should add
> method to return browser view/extender to delegate?  What do you think?

I'm OK with this for now. How about adding what you just said (options for
alternate implementations) to the comment inside the function. Then it's more
clear how to fix it if we decide to keep the compact location bar.

[oshima, 2009-10-28 17:56:53]

I just uploaded patch but it's got a few changes from latest trunk when I run
"git rebase". I'll run trybots first and commit.

http://codereview.chromium.org/341008/diff/1031/1038
File chrome/browser/views/tabs/tab.cc (right):

http://codereview.chromium.org/341008/diff/1031/1038#newcode262
Line 262: return
static_cast<BrowserView*>(tab_strip->GetParent())->browser_extender();
On 2009/10/28 17:33:50, brettw wrote:
> On 2009/10/28 17:17:56, oshima wrote:
> > On 2009/10/28 16:36:57, brettw wrote:
> > > Ugh. I guess I don't have a better suggestion for this. Hopefully it won't
> > have
> > > to stay this way for long.
> > 
> > Ah, yes. I forgot to mention this. I agree that this is ugly, and I was
> > wondering if there is a better way. One alternative is to use View ID (and
> > probably cache it), or pass reference around to this. Or may be we should
add
> > method to return browser view/extender to delegate?  What do you think?
> 
> I'm OK with this for now. How about adding what you just said (options for
> alternate implementations) to the comment inside the function. Then it's more
> clear how to fix it if we decide to keep the compact location bar.

Good idea. Done.