SearchTabHelper: Find browser/window instead of having them passed in

SearchTabHelper: Find browser/window instead of having them passed in

This fixes a crash introduced in https://codereview.chromium.org/2739943005,
and it generally seems much less fragile: No need to rely on magic callbacks
getting called in just the right places.

BUG=627747, 702505
Review-Url: https://codereview.chromium.org/2757863002
Cr-Commit-Position: refs/heads/master@{#458366}
Committed: https://chromium.googlesource.com/chromium/src/+/f2d087b2e71a7f87b4dd71ebf424820488459ca8

[Marc Treib, 2017-03-17 11:21:59]

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

[commit-bot: I haz the power, 2017-03-17 11:22:14]

Dry run: 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-17 12:23:30]

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

[commit-bot: I haz the power, 2017-03-17 12:23:31]

Dry run: This issue passed the CQ dry run.

[Marc Treib, 2017-03-17 12:32:14]

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

[Marc Treib, 2017-03-17 12:32:15]

PTAL!

[Peter Kasting, 2017-03-17 21:16:13]

LGTM

Is it possible to write a test that would have failed with the previous code? 
I'd really like to have that if at all possible.

https://codereview.chromium.org/2757863002/diff/1/chrome/browser/ui/search/se...
File chrome/browser/ui/search/search_tab_helper.h (right):

https://codereview.chromium.org/2757863002/diff/1/chrome/browser/ui/search/se...
chrome/browser/ui/search/search_tab_helper.h:170: OmniboxView* GetOmniboxView()
const;
Nit: const methods should not return non-const pointers.

[Marc Treib, 2017-03-20 14:51:39]

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

[commit-bot: I haz the power, 2017-03-20 14:51:52]

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-20 14:52:31]

I've added a test, PTAL.
I've checked that the test crashes without the fix, but I'm afraid it's not
particularly easy to follow.

https://codereview.chromium.org/2757863002/diff/1/chrome/browser/ui/search/se...
File chrome/browser/ui/search/search_tab_helper.h (right):

https://codereview.chromium.org/2757863002/diff/1/chrome/browser/ui/search/se...
chrome/browser/ui/search/search_tab_helper.h:170: OmniboxView* GetOmniboxView()
const;
On 2017/03/17 21:16:13, Peter Kasting wrote:
> Nit: const methods should not return non-const pointers.

Done (made const and non-const overloads).

[commit-bot: I haz the power, 2017-03-20 15:32:56]

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

[commit-bot: I haz the power, 2017-03-20 15:32:57]

Dry run: This issue passed the CQ dry run.

[Peter Kasting, 2017-03-20 20:11:10]

Still LGTM

https://codereview.chromium.org/2757863002/diff/20001/chrome/browser/ui/brows...
File chrome/browser/ui/browser_browsertest.cc (right):

https://codereview.chromium.org/2757863002/diff/20001/chrome/browser/ui/brows...
chrome/browser/ui/browser_browsertest.cc:1525: GURL
ntp_url(search::GetNewTabPageURL(browser()->profile()));
Nit: Prefer = to () in this case; see
https://www.chromium.org/developers/coding-style/cpp-dos-and-donts#TOC-Variab...

https://codereview.chromium.org/2757863002/diff/20001/chrome/browser/ui/brows...
chrome/browser/ui/browser_browsertest.cc:1556: // OmniboxView* has changed, and
the old OmniboxVie has been deleted.
Nit: old OmniboxView

[Marc Treib, 2017-03-21 08:49:08]

Thanks! Nits addressed.

https://codereview.chromium.org/2757863002/diff/20001/chrome/browser/ui/brows...
File chrome/browser/ui/browser_browsertest.cc (right):

https://codereview.chromium.org/2757863002/diff/20001/chrome/browser/ui/brows...
chrome/browser/ui/browser_browsertest.cc:1525: GURL
ntp_url(search::GetNewTabPageURL(browser()->profile()));
On 2017/03/20 20:11:10, Peter Kasting wrote:
> Nit: Prefer = to () in this case; see
>
https://www.chromium.org/developers/coding-style/cpp-dos-and-donts#TOC-Variab...

Done.

https://codereview.chromium.org/2757863002/diff/20001/chrome/browser/ui/brows...
chrome/browser/ui/browser_browsertest.cc:1556: // OmniboxView* has changed, and
the old OmniboxVie has been deleted.
On 2017/03/20 20:11:10, Peter Kasting wrote:
> Nit: old OmniboxView

Done.

[Marc Treib, 2017-03-21 08:55:36]

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

[commit-bot: I haz the power, 2017-03-21 08:55:57]

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-21 09:35:52]

The CQ bit was unchecked by treib@chromium.org

[Marc Treib, 2017-03-21 09:35:52]

The CQ bit was checked by treib@chromium.org

[Marc Treib, 2017-03-21 09:35:53]

The patchset sent to the CQ was uploaded after l-g-t-m from
pkasting@chromium.org
Link to the patchset: https://codereview.chromium.org/2757863002/#ps40001
(title: "nits")

[commit-bot: I haz the power, 2017-03-21 09:36:03]

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-21 10:35:40]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490088952481110,
"parent_rev": "477e319330d3ad4eccfb32fbd5c7d28a75c6d7e3", "commit_rev":
"f2d087b2e71a7f87b4dd71ebf424820488459ca8"}

[commit-bot: I haz the power, 2017-03-21 10:36:21]

Description was changed from

==========
SearchTabHelper: Find browser/window instead of having them passed in

This fixes a crash introduced in https://codereview.chromium.org/2739943005,
and it generally seems much less fragile: No need to rely on magic callbacks
getting called in just the right places.

BUG=627747,702505
==========

to

==========
SearchTabHelper: Find browser/window instead of having them passed in

This fixes a crash introduced in https://codereview.chromium.org/2739943005,
and it generally seems much less fragile: No need to rely on magic callbacks
getting called in just the right places.

BUG=627747,702505

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

[commit-bot: I haz the power, 2017-03-21 10:36:23]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/f2d087b2e71a7f87b4dd71ebf424...