Remove AutocompleteInput Type and PageClassification.

Remove AutocompleteInput Type and PageClassification.

They are just as same as the enums defined in .proto files. Removing
the declaration of these types reduces unnecessary code dependency
to autocomplete.

BUG=384232
R=blundell@chromium.org, mpearson@chromium.org, pkasting@chromium.org
TBR=brettw@chromium.org, cpu@chromium.org, miguelg@chromium.org, mpcomplete@chromium.org
TEST=compile succeeds

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=278722

[Mark P, 2014-06-17 23:11:29]

https://codereview.chromium.org/343523003/diff/20001/chrome/browser/autocompl...
File chrome/browser/autocomplete/autocomplete_input.h (left):

https://codereview.chromium.org/343523003/diff/20001/chrome/browser/autocompl...
chrome/browser/autocomplete/autocomplete_input.h:20: typedef
metrics::OmniboxInputType::Type Type;
pkasting explicitly asked me to add this alias when I removed
AutocompleteInput::Type from this class to make things more readable.  Please
ask him before you remove it.

[Peter Kasting, 2014-06-17 23:56:14]

https://codereview.chromium.org/343523003/diff/20001/chrome/browser/autocompl...
File chrome/browser/autocomplete/autocomplete_input.h (left):

https://codereview.chromium.org/343523003/diff/20001/chrome/browser/autocompl...
chrome/browser/autocomplete/autocomplete_input.h:20: typedef
metrics::OmniboxInputType::Type Type;
On 2014/06/17 23:11:29, Mark P wrote:
> pkasting explicitly asked me to add this alias when I removed
> AutocompleteInput::Type from this class to make things more readable.  Please
> ask him before you remove it.

I'm OK with removing this.

[Jun Mukai, 2014-06-18 00:38:21]

https://codereview.chromium.org/343523003/diff/20001/chrome/browser/autocompl...
File chrome/browser/autocomplete/autocomplete_input.h (left):

https://codereview.chromium.org/343523003/diff/20001/chrome/browser/autocompl...
chrome/browser/autocomplete/autocomplete_input.h:20: typedef
metrics::OmniboxInputType::Type Type;
On 2014/06/17 23:56:14, Peter Kasting wrote:
> On 2014/06/17 23:11:29, Mark P wrote:
> > pkasting explicitly asked me to add this alias when I removed
> > AutocompleteInput::Type from this class to make things more readable. 
Please
> > ask him before you remove it.
> 
> I'm OK with removing this.

Cool!
so... is this CL looking good?

[blundell, 2014-06-18 16:21:38]

LGTM

Did you check IWYU everywhere? I'm a little surprised by how few includes are
added.

https://codereview.chromium.org/343523003/diff/20001/chrome/browser/android/o...
File chrome/browser/android/omnibox/autocomplete_controller_android.h (right):

