New issue
Advanced search Search tips

Issue 849936 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Improve app list search for Crostini Apps to check keywords and executable name

Project Member Reported by timloh@chromium.org, Jun 6 2018

Issue description

(branched off bug 836137)

We should improve app list search for Crostini apps to not just match the app name. If you install 'gedit' and search for 'gedit', it doesn't find it!

I chatted with tbuckley@ a while ago and we agreed to use the Keywords field as well as the name of the executable. AppSearchProvider::App already has a searchable_keywords fields which we can use, which will appropriately use the keywords as lowest priority.
 

Comment 1 by vapier@chromium.org, Jun 21 2018

Components: OS>Systems>Containers
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/6130b5431a4628df0a9145229be2364b3fbdc212

commit 6130b5431a4628df0a9145229be2364b3fbdc212
Author: Daniel Ng <danielng@google.com>
Date: Wed Dec 05 09:13:41 2018

system_api: Added keywords field to apps.proto to improve Crostini app search

The keywords field is localized, we needed to add a new message type to
handle localized lists.

BUG=chromium:849936

TEST=Checked that files reading/writing from proto could access this new field
Change-Id: I8cdd1e4051c94921d0efbc3de1688f0191dbc731
Reviewed-on: https://chromium-review.googlesource.com/1359635
Commit-Ready: Daniel Ng <danielng@google.com>
Tested-by: Daniel Ng <danielng@google.com>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/6130b5431a4628df0a9145229be2364b3fbdc212/system_api/dbus/vm_applications/apps.proto

Project Member

Comment 3 by bugdroid1@chromium.org, Dec 6

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/fa65cb8119f8fcc5f5ddb25851ada5afd2b7c5f1

commit fa65cb8119f8fcc5f5ddb25851ada5afd2b7c5f1
Author: Daniel Ng <danielng@google.com>
Date: Thu Dec 06 22:35:26 2018

system_api: Adjusted keywords field to be consistent

Made a change in apps proto from CL:1362632, as per suggestion of
jkardatzke@, made change here to keep protos consistent.

TEST=Rebuilt system and ran unittests

BUG=chromium:849936

Change-Id: I8d49a67c9dfc144fd2e2c4c21f980594e836d6c6
Reviewed-on: https://chromium-review.googlesource.com/1364951
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Daniel Ng <danielng@google.com>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/fa65cb8119f8fcc5f5ddb25851ada5afd2b7c5f1/system_api/dbus/vm_applications/apps.proto

Project Member

Comment 4 by bugdroid1@chromium.org, Dec 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/950c910f692d85e63484bf03a58925eeca00b94e

commit 950c910f692d85e63484bf03a58925eeca00b94e
Author: Daniel Ng <danielng@google.com>
Date: Fri Dec 07 06:06:55 2018

vm_tools: Added keywords to app data

In relation to the bug referenced, this CL is to add changes to
garcon and cicerone so that keyword data is taken from the crostini
apps and is also appropriately communicated via protos

TEST=updated desktop_file_test to test for keywords, also manually
tested interconnected parts to ensure data was being sent correctly

BUG=chromium:849936

CQ-DEPEND=CL:1364951

Change-Id: I353a9c8e4438baac9f53be4e60017d4dcf8f13a1
Reviewed-on: https://chromium-review.googlesource.com/1362632
Commit-Ready: Daniel Ng <danielng@google.com>
Tested-by: Daniel Ng <danielng@google.com>
Reviewed-by: Jeffrey Kardatzke <jkardatzke@google.com>

[modify] https://crrev.com/950c910f692d85e63484bf03a58925eeca00b94e/vm_tools/cicerone/container_listener_impl.cc
[modify] https://crrev.com/950c910f692d85e63484bf03a58925eeca00b94e/vm_tools/proto/container_host.proto
[modify] https://crrev.com/950c910f692d85e63484bf03a58925eeca00b94e/vm_tools/garcon/desktop_file_test.cc
[modify] https://crrev.com/950c910f692d85e63484bf03a58925eeca00b94e/vm_tools/garcon/desktop_file.cc
[modify] https://crrev.com/950c910f692d85e63484bf03a58925eeca00b94e/vm_tools/garcon/desktop_file.h
[modify] https://crrev.com/950c910f692d85e63484bf03a58925eeca00b94e/vm_tools/garcon/host_notifier.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Dec 10

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

commit 35589958431707c10dbfc1330b423f525874e9e1
Author: Daniel Ng <danielng@google.com>
Date: Mon Dec 10 05:01:52 2018

Added keyword search for crostini apps

This CL updates CrostiniRegistryService to store app keywords for use in the
launcher search. AppSearchProvider::App is correspondingly updated to support
multiple searchable strings.

BUG=chromium:849936

CQ-DEPEND=CL:1364951

Change-Id: Ib5d387c317a119b418f397ccdefb73f06081dd83
Reviewed-on: https://chromium-review.googlesource.com/c/1361759
Commit-Queue: Daniel Ng <danielng@google.com>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Timothy Loh <timloh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615048}
[modify] https://crrev.com/35589958431707c10dbfc1330b423f525874e9e1/chrome/browser/chromeos/crostini/crostini_registry_service.cc
[modify] https://crrev.com/35589958431707c10dbfc1330b423f525874e9e1/chrome/browser/chromeos/crostini/crostini_registry_service.h
[modify] https://crrev.com/35589958431707c10dbfc1330b423f525874e9e1/chrome/browser/chromeos/crostini/crostini_registry_service_unittest.cc
[modify] https://crrev.com/35589958431707c10dbfc1330b423f525874e9e1/chrome/browser/chromeos/crostini/crostini_test_helper.cc
[modify] https://crrev.com/35589958431707c10dbfc1330b423f525874e9e1/chrome/browser/chromeos/crostini/crostini_test_helper.h
[modify] https://crrev.com/35589958431707c10dbfc1330b423f525874e9e1/chrome/browser/ui/app_list/search/app_search_provider.cc
[modify] https://crrev.com/35589958431707c10dbfc1330b423f525874e9e1/chrome/browser/ui/app_list/search/tests/app_search_provider_unittest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Dec 20

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/69520a101317bdf106f4f9e908c61c13b0029fe4

commit 69520a101317bdf106f4f9e908c61c13b0029fe4
Author: Daniel Ng <danielng@google.com>
Date: Thu Dec 20 21:49:46 2018

system_api: Added exec field to apps.proto to improve Crostini app search

Added the executable name of apps (exec) so that they can be used
in app search

TEST=manual testing

BUG=chromium:849936

Change-Id: I4d1089f09885eab5e2bd007d0e8c6be0c5995a27
Reviewed-on: https://chromium-review.googlesource.com/1381311
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Daniel Ng <danielng@google.com>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/69520a101317bdf106f4f9e908c61c13b0029fe4/system_api/dbus/vm_applications/apps.proto

Project Member

Comment 7 by bugdroid1@chromium.org, Jan 3

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

commit 3cafe43c57fc74537a48fa4f740f53b962d87a42
Author: Daniel Ng <danielng@google.com>
Date: Thu Jan 03 01:26:33 2019

Added exec as a search term for crostini apps

Modified Crostini registry to store exec data of apps and updated
AppSearchProvider to add exec to the searchable words field. Updated
the appropriate test files to also test for this new case.

Bug: 849936

CQ-DEPEND=CL:1381311

Change-Id: Ice016099aa10a42dabb788c7ae4f05e35b036477
Reviewed-on: https://chromium-review.googlesource.com/c/1382658
Commit-Queue: Daniel Ng <danielng@google.com>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Timothy Loh <timloh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619557}
[modify] https://crrev.com/3cafe43c57fc74537a48fa4f740f53b962d87a42/chrome/browser/chromeos/crostini/crostini_registry_service.cc
[modify] https://crrev.com/3cafe43c57fc74537a48fa4f740f53b962d87a42/chrome/browser/chromeos/crostini/crostini_registry_service.h
[modify] https://crrev.com/3cafe43c57fc74537a48fa4f740f53b962d87a42/chrome/browser/chromeos/crostini/crostini_registry_service_unittest.cc
[modify] https://crrev.com/3cafe43c57fc74537a48fa4f740f53b962d87a42/chrome/browser/ui/app_list/search/app_search_provider.cc
[modify] https://crrev.com/3cafe43c57fc74537a48fa4f740f53b962d87a42/chrome/browser/ui/app_list/search/tests/app_search_provider_unittest.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Jan 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/f8bd43c688fef144047388d0ff999ffcb7561458

commit f8bd43c688fef144047388d0ff999ffcb7561458
Author: Daniel Ng <danielng@google.com>
Date: Fri Jan 04 20:46:38 2019

vm_tools: Added exec to app data

This changes garcon and cicerone to send the exec names of desktop files
are appropriately communicated via protos. Note that the exec name is parsed
so that it is the exec name and not just the raw exec field.

TEST=updated desktop_file_test to test for exec, mannually tested overall
system

BUG=chromium:849936

CQ-DEPEND=CL:1381311

Change-Id: I60422da5649962193154cd163431d8d22ef4a19f
Reviewed-on: https://chromium-review.googlesource.com/1381320
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Daniel Ng <danielng@google.com>
Reviewed-by: Jeffrey Kardatzke <jkardatzke@google.com>

[modify] https://crrev.com/f8bd43c688fef144047388d0ff999ffcb7561458/vm_tools/cicerone/container_listener_impl.cc
[modify] https://crrev.com/f8bd43c688fef144047388d0ff999ffcb7561458/vm_tools/proto/container_host.proto
[modify] https://crrev.com/f8bd43c688fef144047388d0ff999ffcb7561458/vm_tools/garcon/desktop_file_test.cc
[modify] https://crrev.com/f8bd43c688fef144047388d0ff999ffcb7561458/vm_tools/garcon/desktop_file.cc
[modify] https://crrev.com/f8bd43c688fef144047388d0ff999ffcb7561458/vm_tools/garcon/desktop_file.h
[modify] https://crrev.com/f8bd43c688fef144047388d0ff999ffcb7561458/vm_tools/garcon/host_notifier.cc

Sign in to add a comment