Fix some issues with OmniboxViewViews' use of OmniboxClient.

Fix some issues with OmniboxViewViews' use of OmniboxClient.

Instead of creating this in the constructor, have callers pass it in.  Also, use
this client to get the scheme classifier instead of constructing a new one each
time it's needed.  Not only is this mode efficient, it means we can now avoid
passing in the Profile at all.

BUG=none
TEST=none

Review-Url: https://codereview.chromium.org/2731123002
Cr-Commit-Position: refs/heads/master@{#455437}
Committed: https://chromium.googlesource.com/chromium/src/+/b8632833b06502d99b8607b48a5ca048b8e72842

[Peter Kasting, 2017-03-04 03:17:56]

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

[commit-bot: I haz the power, 2017-03-04 03:18:04]

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-04 03:31:06]

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

[commit-bot: I haz the power, 2017-03-04 03:31:07]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Peter Kasting, 2017-03-04 04:58:43]

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

[commit-bot: I haz the power, 2017-03-04 04:58:46]

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

[Peter Kasting, 2017-03-04 05:22:26]

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

[commit-bot: I haz the power, 2017-03-04 05:37:36]

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

[commit-bot: I haz the power, 2017-03-04 05:37:37]

Dry run: This issue passed the CQ dry run.

[Peter Kasting, 2017-03-06 20:17:38]

Description was changed from

==========
Fix some issues with OmniboxViewViews' use of OmniboxClient.

Instead of creating this in the constructor, have callers pass it in.  This will
allow the unittest to replace it with a different implementation.

Also, use this client to get the scheme classifier instead of constructing a new
one each time it's needed.  Not only is this mode efficient, it means we can now
avoid passing in the Profile at all.

BUG=none
TEST=none
==========

to

==========
Fix some issues with OmniboxViewViews' use of OmniboxClient.

Instead of creating this in the constructor, have callers pass it in.  Also, use
this client to get the scheme classifier instead of constructing a new one each
time it's needed.  Not only is this mode efficient, it means we can now avoid
passing in the Profile at all.

BUG=none
TEST=none
==========

[Peter Kasting, 2017-03-06 20:18:50]

This isn't strictly necessary for the followon patch anymore, now that I found a
way to do what I wanted without injecting a different client.

Nevertheless, I think this route is still better and more testable, so rather
than withdrawing the patch, I simply updated the CL comment.

[Peter Kasting, 2017-03-06 21:13:30]

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

[commit-bot: I haz the power, 2017-03-06 21:14:11]

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-06 23:03:58]

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

[commit-bot: I haz the power, 2017-03-06 23:04:00]

Dry run: This issue passed the CQ dry run.

[Mark P, 2017-03-08 05:43:38]

lgtm

Three cheers for managing to stop passing around Profiles!

--mark

https://codereview.chromium.org/2731123002/diff/60001/chrome/browser/ui/views...
File chrome/browser/ui/views/omnibox/omnibox_view_views.cc (right):

https://codereview.chromium.org/2731123002/diff/60001/chrome/browser/ui/views...
chrome/browser/ui/views/omnibox/omnibox_view_views.cc:25: #include
"components/omnibox/browser/omnibox_client.h"
might not be necessary depending on how you resolve the .h comment

https://codereview.chromium.org/2731123002/diff/60001/chrome/browser/ui/views...
File chrome/browser/ui/views/omnibox/omnibox_view_views.h (right):

https://codereview.chromium.org/2731123002/diff/60001/chrome/browser/ui/views...
chrome/browser/ui/views/omnibox/omnibox_view_views.h:58:
std::unique_ptr<OmniboxClient> client,
nit: please add appropriate include or forward declaration for omnibox_client

[Peter Kasting, 2017-03-08 11:11:32]

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

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

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

[Peter Kasting, 2017-03-08 11:44:57]

https://codereview.chromium.org/2731123002/diff/60001/chrome/browser/ui/views...
File chrome/browser/ui/views/omnibox/omnibox_view_views.h (right):

https://codereview.chromium.org/2731123002/diff/60001/chrome/browser/ui/views...
chrome/browser/ui/views/omnibox/omnibox_view_views.h:58:
std::unique_ptr<OmniboxClient> client,
On 2017/03/08 05:43:38, Mark P wrote:
> nit: please add appropriate include or forward declaration for omnibox_client

Done, though in the absence of formal IWYU tooling I'm always leery about adding
such things when the file compiles fine without them.

[Peter Kasting, 2017-03-08 11:45:01]

The CQ bit was checked by pkasting@chromium.org

[Peter Kasting, 2017-03-08 11:45:01]

The patchset sent to the CQ was uploaded after l-g-t-m from
mpearson@chromium.org
Link to the patchset: https://codereview.chromium.org/2731123002/#ps100001
(title: "Review comments")

[commit-bot: I haz the power, 2017-03-08 11:45:14]

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-08 12:50:20]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1488973501492510,
"parent_rev": "67b145d3666d209bf534e0161eaccd4c0a7deccd", "commit_rev":
"b8632833b06502d99b8607b48a5ca048b8e72842"}

[commit-bot: I haz the power, 2017-03-08 12:51:00]

Description was changed from

==========
Fix some issues with OmniboxViewViews' use of OmniboxClient.

Instead of creating this in the constructor, have callers pass it in.  Also, use
this client to get the scheme classifier instead of constructing a new one each
time it's needed.  Not only is this mode efficient, it means we can now avoid
passing in the Profile at all.

BUG=none
TEST=none
==========

to

==========
Fix some issues with OmniboxViewViews' use of OmniboxClient.

Instead of creating this in the constructor, have callers pass it in.  Also, use
this client to get the scheme classifier instead of constructing a new one each
time it's needed.  Not only is this mode efficient, it means we can now avoid
passing in the Profile at all.

BUG=none
TEST=none

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

[commit-bot: I haz the power, 2017-03-08 12:51:02]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/b8632833b06502d99b8607b48a5c...