Remove SearchTabHelperDelegate::OnWebContentsInstantSupportDisabled

Remove SearchTabHelperDelegate::OnWebContentsInstantSupportDisabled

This was used to update the Toolbar state, but the Toolbar doesn't
depend on Instant anymore (and hasn't for a long time).

BUG=627747
Review-Url: https://codereview.chromium.org/2736243002
Cr-Commit-Position: refs/heads/master@{#455717}
Committed: https://chromium.googlesource.com/chromium/src/+/e35f51a67fb4f774e154ed0e89e0549ab3fd5b1b

[Marc Treib, 2017-03-08 12:59:34]

The CQ bit was checked by treib@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-08 12:59:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Marc Treib, 2017-03-08 13:34:57]

treib@chromium.org changed reviewers:
+ pkasting@chromium.org

[Marc Treib, 2017-03-08 13:34:58]

PTAL!

https://codereview.chromium.org/2736243002/diff/1/chrome/browser/ui/browser.cc
File chrome/browser/ui/browser.cc (right):

https://codereview.chromium.org/2736243002/diff/1/chrome/browser/ui/browser.c...
chrome/browser/ui/browser.cc:1972: OmniboxView* Browser::GetOmniboxView() {
This is now the only method left in SearchTabHelperDelegate. I think we can
remove the delegate, and instead just pass the OmniboxView* to the
SearchTabHelper directly, e.g. in Browser::TabInsertedAt. (That assumes that the
OmniboxView* won't change during the lifetime of the tab, which I think is
safe.)

If you agree, I'll do that in a follow-up (or in this CL, whichever you prefer).

[commit-bot: I haz the power, 2017-03-08 13:43:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-08 13:43:32]

Dry run: This issue passed the CQ dry run.

[Peter Kasting, 2017-03-09 06:12:24]

LGTM

https://codereview.chromium.org/2736243002/diff/1/chrome/browser/ui/browser.cc
File chrome/browser/ui/browser.cc (right):

https://codereview.chromium.org/2736243002/diff/1/chrome/browser/ui/browser.c...
chrome/browser/ui/browser.cc:1972: OmniboxView* Browser::GetOmniboxView() {
On 2017/03/08 13:34:58, Marc Treib wrote:
> This is now the only method left in SearchTabHelperDelegate. I think we can
> remove the delegate, and instead just pass the OmniboxView* to the
> SearchTabHelper directly, e.g. in Browser::TabInsertedAt. (That assumes that
the
> OmniboxView* won't change during the lifetime of the tab, which I think is
> safe.)

I'd worry about the case of dragging a tab from one window to another.  An
OmniboxView is tied to a browser window, so if you dragged a tab from one window
to a different one, you'd want to make sure you weren't still pointing at the
omnibox from the previous window.

If in that case things get updated correctly, then you're welcome to make the
change in a follow-up CL.

[Marc Treib, 2017-03-09 10:04:47]

https://codereview.chromium.org/2736243002/diff/1/chrome/browser/ui/browser.cc
File chrome/browser/ui/browser.cc (right):

https://codereview.chromium.org/2736243002/diff/1/chrome/browser/ui/browser.c...
chrome/browser/ui/browser.cc:1972: OmniboxView* Browser::GetOmniboxView() {
On 2017/03/09 06:12:24, Peter Kasting wrote:
> On 2017/03/08 13:34:58, Marc Treib wrote:
> > This is now the only method left in SearchTabHelperDelegate. I think we can
> > remove the delegate, and instead just pass the OmniboxView* to the
> > SearchTabHelper directly, e.g. in Browser::TabInsertedAt. (That assumes that
> the
> > OmniboxView* won't change during the lifetime of the tab, which I think is
> > safe.)
> 
> I'd worry about the case of dragging a tab from one window to another.  An
> OmniboxView is tied to a browser window, so if you dragged a tab from one
window
> to a different one, you'd want to make sure you weren't still pointing at the
> omnibox from the previous window.
> 
> If in that case things get updated correctly, then you're welcome to make the
> change in a follow-up CL.

Thanks, I didn't think about that case! I'll investigate and then possibly send
that follow-up.

[Marc Treib, 2017-03-09 10:04:51]

The CQ bit was checked by treib@chromium.org

[commit-bot: I haz the power, 2017-03-09 10:05:10]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-09 11:24:27]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1489053891733360, "parent_rev":
"a715c8f907438fbe778b2d54372108023f2ee973", "commit_rev":
"e35f51a67fb4f774e154ed0e89e0549ab3fd5b1b"}

[commit-bot: I haz the power, 2017-03-09 11:25:08]

Description was changed from

==========
Remove SearchTabHelperDelegate::OnWebContentsInstantSupportDisabled

This was used to update the Toolbar state, but the Toolbar doesn't
depend on Instant anymore (and hasn't for a long time).

BUG=627747
==========

to

==========
Remove SearchTabHelperDelegate::OnWebContentsInstantSupportDisabled

This was used to update the Toolbar state, but the Toolbar doesn't
depend on Instant anymore (and hasn't for a long time).

BUG=627747

Review-Url: https://codereview.chromium.org/2736243002
Cr-Commit-Position: refs/heads/master@{#455717}
Committed:
https://chromium.googlesource.com/chromium/src/+/e35f51a67fb4f774e154ed0e89e0...
==========

[commit-bot: I haz the power, 2017-03-09 11:25:11]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/e35f51a67fb4f774e154ed0e89e0...