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

Issue 736552 link

Starred by 1 user

Issue metadata

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

Blocking:
issue 719523



Sign in to add a comment

Add Play Store App Search

Project Member Reported by hejq@chromium.org, Jun 24 2017

Issue description

Feature description:
Add Play Store app search to ChromeOS launcher with ARC++

Design doc: https://docs.google.com/document/d/1oyK5m15lHbNguFiz4-_7QHDejdDldOgHm2ZkH4doNMQ/edit
 

Comment 1 by hejq@chromium.org, Jun 24 2017

Blocking: 719523
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 24 2017

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

commit fa1bfdf6d5fb0952a524f4ca048ff104fdbdb2dd
Author: hejq <hejq@chromium.org>
Date: Sat Jun 24 03:13:15 2017

Add the Play Store app search to the launcher.

Bug= 736552 

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

[modify] https://crrev.com/fa1bfdf6d5fb0952a524f4ca048ff104fdbdb2dd/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/fa1bfdf6d5fb0952a524f4ca048ff104fdbdb2dd/chrome/browser/ui/app_list/app_context_menu.h
[add] https://crrev.com/fa1bfdf6d5fb0952a524f4ca048ff104fdbdb2dd/chrome/browser/ui/app_list/arc/arc_playstore_app_context_menu.cc
[add] https://crrev.com/fa1bfdf6d5fb0952a524f4ca048ff104fdbdb2dd/chrome/browser/ui/app_list/arc/arc_playstore_app_context_menu.h
[add] https://crrev.com/fa1bfdf6d5fb0952a524f4ca048ff104fdbdb2dd/chrome/browser/ui/app_list/search/arc/arc_playstore_search_provider.cc
[add] https://crrev.com/fa1bfdf6d5fb0952a524f4ca048ff104fdbdb2dd/chrome/browser/ui/app_list/search/arc/arc_playstore_search_provider.h
[add] https://crrev.com/fa1bfdf6d5fb0952a524f4ca048ff104fdbdb2dd/chrome/browser/ui/app_list/search/arc/arc_playstore_search_result.cc
[add] https://crrev.com/fa1bfdf6d5fb0952a524f4ca048ff104fdbdb2dd/chrome/browser/ui/app_list/search/arc/arc_playstore_search_result.h
[modify] https://crrev.com/fa1bfdf6d5fb0952a524f4ca048ff104fdbdb2dd/chrome/browser/ui/app_list/search/arc_app_result.cc
[modify] https://crrev.com/fa1bfdf6d5fb0952a524f4ca048ff104fdbdb2dd/chrome/browser/ui/app_list/search/search_controller_factory.cc
[modify] https://crrev.com/fa1bfdf6d5fb0952a524f4ca048ff104fdbdb2dd/components/arc/common/app.mojom
[modify] https://crrev.com/fa1bfdf6d5fb0952a524f4ca048ff104fdbdb2dd/components/arc/test/fake_app_instance.cc
[modify] https://crrev.com/fa1bfdf6d5fb0952a524f4ca048ff104fdbdb2dd/components/arc/test/fake_app_instance.h
[modify] https://crrev.com/fa1bfdf6d5fb0952a524f4ca048ff104fdbdb2dd/ui/app_list/BUILD.gn
[add] https://crrev.com/fa1bfdf6d5fb0952a524f4ca048ff104fdbdb2dd/ui/app_list/vector_icons/ic_badge_instant.1x.icon
[add] https://crrev.com/fa1bfdf6d5fb0952a524f4ca048ff104fdbdb2dd/ui/app_list/vector_icons/ic_badge_instant.icon
[add] https://crrev.com/fa1bfdf6d5fb0952a524f4ca048ff104fdbdb2dd/ui/app_list/vector_icons/ic_badge_play.1x.icon
[add] https://crrev.com/fa1bfdf6d5fb0952a524f4ca048ff104fdbdb2dd/ui/app_list/vector_icons/ic_badge_play.icon
[modify] https://crrev.com/fa1bfdf6d5fb0952a524f4ca048ff104fdbdb2dd/ui/app_list/views/search_result_tile_item_view.cc

Comment 3 by hejq@chromium.org, Jun 24 2017

Status: Fixed (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Jun 24 2017

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

commit 1fef8cf8bc356c704eb27eee979c45895e6f4d69
Author: dcheng <dcheng@chromium.org>
Date: Sat Jun 24 03:54:15 2017

Revert of Add the Play Store app search to the launcher. (patchset #17 id:300001 of https://codereview.chromium.org/2929273002/ )

Reason for revert:
Linux ChromiumOS Builder (dbg) is failing compile:

../../chrome/browser/ui/app_list/search/arc/arc_playstore_search_result.cc:0: error: undefined reference to 'app_list::kIcBadgeInstantIcon'
../../chrome/browser/ui/app_list/search/arc/arc_playstore_search_result.cc:0: error: undefined reference to 'app_list::kIcBadgePlayIcon'

Original issue's description:
> Add the Play Store app search to the launcher.
>
> Bug= 736552 
>
> Review-Url: https://codereview.chromium.org/2929273002
> Cr-Commit-Position: refs/heads/master@{#482130}
> Committed: https://chromium.googlesource.com/chromium/src/+/fa1bfdf6d5fb0952a524f4ca048ff104fdbdb2dd

TBR=lhchavez@chromium.org,xiyuan@chromium.org,hejq@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Bug= 736552 

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

[modify] https://crrev.com/1fef8cf8bc356c704eb27eee979c45895e6f4d69/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/1fef8cf8bc356c704eb27eee979c45895e6f4d69/chrome/browser/ui/app_list/app_context_menu.h
[delete] https://crrev.com/849ea0481b84658c0029c5b044d005b817ce9273/chrome/browser/ui/app_list/arc/arc_playstore_app_context_menu.cc
[delete] https://crrev.com/849ea0481b84658c0029c5b044d005b817ce9273/chrome/browser/ui/app_list/arc/arc_playstore_app_context_menu.h
[delete] https://crrev.com/849ea0481b84658c0029c5b044d005b817ce9273/chrome/browser/ui/app_list/search/arc/arc_playstore_search_provider.cc
[delete] https://crrev.com/849ea0481b84658c0029c5b044d005b817ce9273/chrome/browser/ui/app_list/search/arc/arc_playstore_search_provider.h
[delete] https://crrev.com/849ea0481b84658c0029c5b044d005b817ce9273/chrome/browser/ui/app_list/search/arc/arc_playstore_search_result.cc
[delete] https://crrev.com/849ea0481b84658c0029c5b044d005b817ce9273/chrome/browser/ui/app_list/search/arc/arc_playstore_search_result.h
[modify] https://crrev.com/1fef8cf8bc356c704eb27eee979c45895e6f4d69/chrome/browser/ui/app_list/search/arc_app_result.cc
[modify] https://crrev.com/1fef8cf8bc356c704eb27eee979c45895e6f4d69/chrome/browser/ui/app_list/search/search_controller_factory.cc
[modify] https://crrev.com/1fef8cf8bc356c704eb27eee979c45895e6f4d69/components/arc/common/app.mojom
[modify] https://crrev.com/1fef8cf8bc356c704eb27eee979c45895e6f4d69/components/arc/test/fake_app_instance.cc
[modify] https://crrev.com/1fef8cf8bc356c704eb27eee979c45895e6f4d69/components/arc/test/fake_app_instance.h
[modify] https://crrev.com/1fef8cf8bc356c704eb27eee979c45895e6f4d69/ui/app_list/BUILD.gn
[delete] https://crrev.com/849ea0481b84658c0029c5b044d005b817ce9273/ui/app_list/vector_icons/ic_badge_instant.1x.icon
[delete] https://crrev.com/849ea0481b84658c0029c5b044d005b817ce9273/ui/app_list/vector_icons/ic_badge_instant.icon
[delete] https://crrev.com/849ea0481b84658c0029c5b044d005b817ce9273/ui/app_list/vector_icons/ic_badge_play.1x.icon
[delete] https://crrev.com/849ea0481b84658c0029c5b044d005b817ce9273/ui/app_list/vector_icons/ic_badge_play.icon
[modify] https://crrev.com/1fef8cf8bc356c704eb27eee979c45895e6f4d69/ui/app_list/views/search_result_tile_item_view.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Jul 11 2017

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

commit 0f8982efb8c8db9ef3aae23443b6f8b3b732df17
Author: hejq <hejq@chromium.org>
Date: Tue Jul 11 00:11:56 2017

Use EnablePlayStoreAppSearch feature flag.

We apply EnablePlayStoreAppSearch to decide whether to:

- search for uninstalled apps from Play Store;
- show a proper badge in the Play Store app result;
- show the review score and price of every searched app.

Currently using the IsFullscreenAppListEnabled flag is incorrect.

BUG= 736552 

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

[modify] https://crrev.com/0f8982efb8c8db9ef3aae23443b6f8b3b732df17/chrome/browser/ui/app_list/search/search_controller_factory.cc
[modify] https://crrev.com/0f8982efb8c8db9ef3aae23443b6f8b3b732df17/ui/app_list/app_list_features.cc
[modify] https://crrev.com/0f8982efb8c8db9ef3aae23443b6f8b3b732df17/ui/app_list/app_list_features.h
[modify] https://crrev.com/0f8982efb8c8db9ef3aae23443b6f8b3b732df17/ui/app_list/views/search_result_tile_item_list_view.cc
[modify] https://crrev.com/0f8982efb8c8db9ef3aae23443b6f8b3b732df17/ui/app_list/views/search_result_tile_item_list_view.h
[modify] https://crrev.com/0f8982efb8c8db9ef3aae23443b6f8b3b732df17/ui/app_list/views/search_result_tile_item_view.cc
[modify] https://crrev.com/0f8982efb8c8db9ef3aae23443b6f8b3b732df17/ui/app_list/views/search_result_tile_item_view.h
[modify] https://crrev.com/0f8982efb8c8db9ef3aae23443b6f8b3b732df17/ui/app_list/views/tile_item_view.cc
[modify] https://crrev.com/0f8982efb8c8db9ef3aae23443b6f8b3b732df17/ui/app_list/views/tile_item_view.h

Project Member

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

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

commit f87a709b2218049de5a5c533986f01a4fe0913d9
Author: Pavel Kalinnikov <pkalinnikov@chromium.org>
Date: Tue Jul 25 14:31:30 2017

Revert "Add tests for ArcPlayStoreAppSearchProvider."

This reverts commit cc2fbf55db543c262459dd9ae07bd7753f66e486.

Reason for revert: ArcPlayStoreSearchProviderTest.Basic is flaky on "Linux ChromiumOS Tests" and 2 more ChromiumOS bots. Example fail: https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%281%29/builds/42073

Original change's description:
> Add tests for ArcPlayStoreAppSearchProvider.
> 
> Bug= 736552 
> 
> Change-Id: I6a9da9ce619f302824ef63c447ebce6738de6cf0
> Reviewed-on: https://chromium-review.googlesource.com/576291
> Commit-Queue: Jiaquan He <hejq@google.com>
> Reviewed-by: Luis Hector Chavez <lhchavez@chromium.org>
> Reviewed-by: Yury Khmel <khmel@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#489211}

TBR=lhchavez@chromium.org,khmel@chromium.org,hejq@google.com,hejq@chromium.org

Change-Id: Ib6bce1be6f07bed51b8deb664da6a4eab6af199c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/585008
Reviewed-by: Pavel Kalinnikov <pkalinnikov@chromium.org>
Commit-Queue: Pavel Kalinnikov <pkalinnikov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489286}
[modify] https://crrev.com/f87a709b2218049de5a5c533986f01a4fe0913d9/chrome/browser/ui/app_list/search/arc/arc_playstore_search_provider.h
[delete] https://crrev.com/b856d1a5c9e4426f51500eebe7dd9c2ad1699fc8/chrome/browser/ui/app_list/search/arc/arc_playstore_search_provider_unittest.cc
[modify] https://crrev.com/f87a709b2218049de5a5c533986f01a4fe0913d9/chrome/test/BUILD.gn
[modify] https://crrev.com/f87a709b2218049de5a5c533986f01a4fe0913d9/components/arc/test/fake_app_instance.cc

Project Member

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

Labels: merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e23d5f00055d48da88fc563f4098b50c1d2f37a5

commit e23d5f00055d48da88fc563f4098b50c1d2f37a5
Author: Maajid <maajid@chromium.org>
Date: Sat Jul 29 01:03:30 2017

Add tests for ArcPlayStoreAppSearchProvider.

Bug= 736552 

TBR=hejq@google.com

(cherry picked from commit cc2fbf55db543c262459dd9ae07bd7753f66e486)

Change-Id: I6a9da9ce619f302824ef63c447ebce6738de6cf0
Reviewed-on: https://chromium-review.googlesource.com/576291
Commit-Queue: Jiaquan He <hejq@google.com>
Reviewed-by: Luis Hector Chavez <lhchavez@chromium.org>
Reviewed-by: Yury Khmel <khmel@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#489211}
Bug: 
Reviewed-on: https://chromium-review.googlesource.com/592526
Reviewed-by: Vadim Tryshev <vadimt@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#131}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/e23d5f00055d48da88fc563f4098b50c1d2f37a5/chrome/browser/ui/app_list/search/arc/arc_playstore_search_provider.h
[add] https://crrev.com/e23d5f00055d48da88fc563f4098b50c1d2f37a5/chrome/browser/ui/app_list/search/arc/arc_playstore_search_provider_unittest.cc
[modify] https://crrev.com/e23d5f00055d48da88fc563f4098b50c1d2f37a5/chrome/test/BUILD.gn
[modify] https://crrev.com/e23d5f00055d48da88fc563f4098b50c1d2f37a5/components/arc/test/fake_app_instance.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Aug 1 2017

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

commit 7a7d7202fede9b1a48496c1ff0e02b36cb08fc1f
Author: Jiaquan He <hejq@google.com>
Date: Tue Aug 01 19:17:27 2017

Reland "Add tests for ArcPlayStoreAppSearchProvider."

This relands commit cc2fbf55db543c262459dd9ae07bd7753f66e486,
which was reverted by f87a709b2218049de5a5c533986f01a4fe0913d9.

We create threads to load icons while initializing
ArcPlayStoreSearchResult objects,  so we have to wait for
them to finish, otherwise the test will be flaky and fail
the base::MessageLoop::current()->IsIdleForTesting() test
in test_browser_thread_bundle.cc.

Bug= 736552 

Change-Id: Icd054ea66367f3c28c3333522795e85475f40a1f
Reviewed-on: https://chromium-review.googlesource.com/590027
Reviewed-by: Luis Hector Chavez <lhchavez@chromium.org>
Reviewed-by: Yury Khmel <khmel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491077}
[modify] https://crrev.com/7a7d7202fede9b1a48496c1ff0e02b36cb08fc1f/chrome/browser/ui/app_list/search/arc/arc_playstore_search_provider.h
[add] https://crrev.com/7a7d7202fede9b1a48496c1ff0e02b36cb08fc1f/chrome/browser/ui/app_list/search/arc/arc_playstore_search_provider_unittest.cc
[modify] https://crrev.com/7a7d7202fede9b1a48496c1ff0e02b36cb08fc1f/chrome/browser/ui/app_list/search/arc/arc_playstore_search_result.cc
[modify] https://crrev.com/7a7d7202fede9b1a48496c1ff0e02b36cb08fc1f/chrome/browser/ui/app_list/search/arc/arc_playstore_search_result.h
[modify] https://crrev.com/7a7d7202fede9b1a48496c1ff0e02b36cb08fc1f/chrome/test/BUILD.gn
[modify] https://crrev.com/7a7d7202fede9b1a48496c1ff0e02b36cb08fc1f/components/arc/test/fake_app_instance.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Aug 3 2017

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

commit 00c71fe692c9f0540239b70c667cbd53a2b8c4b8
Author: Jiaquan He <hejq@google.com>
Date: Thu Aug 03 22:24:39 2017

Reland "Add tests for ArcPlayStoreAppSearchProvider."

This relands commit cc2fbf55db543c262459dd9ae07bd7753f66e486,
which was reverted by f87a709b2218049de5a5c533986f01a4fe0913d9.

We create threads to load icons while initializing
ArcPlayStoreSearchResult objects,  so we have to wait for
them to finish, otherwise the test will be flaky and fail
the base::MessageLoop::current()->IsIdleForTesting() test
in test_browser_thread_bundle.cc.

Bug= 736552 

TBR=hejq@google.com

(cherry picked from commit 7a7d7202fede9b1a48496c1ff0e02b36cb08fc1f)

Change-Id: Icd054ea66367f3c28c3333522795e85475f40a1f
Reviewed-on: https://chromium-review.googlesource.com/590027
Reviewed-by: Luis Hector Chavez <lhchavez@chromium.org>
Reviewed-by: Yury Khmel <khmel@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#491077}
Reviewed-on: https://chromium-review.googlesource.com/600865
Reviewed-by: Vadim Tryshev <vadimt@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#293}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/00c71fe692c9f0540239b70c667cbd53a2b8c4b8/chrome/browser/ui/app_list/search/arc/arc_playstore_search_provider_unittest.cc
[modify] https://crrev.com/00c71fe692c9f0540239b70c667cbd53a2b8c4b8/chrome/browser/ui/app_list/search/arc/arc_playstore_search_result.cc
[modify] https://crrev.com/00c71fe692c9f0540239b70c667cbd53a2b8c4b8/chrome/browser/ui/app_list/search/arc/arc_playstore_search_result.h

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

Status: Archived (was: Fixed)

Sign in to add a comment