New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 628207 link

Starred by 2 users
Status: Fixed
Owner:
treib@chromium.org
Closed: Nov 2016
Cc:
█ kmadhusu@chromium.org
█ sfiera@chromium.org
█ jered@chromium.org
█ ramyan@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocking:
issue 627747



Sign in to add a comment

Cleanup up dead/unused parts of the searchbox/embeddedsearch API

Project Member Reported by treib@chromium.org, Jul 14 2016 
Some parts of the searchbox api have been never fully implemented and/or abandoned.

http://dev.chromium.org/embeddedsearch

https://cs.chromium.org/chromium/src/chrome/renderer/resources/extensions/searchbox_api.js
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 25 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9f7f6f163a4f1660ea8b078e76dd2261eaee224c

commit 9f7f6f163a4f1660ea8b078e76dd2261eaee224c
Author: treib <treib@chromium.org>
Date: Mon Jul 25 17:13:44 2016

Remove embeddedSearch.newTabPage.GetAppLauncherEnabled
and all the related IPC code

It's not used anywhere, and it's not documented at http://dev.chromium.org/embeddedsearch

Also remove searchBox.setRestrictedValue

BUG= 628207 

Review-Url: https://codereview.chromium.org/2151483004
Cr-Commit-Position: refs/heads/master@{#407499}

[modify] https://crrev.com/9f7f6f163a4f1660ea8b078e76dd2261eaee224c/chrome/browser/ui/search/search_ipc_router.cc
[modify] https://crrev.com/9f7f6f163a4f1660ea8b078e76dd2261eaee224c/chrome/browser/ui/search/search_ipc_router.h
[modify] https://crrev.com/9f7f6f163a4f1660ea8b078e76dd2261eaee224c/chrome/browser/ui/search/search_ipc_router_policy_impl.cc
[modify] https://crrev.com/9f7f6f163a4f1660ea8b078e76dd2261eaee224c/chrome/browser/ui/search/search_ipc_router_policy_impl.h
[modify] https://crrev.com/9f7f6f163a4f1660ea8b078e76dd2261eaee224c/chrome/browser/ui/search/search_ipc_router_policy_unittest.cc
[modify] https://crrev.com/9f7f6f163a4f1660ea8b078e76dd2261eaee224c/chrome/browser/ui/search/search_ipc_router_unittest.cc
[modify] https://crrev.com/9f7f6f163a4f1660ea8b078e76dd2261eaee224c/chrome/browser/ui/search/search_tab_helper.cc
[modify] https://crrev.com/9f7f6f163a4f1660ea8b078e76dd2261eaee224c/chrome/browser/ui/search/search_tab_helper.h
[modify] https://crrev.com/9f7f6f163a4f1660ea8b078e76dd2261eaee224c/chrome/common/render_messages.h
[modify] https://crrev.com/9f7f6f163a4f1660ea8b078e76dd2261eaee224c/chrome/renderer/resources/extensions/searchbox_api.js
[modify] https://crrev.com/9f7f6f163a4f1660ea8b078e76dd2261eaee224c/chrome/renderer/searchbox/searchbox.cc
[modify] https://crrev.com/9f7f6f163a4f1660ea8b078e76dd2261eaee224c/chrome/renderer/searchbox/searchbox.h
[modify] https://crrev.com/9f7f6f163a4f1660ea8b078e76dd2261eaee224c/chrome/renderer/searchbox/searchbox_extension.cc
[modify] https://crrev.com/9f7f6f163a4f1660ea8b078e76dd2261eaee224c/testing/buildbot/filters/browser-side-navigation.linux.unit_tests.filter
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 26 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3ae19e4141a44e52e3b5877611b8be686a16a1a6

commit 3ae19e4141a44e52e3b5877611b8be686a16a1a6
Author: treib <treib@chromium.org>
Date: Tue Jul 26 17:57:29 2016

Remove embeddedSearch.searchBox.displayInstantResults
and all the related IPC code

It's not used anywhere, and it's not documented at http://dev.chromium.org/embeddedsearch

BUG= 628207 

Review-Url: https://codereview.chromium.org/2149973002
Cr-Commit-Position: refs/heads/master@{#407849}

[modify] https://crrev.com/3ae19e4141a44e52e3b5877611b8be686a16a1a6/chrome/browser/ui/search/search_ipc_router.cc
[modify] https://crrev.com/3ae19e4141a44e52e3b5877611b8be686a16a1a6/chrome/browser/ui/search/search_ipc_router.h
[modify] https://crrev.com/3ae19e4141a44e52e3b5877611b8be686a16a1a6/chrome/browser/ui/search/search_ipc_router_policy_impl.cc
[modify] https://crrev.com/3ae19e4141a44e52e3b5877611b8be686a16a1a6/chrome/browser/ui/search/search_ipc_router_policy_impl.h
[modify] https://crrev.com/3ae19e4141a44e52e3b5877611b8be686a16a1a6/chrome/browser/ui/search/search_ipc_router_policy_unittest.cc
[modify] https://crrev.com/3ae19e4141a44e52e3b5877611b8be686a16a1a6/chrome/browser/ui/search/search_ipc_router_unittest.cc
[modify] https://crrev.com/3ae19e4141a44e52e3b5877611b8be686a16a1a6/chrome/browser/ui/search/search_tab_helper.cc
[modify] https://crrev.com/3ae19e4141a44e52e3b5877611b8be686a16a1a6/chrome/browser/ui/search/search_tab_helper.h
[modify] https://crrev.com/3ae19e4141a44e52e3b5877611b8be686a16a1a6/chrome/common/render_messages.h
[modify] https://crrev.com/3ae19e4141a44e52e3b5877611b8be686a16a1a6/chrome/renderer/resources/extensions/searchbox_api.js
[modify] https://crrev.com/3ae19e4141a44e52e3b5877611b8be686a16a1a6/chrome/renderer/searchbox/searchbox.cc
[modify] https://crrev.com/3ae19e4141a44e52e3b5877611b8be686a16a1a6/chrome/renderer/searchbox/searchbox.h
[modify] https://crrev.com/3ae19e4141a44e52e3b5877611b8be686a16a1a6/chrome/renderer/searchbox/searchbox_extension.cc
[modify] https://crrev.com/3ae19e4141a44e52e3b5877611b8be686a16a1a6/testing/buildbot/filters/browser-side-navigation.linux.unit_tests.filter

Comment 3 by fi...@chromium.org, Jul 29 2016

Labels: zine-ntp-pe zine-triaged

Comment 5 by treib@chromium.org, Oct 12 2016 

I think all dead APIs have now been removed. Some remaining small things:
- getMostVisitedItemData contains some logic for the icon NTP, which never launched. This can probably be removed along with the rest of the icon NTP.
- getMostVisitedItemData lives in embeddedSearch.searchBox - seems to me it should really be in embeddedSearch.newTabPage instead. Since it's only usable from the Most Visited iframe anyway, it should be fairly simple to move it.
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 24 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8d3b5a0a5d710c5bc3fe0132e7d301016a4cf858

commit 8d3b5a0a5d710c5bc3fe0132e7d301016a4cf858
Author: treib <treib@chromium.org>
Date: Thu Nov 24 21:53:31 2016

Searchbox API cleanup: Move getMostVisitedItemData to newTabPage

Prior to this CL, getMostVisitedItemData was defined in chrome.embeddedSearch.searchBox. This CL moves it to chrome.embeddedSearch.newTabPage where it belongs.

Tested: Both Google (single-iframe) and third-party (multi-iframe) NTPs still work.

BUG= 628207 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2531443003
Cr-Commit-Position: refs/heads/master@{#434397}

[modify] https://crrev.com/8d3b5a0a5d710c5bc3fe0132e7d301016a4cf858/chrome/browser/resources/local_ntp/most_visited_single.js
[modify] https://crrev.com/8d3b5a0a5d710c5bc3fe0132e7d301016a4cf858/chrome/browser/resources/local_ntp/most_visited_util.js
[modify] https://crrev.com/8d3b5a0a5d710c5bc3fe0132e7d301016a4cf858/chrome/renderer/resources/extensions/searchbox_api.js

Comment 7 by treib@chromium.org, Nov 28 2016

Status: Fixed (was: Started)
The only thing still missing here is the icon NTP code. That's tracked separately in  bug 669073 , so I'm closing this one.
►

Sign in to add a comment