arc: Add package app list updated event.

arc: Add package app list updated event.

This requires to keep state of installed Arc apps when a package gets
updated. Before 2 events were issued, package removed and packaged
added. First event removes all relevant data from Chrome prefs and this
also makes pin disappearing.

BUG=b/30564914
TEST=Manually on device. Pin and position in recent are preserved on
     package update. Pin and item from app list are removed when
     packaged is uninstalled.
TEST=Extend unit_tests browser_tests

Committed: https://crrev.com/500b22fce0a7c794d90d3ddbba5aec93f9e8f9ab
Cr-Commit-Position: refs/heads/master@{#411194}

[khmel, 2016-08-09 15:31:28]

khmel@chromium.org changed reviewers:
+ lhchavez@chromium.org, xiyuan@chromium.org

[khmel, 2016-08-09 15:31:29]

Hi Xiyuan and Luis,

PTAL

https://codereview.chromium.org/2228663003/diff/1/chrome/browser/ui/ash/launc...
File chrome/browser/ui/ash/launcher/arc_app_launcher_browsertest.cc (right):

https://codereview.chromium.org/2228663003/diff/1/chrome/browser/ui/ash/launc...
chrome/browser/ui/ash/launcher/arc_app_launcher_browsertest.cc:45: //
TODO(khmel): use '.' when it is safe in ArcAppListPrefs.
Due buggy implementation of package support in arc app prefs

[xiyuan, 2016-08-09 16:51:14]

https://codereview.chromium.org/2228663003/diff/1/chrome/browser/ui/ash/launc...
File chrome/browser/ui/ash/launcher/arc_app_launcher_browsertest.cc (right):

https://codereview.chromium.org/2228663003/diff/1/chrome/browser/ui/ash/launc...
chrome/browser/ui/ash/launcher/arc_app_launcher_browsertest.cc:45: //
TODO(khmel): use '.' when it is safe in ArcAppListPrefs.
On 2016/08/09 15:31:28, khmel wrote:
> Due buggy implementation of package support in arc app prefs

Can we fix this? Losting data for dogfooder might be okay. :p

GetDictionary -> GetDictionaryWithoutPathExpansion in
https://cs.chromium.org/chromium/src/chrome/browser/ui/app_list/arc/arc_app_l...

and

Remove -> RemoveWithoutPathExpansion in
https://cs.chromium.org/chromium/src/chrome/browser/ui/app_list/arc/arc_app_l...

?

https://codereview.chromium.org/2228663003/diff/1/components/arc/common/app.m...
File components/arc/common/app.mojom (right):

https://codereview.chromium.org/2228663003/diff/1/components/arc/common/app.m...
components/arc/common/app.mojom:82: array<AppInfo> apps);
So we are tracking apps from a package in ARC side? Just wondering how we can
make this call.

[Luis Héctor Chávez, 2016-08-09 17:25:04]

https://codereview.chromium.org/2228663003/diff/1/chrome/browser/ui/app_list/...
File chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc (right):

https://codereview.chromium.org/2228663003/diff/1/chrome/browser/ui/app_list/...
chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc:625: const bool was_disable
= ready_apps_.count(app_id) == 0;
nit: was_disabled

