browser: Add missing OVERRIDE annotation to ExternalTabContainer implementing TabContentsDelegate.

browser: Add missing OVERRIDE annotation to ExternalTabContainer implementing TabContentsDelegate.

BUG=100276
R=thestig@chromium.org

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

[Lei Zhang, 2011-11-14 20:51:32]

http://codereview.chromium.org/8551002/diff/3001/chrome/browser/external_tab_...
File chrome/browser/external_tab_container_win.h (right):

http://codereview.chromium.org/8551002/diff/3001/chrome/browser/external_tab_...
chrome/browser/external_tab_container_win.h:192: bool show_history);
Doesn't this need an OVERRIDE, since it's overriding
TabContentsDelegate::ShowPageInfo()? Shouldn't it stay up near line ~150?

[tfarina, 2011-11-15 02:30:45]

On 2011/11/14 20:51:32, Lei Zhang wrote:
>
http://codereview.chromium.org/8551002/diff/3001/chrome/browser/external_tab_...
> File chrome/browser/external_tab_container_win.h (right):
> 
>
http://codereview.chromium.org/8551002/diff/3001/chrome/browser/external_tab_...
> chrome/browser/external_tab_container_win.h:192: bool show_history);
> Doesn't this need an OVERRIDE, since it's overriding
> TabContentsDelegate::ShowPageInfo()? Shouldn't it stay up near line ~150?

According to this log
http://build.chromium.org/p/tryserver.chromium/builders/win/builds/2791/steps...,
no.

[Lei Zhang, 2011-11-15 02:38:22]

On 2011/11/15 02:30:45, tfarina wrote:
> On 2011/11/14 20:51:32, Lei Zhang wrote:
> >
>
http://codereview.chromium.org/8551002/diff/3001/chrome/browser/external_tab_...
> > File chrome/browser/external_tab_container_win.h (right):
> > 
> >
>
http://codereview.chromium.org/8551002/diff/3001/chrome/browser/external_tab_...
> > chrome/browser/external_tab_container_win.h:192: bool show_history);
> > Doesn't this need an OVERRIDE, since it's overriding
> > TabContentsDelegate::ShowPageInfo()? Shouldn't it stay up near line ~150?
> 
> According to this log
>
http://build.chromium.org/p/tryserver.chromium/builders/win/builds/2791/steps...,
> no.

Ah, the signature is that of BrowserWindow::ShowPageInfo, which
ExternalTabContainer does not implement. Can you check to see if it's dead code?
If it's not overriding anything, then who is calling it?

[tfarina, 2011-11-15 02:46:10]

On 2011/11/15 02:38:22, Lei Zhang wrote:
> Ah, the signature is that of BrowserWindow::ShowPageInfo, which
> ExternalTabContainer does not implement. Can you check to see if it's dead
code?
> If it's not overriding anything, then who is calling it?

It seems no one is calling it. The calls I see are:

source_tab_contents_->ShowPageInfo(nav_entry->url(), nav_entry->ssl(),
source_tab_contents_->ShowPageInfo(params_.frame_url, ssl,
window()->ShowPageInfo(profile, url, ssl, show_history);
tab->ShowPageInfo(nav_entry->url(), nav_entry->ssl(), true);
tab->ShowPageInfo(nav_entry->url(), nav_entry->ssl(), true);
tab->ShowPageInfo(nav_entry->url(), nav_entry->ssl(), true);
delegate_->ShowPageInfo(browser_context(), url, ssl, show_history);

[Lei Zhang, 2011-11-15 03:45:50]

On 2011/11/15 02:46:10, tfarina wrote:
> On 2011/11/15 02:38:22, Lei Zhang wrote:
> > Ah, the signature is that of BrowserWindow::ShowPageInfo, which
> > ExternalTabContainer does not implement. Can you check to see if it's dead
> code?
> > If it's not overriding anything, then who is calling it?
> 
> It seems no one is calling it. The calls I see are:
> 
> source_tab_contents_->ShowPageInfo(nav_entry->url(), nav_entry->ssl(),
> source_tab_contents_->ShowPageInfo(params_.frame_url, ssl,
> window()->ShowPageInfo(profile, url, ssl, show_history);
> tab->ShowPageInfo(nav_entry->url(), nav_entry->ssl(), true);
> tab->ShowPageInfo(nav_entry->url(), nav_entry->ssl(), true);
> tab->ShowPageInfo(nav_entry->url(), nav_entry->ssl(), true);
> delegate_->ShowPageInfo(browser_context(), url, ssl, show_history);

In no one is calling it, then it's dead code and you can probably just delete
it.

[tfarina, 2011-11-15 15:19:08]

On 2011/11/15 03:45:50, Lei Zhang wrote:
> In no one is calling it, then it's dead code and you can probably just delete
> it.

Done. PTAL!

[Lei Zhang, 2011-11-15 18:27:32]

LGTM

ExternalTabPageInfoBubbleView looks like dead code too at thi s point. Feel free
to remove it here or in a separate CL.