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

Issue 736038 link

Starred by 2 users

Issue metadata

Status: Archived
Owner:
Last visit > 30 days ago
Closed: Jul 2017
Cc:
Components:
EstimatedDays: 1
NextAction: ----
OS: Chrome
Pri: 1
Type: Feature



Sign in to add a comment

Replace google icon with magnifying glass when search provider is not google.

Project Member Reported by newcomer@chromium.org, Jun 22 2017

Issue description

Use the magnifying glass icon in assets.


 

Comment 1 by zalcorn@google.com, Jun 23 2017

Status: Assigned (was: Untriaged)
Labels: NewLauncherUIV1
Status: Available (was: Assigned)
Owner: maajid@chromium.org
Status: Assigned (was: Available)

Comment 5 by maajid@chromium.org, Jun 29 2017

Status: Started (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 13 2017

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

commit 866502711aed99c079b5871d733ec005865f4264
Author: Maajid <maajid@chromium.org>
Date: Thu Jul 13 01:52:39 2017

Use the magnifying glass icon instead of the Google icon for the search box when the default search
provider is not Google.

Bug:736038
TEST:unit tested, device tested with --enable-features=EnableFullcreenAppList

Change-Id: Ib305fbbdfea06b7f21fad2aa717af91e0f20aec0
Reviewed-on: https://chromium-review.googlesource.com/557505
Commit-Queue: Maajid <maajid@chromium.org>
Reviewed-by: Yury Khmel <khmel@chromium.org>
Reviewed-by: Qiang(Joe) Xu <warx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486215}
[modify] https://crrev.com/866502711aed99c079b5871d733ec005865f4264/ui/app_list/BUILD.gn
[modify] https://crrev.com/866502711aed99c079b5871d733ec005865f4264/ui/app_list/app_list_constants.cc
[modify] https://crrev.com/866502711aed99c079b5871d733ec005865f4264/ui/app_list/app_list_constants.h
[modify] https://crrev.com/866502711aed99c079b5871d733ec005865f4264/ui/app_list/test/app_list_test_view_delegate.cc
[modify] https://crrev.com/866502711aed99c079b5871d733ec005865f4264/ui/app_list/test/app_list_test_view_delegate.h
[modify] https://crrev.com/866502711aed99c079b5871d733ec005865f4264/ui/app_list/vector_icons/BUILD.gn
[add] https://crrev.com/866502711aed99c079b5871d733ec005865f4264/ui/app_list/vector_icons/ic_search_engine_not_google.1x.icon
[add] https://crrev.com/866502711aed99c079b5871d733ec005865f4264/ui/app_list/vector_icons/ic_search_engine_not_google.icon
[modify] https://crrev.com/866502711aed99c079b5871d733ec005865f4264/ui/app_list/views/search_box_view.cc
[modify] https://crrev.com/866502711aed99c079b5871d733ec005865f4264/ui/app_list/views/search_box_view.h
[modify] https://crrev.com/866502711aed99c079b5871d733ec005865f4264/ui/app_list/views/search_box_view_unittest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Jul 13 2017

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

commit cb12b2ae7a6e109daa0d141406424dcd69964373
Author: Friedrich Horschig <fhorschig@chromium.org>
Date: Thu Jul 13 07:38:29 2017

Revert "Use the magnifying glass icon instead of the Google icon for the search box when the default search"

This reverts commit 866502711aed99c079b5871d733ec005865f4264.

Reason for revert: The search_box_view_unittest.cc broke ASan for your last two tests. You apparently leak the images, maybe you just forgot to clear "SearchBoxView* view_"?
Can you use unique_pointer for your test raw pointer members?
Link to the first break:
https://build.chromium.org/p/chromium.memory/builders/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/builds/22297


Original change's description:
> Use the magnifying glass icon instead of the Google icon for the search box when the default search
> provider is not Google.
> 
> Bug:736038
> TEST:unit tested, device tested with --enable-features=EnableFullcreenAppList
> 
> Change-Id: Ib305fbbdfea06b7f21fad2aa717af91e0f20aec0
> Reviewed-on: https://chromium-review.googlesource.com/557505
> Commit-Queue: Maajid <maajid@chromium.org>
> Reviewed-by: Yury Khmel <khmel@chromium.org>
> Reviewed-by: Qiang(Joe) Xu <warx@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#486215}

TBR=khmel@chromium.org,warx@chromium.org,newcomer@chromium.org,maajid@chromium.org

