InstantExtended: remove old flags, add new flag for query extraction.

InstantExtended: remove old flags, add new flag for query extraction.

This change does three things:
  (1) Remove flags for Instant Extended. Instant Extended is now always on.
  (2) Add a separate flag to control query extraction. Also rename
      suppress_on_srp to query_extraction to make its purpose clearer.
  (3) Assorted cleanup resulting from (1) and (2).

Note that a lot more code (especially in search.cc) can be cleaned up and I
will address that in follow up changes. Attempting to keep this from
snowballing into a huge CL.

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

[samarth, 2013-11-15 03:45:55]

jered -> c/b/search
sky -> OWNERS stamp for everything else

Note that this depends on http://crrev.com/72423002 and I will wait for that to
go in first.

Thanks,
Samarth

[sky, 2013-11-15 17:15:13]

LGTM

[Jered, 2013-11-15 17:40:11]

https://codereview.chromium.org/73613002/diff/40001/chrome/browser/search/sea...
File chrome/browser/search/search.cc (left):

https://codereview.chromium.org/73613002/diff/40001/chrome/browser/search/sea...
chrome/browser/search/search.cc:176:
UMA_HISTOGRAM_ENUMERATION("InstantExtended.NewOptInState", state,
Also remove this from histograms.xml

https://codereview.chromium.org/73613002/diff/40001/chrome/browser/search/sea...
File chrome/browser/search/search.cc (right):

https://codereview.chromium.org/73613002/diff/40001/chrome/browser/search/sea...
chrome/browser/search/search.cc:74: #if !defined(OS_IOS) && !defined(OS_ANDROID)
It's probably fine to keep the string constant around on mobile.

https://codereview.chromium.org/73613002/diff/40001/chrome/browser/search/sea...
File chrome/browser/search/search.h (right):

https://codereview.chromium.org/73613002/diff/40001/chrome/browser/search/sea...
chrome/browser/search/search.h:227: // Forces query in the omnibox to be forced
on for tests.
to be forced on -> to be on

https://codereview.chromium.org/73613002/diff/40001/chrome/browser/search/sea...
File chrome/browser/search/search_unittest.cc (right):

https://codereview.chromium.org/73613002/diff/40001/chrome/browser/search/sea...
chrome/browser/search/search_unittest.cc:965:
EXPECT_TRUE(IsInstantExtendedAPIEnabled());
This won't work on ios/android.

[Jered, 2013-11-15 17:41:15]

https://codereview.chromium.org/73613002/diff/40001/chrome/browser/search/sea...
File chrome/browser/search/search.cc (left):

https://codereview.chromium.org/73613002/diff/40001/chrome/browser/search/sea...
chrome/browser/search/search.cc:176:
UMA_HISTOGRAM_ENUMERATION("InstantExtended.NewOptInState", state,
On 2013/11/15 17:40:11, Jered wrote:
> Also remove this from histograms.xml

By remove I guess I mean "mark deprecated", since stuff doesn't actually get
removed from that.

[samarth, 2013-11-18 01:45:00]

+asvitkine for tools/metrics

https://codereview.chromium.org/73613002/diff/40001/chrome/browser/search/sea...
File chrome/browser/search/search.cc (left):

https://codereview.chromium.org/73613002/diff/40001/chrome/browser/search/sea...
chrome/browser/search/search.cc:176:
UMA_HISTOGRAM_ENUMERATION("InstantExtended.NewOptInState", state,
On 2013/11/15 17:40:11, Jered wrote:
> Also remove this from histograms.xml

Done.

https://codereview.chromium.org/73613002/diff/40001/chrome/browser/search/sea...
chrome/browser/search/search.cc:176:
UMA_HISTOGRAM_ENUMERATION("InstantExtended.NewOptInState", state,
On 2013/11/15 17:41:15, Jered wrote:
> On 2013/11/15 17:40:11, Jered wrote:
> > Also remove this from histograms.xml
> 
> By remove I guess I mean "mark deprecated", since stuff doesn't actually get
> removed from that.

Done.

https://codereview.chromium.org/73613002/diff/40001/chrome/browser/search/sea...
File chrome/browser/search/search.cc (right):

https://codereview.chromium.org/73613002/diff/40001/chrome/browser/search/sea...
chrome/browser/search/search.cc:74: #if !defined(OS_IOS) && !defined(OS_ANDROID)
On 2013/11/15 17:40:11, Jered wrote:
> It's probably fine to keep the string constant around on mobile.

Compiler doesn't like the unused variable.

https://codereview.chromium.org/73613002/diff/40001/chrome/browser/search/sea...
File chrome/browser/search/search.h (right):

https://codereview.chromium.org/73613002/diff/40001/chrome/browser/search/sea...
chrome/browser/search/search.h:227: // Forces query in the omnibox to be forced
on for tests.
On 2013/11/15 17:40:11, Jered wrote:
> to be forced on -> to be on

Done.

[Alexei Svitkine (slow), 2013-11-18 14:29:59]

histograms.xml lgtm

[samarth, 2013-11-18 22:31:54]

ping jered@?

[Jered, 2013-11-18 22:35:36]

On 2013/11/18 22:31:54, samarth wrote:
> ping jered@?

Did you see my comment in PS2 about search_unittest.cc?

[Jered, 2013-11-18 22:42:40]

On 2013/11/18 22:35:36, Jered wrote:
> On 2013/11/18 22:31:54, samarth wrote:
> > ping jered@?
> 
> Did you see my comment in PS2 about search_unittest.cc?

lgtm since the test is excluded.
(We should unexclude it at some point.)

[commit-bot: I haz the power, 2013-11-19 00:23:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/samarth@chromium.org/73613002/250001

[commit-bot: I haz the power, 2013-11-19 01:47:33]

Retried try job too often on linux_aura for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

[commit-bot: I haz the power, 2013-11-19 18:46:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/samarth@chromium.org/73613002/500001

[commit-bot: I haz the power, 2013-11-19 19:24:17]

Retried try job too often on linux_rel for step(s) base_unittests,
browser_tests, interactive_ui_tests, net_unittests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2013-11-19 19:52:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/samarth@chromium.org/73613002/720001

[commit-bot: I haz the power, 2013-11-19 21:20:32]

Retried try job too often on mac_rel for step(s) app_list_unittests,
base_unittests, browser_tests, cacheinvalidation_unittests, cc_unittests,
check_deps, chromedriver2_unittests, components_unittests, content_browsertests,
content_unittests, crypto_unittests, google_apis_unittests, gpu_unittests,
interactive_ui_tests, ipc_tests, jingle_unittests, media_unittests,
message_center_unittests, nacl_integration, net_unittests, ppapi_unittests,
printing_unittests, remoting_unittests, sql_unittests, sync_integration_tests,
sync_unit_tests, telemetry_unittests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2013-11-19 21:29:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/samarth@chromium.org/73613002/950001

[commit-bot: I haz the power, 2013-11-20 03:11:43]

Change committed as 236116