https://codereview.chromium.org/343523003/diff/20001/chrome/browser/android/o...
chrome/browser/android/omnibox/autocomplete_controller_android.h:114:
metrics::OmniboxEventProto::PageClassification ClassifyPage(
nit: Does this not need a new include?

[Jun Mukai, 2014-06-18 18:01:38]

https://codereview.chromium.org/343523003/diff/20001/chrome/browser/android/o...
File chrome/browser/android/omnibox/autocomplete_controller_android.h (right):

https://codereview.chromium.org/343523003/diff/20001/chrome/browser/android/o...
chrome/browser/android/omnibox/autocomplete_controller_android.h:114:
metrics::OmniboxEventProto::PageClassification ClassifyPage(
On 2014/06/18 16:21:38, blundell wrote:
> nit: Does this not need a new include?

including autocomplete_input.h will include the proto implicitly. Did you mean
to add the proto include as well?
# note that I cannot remove autocomplete_input.h in this file so far because it
uses the class below, line 139.

[blundell, 2014-06-18 20:08:50]

On 2014/06/18 18:01:38, Jun Mukai wrote:
>
https://codereview.chromium.org/343523003/diff/20001/chrome/browser/android/o...
> File chrome/browser/android/omnibox/autocomplete_controller_android.h (right):
> 
>
https://codereview.chromium.org/343523003/diff/20001/chrome/browser/android/o...
> chrome/browser/android/omnibox/autocomplete_controller_android.h:114:
> metrics::OmniboxEventProto::PageClassification ClassifyPage(
> On 2014/06/18 16:21:38, blundell wrote:
> > nit: Does this not need a new include?
> 
> including autocomplete_input.h will include the proto implicitly. Did you mean
> to add the proto include as well?
> # note that I cannot remove autocomplete_input.h in this file so far because
it
> uses the class below, line 139.

OK

[Mark P, 2014-06-18 21:51:45]

On 2014/06/18 16:21:38, blundell wrote:
> LGTM
> 
> Did you check IWYU everywhere? I'm a little surprised by how few includes are
> added.

I'm pretty sure IWYU means that any file that refers to
metrics::OmniboxEventProto
anywhere should have the corresponding #include at the top.

It doesn't matter whether you get it via indirect includes from somewhere else.

--mark

[Peter Kasting, 2014-06-18 21:55:26]

On 2014/06/18 21:51:45, Mark P wrote:
> On 2014/06/18 16:21:38, blundell wrote:
> > LGTM
> > 
> > Did you check IWYU everywhere? I'm a little surprised by how few includes
are
> > added.
> 
> I'm pretty sure IWYU means that any file that refers to
> metrics::OmniboxEventProto
> anywhere should have the corresponding #include at the top.

That's true, but so few Chromium files do IWYU, and we lack a tool to properly
enforce it, that I find it easier to standardize on "use the minimum set of
includes necessary to build the file".  This very occasionally means someone
needs to tweak a few extra files when they change the #includes in a different
.h, but for the most part there are no side effects, and it's easier to describe
and less likely to lead to "orphaned" #includes that stick around past the time
when they're actually necessary.

[Mark P, 2014-06-18 22:06:01]

On 2014/06/18 21:55:26, Peter Kasting wrote:
> On 2014/06/18 21:51:45, Mark P wrote:
> > On 2014/06/18 16:21:38, blundell wrote:
> > > LGTM
> > > 
> > > Did you check IWYU everywhere? I'm a little surprised by how few includes
> are
> > > added.
> > 
> > I'm pretty sure IWYU means that any file that refers to
> > metrics::OmniboxEventProto
> > anywhere should have the corresponding #include at the top.
> 
> That's true, but so few Chromium files do IWYU, and we lack a tool to properly
> enforce it, that I find it easier to standardize on "use the minimum set of
> includes necessary to build the file".  This very occasionally means someone
> needs to tweak a few extra files when they change the #includes in a different
> .h, but for the most part there are no side effects, and it's easier to
describe
> and less likely to lead to "orphaned" #includes that stick around past the
time
> when they're actually necessary.

It feels like the chromium community leans toward IWYU judging by this thread I
found on the topic:
https://groups.google.com/a/chromium.org/d/topic/chromium-dev/i_5qAOxe0xY/dis...
My favorite quote: "Ideally we'd have include-what-you-use that would be
tool-assisted so it would be easy to do and enforce.  In the interim time to
that, intentionally not including what you use is absolutely wrong."  Who is
this by?  It's courtesy of Peter Kasting.

:-)

--mark

[Peter Kasting, 2014-06-19 00:12:21]

On 2014/06/18 22:06:01, Mark P wrote:
> On 2014/06/18 21:55:26, Peter Kasting wrote:
> > On 2014/06/18 21:51:45, Mark P wrote:
> > > On 2014/06/18 16:21:38, blundell wrote:
> > > > LGTM
> > > > 
> > > > Did you check IWYU everywhere? I'm a little surprised by how few
includes
> > are
> > > > added.
> > > 
> > > I'm pretty sure IWYU means that any file that refers to
> > > metrics::OmniboxEventProto
> > > anywhere should have the corresponding #include at the top.
> > 
> > That's true, but so few Chromium files do IWYU, and we lack a tool to
properly
> > enforce it, that I find it easier to standardize on "use the minimum set of
> > includes necessary to build the file".  This very occasionally means someone
> > needs to tweak a few extra files when they change the #includes in a
different
> > .h, but for the most part there are no side effects, and it's easier to
> describe
> > and less likely to lead to "orphaned" #includes that stick around past the
> time
> > when they're actually necessary.
> 
> It feels like the chromium community leans toward IWYU judging by this thread
I
> found on the topic:
>
https://groups.google.com/a/chromium.org/d/topic/chromium-dev/i_5qAOxe0xY/dis...
> My favorite quote: "Ideally we'd have include-what-you-use that would be
> tool-assisted so it would be easy to do and enforce.  In the interim time to
> that, intentionally not including what you use is absolutely wrong."  Who is
> this by?  It's courtesy of Peter Kasting.

A foolish consistency is... ah well, never mind.  I concede the point to the
brilliant argumentation of Mr. Kasting.

[Jun Mukai, 2014-06-19 17:49:51]

manually checked the files and added #include. PTAL

[Mark P, 2014-06-20 00:08:30]

lgtm
baring two minor comments below

(wow that touched a lot of files)

--mark

https://codereview.chromium.org/343523003/diff/40001/chrome/browser/autocompl...
File chrome/browser/autocomplete/autocomplete_result.h (right):

https://codereview.chromium.org/343523003/diff/40001/chrome/browser/autocompl...
chrome/browser/autocomplete/autocomplete_result.h:18: class AutocompleteInput;
This isn't necessary if we're including autocomplete_input directly.

https://codereview.chromium.org/343523003/diff/40001/chrome/browser/ui/omnibo...
File chrome/browser/ui/omnibox/omnibox_edit_model.cc (right):

https://codereview.chromium.org/343523003/diff/40001/chrome/browser/ui/omnibo...
chrome/browser/ui/omnibox/omnibox_edit_model.cc:60: #include
"components/url_fixer/url_fixer.h"
you forgot the metrics include here

[Jun Mukai, 2014-06-20 00:29:51]

https://codereview.chromium.org/343523003/diff/40001/chrome/browser/autocompl...
File chrome/browser/autocomplete/autocomplete_result.h (right):

https://codereview.chromium.org/343523003/diff/40001/chrome/browser/autocompl...
chrome/browser/autocomplete/autocomplete_result.h:18: class AutocompleteInput;
On 2014/06/20 00:08:29, Mark P wrote:
> This isn't necessary if we're including autocomplete_input directly.

Done.

https://codereview.chromium.org/343523003/diff/40001/chrome/browser/ui/omnibo...
File chrome/browser/ui/omnibox/omnibox_edit_model.cc (right):

https://codereview.chromium.org/343523003/diff/40001/chrome/browser/ui/omnibo...
chrome/browser/ui/omnibox/omnibox_edit_model.cc:60: #include
"components/url_fixer/url_fixer.h"
On 2014/06/20 00:08:29, Mark P wrote:
> you forgot the metrics include here

Done.

[Jun Mukai, 2014-06-20 00:33:15]

Submit with TBR, following files are trivial include/using fixes:
miguelg: chrome/browser/android
mpcomplete: chrome/browser/extensions/api/omnibox
brettw: chrome/browser/renderer_context_menu
cpu: chrome/browser/rlz

[Jun Mukai, 2014-06-20 00:33:23]

The CQ bit was checked by mukai@chromium.org

[commit-bot: I haz the power, 2014-06-20 00:38:57]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mukai@chromium.org/343523003/60001

[Jun Mukai, 2014-06-20 01:42:09]

The CQ bit was checked by mukai@chromium.org

[commit-bot: I haz the power, 2014-06-20 01:43:24]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mukai@chromium.org/343523003/80001

[commit-bot: I haz the power, 2014-06-20 06:56:58]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)

[commit-bot: I haz the power, 2014-06-20 07:01:50]

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

[commit-bot: I haz the power, 2014-06-20 07:01:52]

Try jobs failed on following builders:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[Jun Mukai, 2014-06-20 07:12:18]

The CQ bit was checked by mukai@chromium.org

[commit-bot: I haz the power, 2014-06-20 07:15:38]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mukai@chromium.org/343523003/80001

[commit-bot: I haz the power, 2014-06-20 10:24:57]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)

[commit-bot: I haz the power, 2014-06-20 10:30:22]

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

[commit-bot: I haz the power, 2014-06-20 10:30:23]

Try jobs failed on following builders:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[Jun Mukai, 2014-06-20 15:15:36]

The CQ bit was checked by mukai@chromium.org

[commit-bot: I haz the power, 2014-06-20 15:16:24]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mukai@chromium.org/343523003/80001

[commit-bot: I haz the power, 2014-06-20 15:24:50]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)

[commit-bot: I haz the power, 2014-06-20 15:30:09]

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

[commit-bot: I haz the power, 2014-06-20 15:30:10]

Try jobs failed on following builders:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)

[Jun Mukai, 2014-06-20 16:56:14]

Committed patchset #5 manually as r278722 (presubmit successful).