Change-Id: Ic2f9d9d0291bd676a01a23ef904d575b11300d51
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  736038 
Reviewed-on: https://chromium-review.googlesource.com/569739
Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
Commit-Queue: Friedrich Horschig <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486305}
[modify] https://crrev.com/cb12b2ae7a6e109daa0d141406424dcd69964373/ui/app_list/BUILD.gn
[modify] https://crrev.com/cb12b2ae7a6e109daa0d141406424dcd69964373/ui/app_list/app_list_constants.cc
[modify] https://crrev.com/cb12b2ae7a6e109daa0d141406424dcd69964373/ui/app_list/app_list_constants.h
[modify] https://crrev.com/cb12b2ae7a6e109daa0d141406424dcd69964373/ui/app_list/test/app_list_test_view_delegate.cc
[modify] https://crrev.com/cb12b2ae7a6e109daa0d141406424dcd69964373/ui/app_list/test/app_list_test_view_delegate.h
[modify] https://crrev.com/cb12b2ae7a6e109daa0d141406424dcd69964373/ui/app_list/vector_icons/BUILD.gn
[delete] https://crrev.com/6048b896826a1cf55096995d89ef4c72ae72ddd9/ui/app_list/vector_icons/ic_search_engine_not_google.1x.icon
[delete] https://crrev.com/6048b896826a1cf55096995d89ef4c72ae72ddd9/ui/app_list/vector_icons/ic_search_engine_not_google.icon
[modify] https://crrev.com/cb12b2ae7a6e109daa0d141406424dcd69964373/ui/app_list/views/search_box_view.cc
[modify] https://crrev.com/cb12b2ae7a6e109daa0d141406424dcd69964373/ui/app_list/views/search_box_view.h
[modify] https://crrev.com/cb12b2ae7a6e109daa0d141406424dcd69964373/ui/app_list/views/search_box_view_unittest.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Jul 14 2017

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

commit 455a5fcddd64b29d81b626cafca1200a0c8e0473
Author: Maajid <maajid@chromium.org>
Date: Fri Jul 14 22:00:07 2017

Reland: Use the magnifying glass icon instead of the Google icon for the search box when the default search provider is not Google.

Bug:736038
TEST:unit tested, device tested with --enable-features=EnableFullcreenAppList, ASAN and LSAN local test.

The fix for the ASAN failure was to use unique_ptr for the search_box_view pointer in the unit test.

Change-Id: Id6026f6193a87f48d44064315ddde5e436d7d333
Reviewed-on: https://chromium-review.googlesource.com/571310
Commit-Queue: Maajid <maajid@chromium.org>
Reviewed-by: Yury Khmel <khmel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486901}
[modify] https://crrev.com/455a5fcddd64b29d81b626cafca1200a0c8e0473/ui/app_list/BUILD.gn
[modify] https://crrev.com/455a5fcddd64b29d81b626cafca1200a0c8e0473/ui/app_list/app_list_constants.cc
[modify] https://crrev.com/455a5fcddd64b29d81b626cafca1200a0c8e0473/ui/app_list/app_list_constants.h
[modify] https://crrev.com/455a5fcddd64b29d81b626cafca1200a0c8e0473/ui/app_list/test/app_list_test_view_delegate.cc
[modify] https://crrev.com/455a5fcddd64b29d81b626cafca1200a0c8e0473/ui/app_list/test/app_list_test_view_delegate.h
[modify] https://crrev.com/455a5fcddd64b29d81b626cafca1200a0c8e0473/ui/app_list/vector_icons/BUILD.gn
[add] https://crrev.com/455a5fcddd64b29d81b626cafca1200a0c8e0473/ui/app_list/vector_icons/ic_search_engine_not_google.1x.icon
[add] https://crrev.com/455a5fcddd64b29d81b626cafca1200a0c8e0473/ui/app_list/vector_icons/ic_search_engine_not_google.icon
[modify] https://crrev.com/455a5fcddd64b29d81b626cafca1200a0c8e0473/ui/app_list/views/search_box_view.cc
[modify] https://crrev.com/455a5fcddd64b29d81b626cafca1200a0c8e0473/ui/app_list/views/search_box_view.h
[modify] https://crrev.com/455a5fcddd64b29d81b626cafca1200a0c8e0473/ui/app_list/views/search_box_view_unittest.cc

Comment 9 by maajid@chromium.org, Jul 17 2017

Status: Fixed (was: Started)

Comment 10 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment