Remove a variety of no-longer-used query params.

Remove a variety of no-longer-used query params.

BUG=571674
TEST=none
TBR=palmer,sdefresne

Committed: https://crrev.com/83dc1e9ac7419fc73e2dca2545d6f9cd9d8a420f
Cr-Commit-Position: refs/heads/master@{#367809}

[Peter Kasting, 2015-12-23 08:18:27]

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

[Peter Kasting, 2015-12-23 08:18:28]

I think this removes the related bits of the embeddedSearch API entirely.  I
assume that's OK.

[samarth, 2015-12-28 20:12:22]

Thanks for doing the cleanup!

FYI, see also https://code.google.com/p/chromium/issues/detail?id=552720 for a
list of Embedded Search APIs we can nuke.

That's partially why I suggested splitting off the
ShowingSearchTermsOnSearchResultsPages change and leaving this focused on the
query params.  If we start pulling the thread here to its fullest extent, this
change would balloon to be much much larger.  On the flip side, it feels odd to
just remove that one function here while leaving all the other dead code around.
 What do you think?

Thanks,
Samarth

https://codereview.chromium.org/1543203002/diff/1/chrome/browser/search_engin...
File chrome/browser/search_engines/ui_thread_search_terms_data.cc (left):

https://codereview.chromium.org/1543203002/diff/1/chrome/browser/search_engin...
chrome/browser/search_engines/ui_thread_search_terms_data.cc:129: bool
UIThreadSearchTermsData::IsShowingSearchTermsOnSearchResultsPages() const {
Unless this is directly related to the query param cleanup, I'd have a slight
preference for splitting this cleanup into a separate change to make the changes
easier to reason about.

https://codereview.chromium.org/1543203002/diff/1/chrome/browser/ui/cocoa/loc...
File chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm (right):

https://codereview.chromium.org/1543203002/diff/1/chrome/browser/ui/cocoa/loc...
chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm:25: #include
"chrome/browser/search/instant_service.h"
I think you can remove these includes now.

https://codereview.chromium.org/1543203002/diff/1/chrome/browser/ui/views/loc...
File chrome/browser/ui/views/location_bar/location_bar_view.cc (right):

https://codereview.chromium.org/1543203002/diff/1/chrome/browser/ui/views/loc...
chrome/browser/ui/views/location_bar/location_bar_view.cc:24: #include
"chrome/browser/search/instant_service.h"
You can kill these includes.

https://codereview.chromium.org/1543203002/diff/1/chrome/test/data/diagnostic...
File chrome/test/data/diagnostics/user/Default/Preferences (left):

https://codereview.chromium.org/1543203002/diff/1/chrome/test/data/diagnostic...
chrome/test/data/diagnostics/user/Default/Preferences:19:
"default_search_provider": {
Unrelated to this change, but should this also have an "ntp_url" field?

[Peter Kasting, 2016-01-04 21:47:11]

On 2015/12/28 20:12:22, samarth wrote:
> FYI, see also https://code.google.com/p/chromium/issues/detail?id=552720 for a
> list of Embedded Search APIs we can nuke.

Once this lands, can you update that bug with the precise list of what's still
remaining that's definitively dead?  Right now, it's not totally clear to me
which things on that bug are dead and aren't, and it also seems like it overlaps
with the bug I'm fixing here.

> That's partially why I suggested splitting off the
> ShowingSearchTermsOnSearchResultsPages change and leaving this focused on the
> query params.  If we start pulling the thread here to its fullest extent, this
> change would balloon to be much much larger.  On the flip side, it feels odd
to
> just remove that one function here while leaving all the other dead code
around.
>  What do you think?

See my inline reply to that specific comment.  This change is only about
removing code that was actually used to implement the params being removed here;
I'm not deleting more or less code than that.

https://codereview.chromium.org/1543203002/diff/1/chrome/browser/search_engin...
File chrome/browser/search_engines/ui_thread_search_terms_data.cc (left):

https://codereview.chromium.org/1543203002/diff/1/chrome/browser/search_engin...
chrome/browser/search_engines/ui_thread_search_terms_data.cc:129: bool
UIThreadSearchTermsData::IsShowingSearchTermsOnSearchResultsPages() const {
On 2015/12/28 20:12:21, samarth wrote:
> Unless this is directly related to the query param cleanup, I'd have a slight
> preference for splitting this cleanup into a separate change to make the
changes
> easier to reason about.

It's directly related; the only caller of it is the code to do the query param
expansion, that I'm removing.

https://codereview.chromium.org/1543203002/diff/1/chrome/browser/ui/cocoa/loc...
File chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm (right):

https://codereview.chromium.org/1543203002/diff/1/chrome/browser/ui/cocoa/loc...
chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm:25: #include
"chrome/browser/search/instant_service.h"
On 2015/12/28 20:12:22, samarth wrote:
> I think you can remove these includes now.

Done.

https://codereview.chromium.org/1543203002/diff/1/chrome/browser/ui/views/loc...
File chrome/browser/ui/views/location_bar/location_bar_view.cc (right):

https://codereview.chromium.org/1543203002/diff/1/chrome/browser/ui/views/loc...
chrome/browser/ui/views/location_bar/location_bar_view.cc:24: #include
"chrome/browser/search/instant_service.h"
On 2015/12/28 20:12:22, samarth wrote:
> You can kill these includes.

Done.

https://codereview.chromium.org/1543203002/diff/1/chrome/test/data/diagnostic...
File chrome/test/data/diagnostics/user/Default/Preferences (left):

https://codereview.chromium.org/1543203002/diff/1/chrome/test/data/diagnostic...
chrome/test/data/diagnostics/user/Default/Preferences:19:
"default_search_provider": {
On 2015/12/28 20:12:22, samarth wrote:
> Unrelated to this change, but should this also have an "ntp_url" field?

If the tests pass without it, then it doesn't need one, since this is just test
data.

[Peter Kasting, 2016-01-05 21:22:59]

Friendly ping.

[samarth, 2016-01-06 05:48:40]

lgtm

On 2016/01/04 21:47:11, Peter Kasting wrote:
> On 2015/12/28 20:12:22, samarth wrote:
> > FYI, see also https://code.google.com/p/chromium/issues/detail?id=552720 for
a
> > list of Embedded Search APIs we can nuke.
> 
> Once this lands, can you update that bug with the precise list of what's still
> remaining that's definitively dead?  Right now, it's not totally clear to me
> which things on that bug are dead and aren't, and it also seems like it
overlaps
> with the bug I'm fixing here.

Sure, can do.

[Peter Kasting, 2016-01-06 09:09:12]

The CQ bit was checked by pkasting@chromium.org

[commit-bot: I haz the power, 2016-01-06 09:10:09]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1543203002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1543203002/20001

[commit-bot: I haz the power, 2016-01-06 09:19:27]

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

[commit-bot: I haz the power, 2016-01-06 09:19:28]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Peter Kasting, 2016-01-06 09:23:59]

Description was changed from

==========
Remove a variety of no-longer-used query params.

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

to

==========
Remove a variety of no-longer-used query params.

BUG=571674
TEST=none
TBR=palmer,sdefresne
==========

[Peter Kasting, 2016-01-06 09:23:59]

pkasting@chromium.org changed reviewers:
+ palmer@chromium.org, sdefresne@chromium.org

[Peter Kasting, 2016-01-06 09:24:51]

TBR=palmer for removing an IPC message in chrome/common/render_messages.h
TBR=sdefresne for removing various code in ios/chrome/browser/

[Peter Kasting, 2016-01-06 09:24:57]

The CQ bit was checked by pkasting@chromium.org

[commit-bot: I haz the power, 2016-01-06 09:25:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1543203002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1543203002/20001

[commit-bot: I haz the power, 2016-01-06 10:19:57]

Description was changed from

==========
Remove a variety of no-longer-used query params.

BUG=571674
TEST=none
TBR=palmer,sdefresne
==========

to

==========
Remove a variety of no-longer-used query params.

BUG=571674
TEST=none
TBR=palmer,sdefresne
==========

[commit-bot: I haz the power, 2016-01-06 10:19:58]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-01-06 10:21:00]

Description was changed from

==========
Remove a variety of no-longer-used query params.

BUG=571674
TEST=none
TBR=palmer,sdefresne
==========

to

==========
Remove a variety of no-longer-used query params.

BUG=571674
TEST=none
TBR=palmer,sdefresne

Committed: https://crrev.com/83dc1e9ac7419fc73e2dca2545d6f9cd9d8a420f
Cr-Commit-Position: refs/heads/master@{#367809}
==========

[commit-bot: I haz the power, 2016-01-06 10:21:00]

Patchset 2 (id:??) landed as
https://crrev.com/83dc1e9ac7419fc73e2dca2545d6f9cd9d8a420f
Cr-Commit-Position: refs/heads/master@{#367809}

[palmer, 2016-01-07 18:53:54]

IPC rubber stamp LGTM.