https://codereview.chromium.org/2228663003/diff/1/chrome/browser/ui/app_list/...
chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc:747:
AddAppAndShortcut(app->name, app->package_name, app->activity,
Don't you also want to do the validation in L728? How about refactoring
OnAppAddedDeprecated and making both this and that function call the refactored
one? (Until the deprecated one is removed, then you can merge it back here).

https://codereview.chromium.org/2228663003/diff/1/chrome/browser/ui/app_list/...
chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc:771: std::set<std::string>
ArcAppListPrefs::GetAppsForPackage(
std::unordered_set?

[khmel, 2016-08-09 17:46:39]

Thanks Luis and Xiyuan for your comments.

PTAL

https://codereview.chromium.org/2228663003/diff/1/chrome/browser/ui/app_list/...
File chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc (right):

https://codereview.chromium.org/2228663003/diff/1/chrome/browser/ui/app_list/...
chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc:625: const bool was_disable
= ready_apps_.count(app_id) == 0;
On 2016/08/09 17:25:02, Luis Héctor Chávez wrote:
> nit: was_disabled

Done.

https://codereview.chromium.org/2228663003/diff/1/chrome/browser/ui/app_list/...
chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc:747:
AddAppAndShortcut(app->name, app->package_name, app->activity,
On 2016/08/09 17:25:02, Luis Héctor Chávez wrote:
> Don't you also want to do the validation in L728? How about refactoring
> OnAppAddedDeprecated and making both this and that function call the
refactored
> one? (Until the deprecated one is removed, then you can merge it back here).

Done

https://codereview.chromium.org/2228663003/diff/1/chrome/browser/ui/ash/launc...
File chrome/browser/ui/ash/launcher/arc_app_launcher_browsertest.cc (right):

https://codereview.chromium.org/2228663003/diff/1/chrome/browser/ui/ash/launc...
chrome/browser/ui/ash/launcher/arc_app_launcher_browsertest.cc:45: //
TODO(khmel): use '.' when it is safe in ArcAppListPrefs.
On 2016/08/09 16:51:14, xiyuan wrote:
> On 2016/08/09 15:31:28, khmel wrote:
> > Due buggy implementation of package support in arc app prefs
> 
> Can we fix this? Losting data for dogfooder might be okay. :p
> 
> GetDictionary -> GetDictionaryWithoutPathExpansion in
>
https://cs.chromium.org/chromium/src/chrome/browser/ui/app_list/arc/arc_app_l...
> 
> and
> 
> Remove -> RemoveWithoutPathExpansion in
>
https://cs.chromium.org/chromium/src/chrome/browser/ui/app_list/arc/arc_app_l...
> 
> ?

Nice easy fix :). Thanks!
Also fixed one more GetDictionary for consistency.

https://codereview.chromium.org/2228663003/diff/1/components/arc/common/app.m...
File components/arc/common/app.mojom (right):

https://codereview.chromium.org/2228663003/diff/1/components/arc/common/app.m...
components/arc/common/app.mojom:82: array<AppInfo> apps);
On 2016/08/09 16:51:14, xiyuan wrote:
> So we are tracking apps from a package in ARC side? Just wondering how we can
> make this call.

Yes, we are sending this event from ARC side. Relative change is here:
https://googleplex-android-review.git.corp.google.com/#/c/1304858/1/ArcAppLau...
Most important change: line 514

[xiyuan, 2016-08-09 17:53:49]

lgtm

Thanks for arc CL link.

[Luis Héctor Chávez, 2016-08-09 17:58:46]

lgtm

[khmel, 2016-08-09 18:04:00]

khmel@chromium.org changed reviewers:
+ lgcheng@chromium.org, oshima@chromium.org, rickyz@chromium.org

[khmel, 2016-08-09 18:04:00]

Thanks for reviewing:

+oshima@ (c/b/ui/ash/launcher/arc_app_launcher_browsertest.cc)
+rickyz@ (components/arc/common/app.mojom)
+lgcheng@ - FYI - fix for package names.

PTAL

[rickyz (no longer on Chrome), 2016-08-09 19:56:07]

mojom lgtm

[oshima, 2016-08-10 18:03:55]

lgtm

[khmel, 2016-08-10 22:48:18]

The CQ bit was checked by khmel@chromium.org

[khmel, 2016-08-10 22:48:19]

The patchset sent to the CQ was uploaded after l-g-t-m from xiyuan@chromium.org,
lhchavez@chromium.org, rickyz@chromium.org, oshima@chromium.org
Link to the patchset: https://codereview.chromium.org/2228663003/#ps60001
(title: "rebase2")

[commit-bot: I haz the power, 2016-08-10 22:49:10]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-10 23:56:44]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-08-10 23:58:34]

Description was changed from

==========
arc: Add package app list updated event.

This requires to keep state of installed Arc apps when a package gets
updated. Before 2 events were issued, package removed and packaged
added. First event removes all relevant data from Chrome prefs and this
also makes pin disappearing.

BUG=b/30564914
TEST=Manually on device. Pin and position in recent are preserved on
     package update. Pin and item from app list are removed when
     packaged is uninstalled.
TEST=Extend unit_tests browser_tests
==========

to

==========
arc: Add package app list updated event.

This requires to keep state of installed Arc apps when a package gets
updated. Before 2 events were issued, package removed and packaged
added. First event removes all relevant data from Chrome prefs and this
also makes pin disappearing.

BUG=b/30564914
TEST=Manually on device. Pin and position in recent are preserved on
     package update. Pin and item from app list are removed when
     packaged is uninstalled.
TEST=Extend unit_tests browser_tests

Committed: https://crrev.com/500b22fce0a7c794d90d3ddbba5aec93f9e8f9ab
Cr-Commit-Position: refs/heads/master@{#411194}
==========

[commit-bot: I haz the power, 2016-08-10 23:58:36]

Patchset 4 (id:??) landed as
https://crrev.com/500b22fce0a7c794d90d3ddbba5aec93f9e8f9ab
Cr-Commit-Position: refs/heads/master@{#411194}