arc: Handle default app not availble case.

arc: Handle default app not availble case.

This CL handles the situation when default app is not available
after Arc starts. In this case, default app entry is removed from
the app launcher.

BUG=b/31934494
TEST=Extended unit tests
TEST=Manually on device.
     Normal flow: Start Arc by clicking default app icon. Spinning
     item appears in shelf for the starting app. Play Store starts,
     spiining item stil exists and installtion stars (view in
     notification). Once default app is installed, it automatically
     pops up and spinning item is replaced with normal icon.
     App is not available flow: Repeat steps above. Installation
     for default app does not appear and spinning item is removed
     from shelf soon after. Default app is also removed from the
     app launcher.
     Re-sign in. In both cases default app state is preserved.

Review-Url: https://codereview.chromium.org/2601323002
Cr-Commit-Position: refs/heads/master@{#442073}
Committed: https://chromium.googlesource.com/chromium/src/+/d9d2b1f6a2243c1aa3fbda163d69c6e257e3a154

[khmel, 2016-12-29 21:57:43]

The CQ bit was checked by khmel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-29 21:57:57]

Dry run: 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-12-29 22:24:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-29 22:24:28]

Dry run: This issue passed the CQ dry run.

[khmel, 2017-01-03 15:43:14]

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

[khmel, 2017-01-03 15:43:15]

Hi,

PTAL

[xiyuan, 2017-01-03 17:42:15]

lgtm

[Luis Héctor Chávez, 2017-01-03 19:08:00]

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

https://codereview.chromium.org/2601323002/diff/1/chrome/browser/ui/app_list/...
chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc:56: constexpr
base::TimeDelta kValidateDefaultAppaAvailabilityTimeout =
nit: s/AppaAvailability/AppAvailability/

https://codereview.chromium.org/2601323002/diff/1/chrome/browser/ui/app_list/...
chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc:406: bool uninstalled;
nit: = false.

https://codereview.chromium.org/2601323002/diff/1/chrome/browser/ui/app_list/...
chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc:932: void
ArcAppListPrefs::ValidateDefaultAppaAvailability() {
This does not perform validation. How about calling it
DetectDefaultAppAvailability?

https://codereview.chromium.org/2601323002/diff/1/chrome/browser/ui/app_list/...
chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc:935: if
(!GetPackage(package) && !default_apps_installations_.count(package)) {
nit: elide braces

https://codereview.chromium.org/2601323002/diff/1/chrome/browser/ui/app_list/...
chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc:942: // Find at least one
not installed defualt app package.
nit: s/defualt/default/

https://codereview.chromium.org/2601323002/diff/1/chrome/browser/ui/app_list/...
chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc:1318:
HandlePackageRemoved(*package_name);
Isn't this a no-op when !GetPackage(*package_name)?

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

https://codereview.chromium.org/2601323002/diff/1/chrome/browser/ui/app_list/...
chrome/browser/ui/app_list/arc/arc_app_list_prefs.h:293: 
nit: remove this blank line to indicate that these methods are still AppHost
overrides.

https://codereview.chromium.org/2601323002/diff/1/chrome/browser/ui/app_list/...
chrome/browser/ui/app_list/arc/arc_app_list_prefs.h:365: void
ValidateDefaultAppaAvailability();
nit: s/AppaAvailability/AppAvailability/

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

https://codereview.chromium.org/2601323002/diff/1/chrome/browser/ui/app_list/...
chrome/browser/ui/app_list/arc/arc_app_unittest.cc:1327: // Timeout does not
affect default app availibilty because all installations
nit: s/availibility/availability/

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

https://codereview.chromium.org/2601323002/diff/1/components/arc/common/app.m...
components/arc/common/app.mojom:142: [MinVersion=17] bool success@1);
doesn't this also need to be optional? otherwise you'll get 'false' as the
default value instead of OptionNone.

[khmel, 2017-01-03 21:40:57]

Thank you for reviewing! PTAL updated version.

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

https://codereview.chromium.org/2601323002/diff/1/chrome/browser/ui/app_list/...
chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc:56: constexpr
base::TimeDelta kValidateDefaultAppaAvailabilityTimeout =
On 2017/01/03 19:08:00, Luis Héctor Chávez wrote:
> nit: s/AppaAvailability/AppAvailability/

Thanks!

https://codereview.chromium.org/2601323002/diff/1/chrome/browser/ui/app_list/...
chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc:406: bool uninstalled;
On 2017/01/03 19:08:00, Luis Héctor Chávez wrote:
> nit: = false.

Done.

https://codereview.chromium.org/2601323002/diff/1/chrome/browser/ui/app_list/...
chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc:932: void
ArcAppListPrefs::ValidateDefaultAppaAvailability() {
On 2017/01/03 19:08:00, Luis Héctor Chávez wrote:
> This does not perform validation. How about calling it
> DetectDefaultAppAvailability?

Done.

https://codereview.chromium.org/2601323002/diff/1/chrome/browser/ui/app_list/...
chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc:935: if
(!GetPackage(package) && !default_apps_installations_.count(package)) {
On 2017/01/03 19:07:59, Luis Héctor Chávez wrote:
> nit: elide braces

Done.

https://codereview.chromium.org/2601323002/diff/1/chrome/browser/ui/app_list/...
chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc:942: // Find at least one
not installed defualt app package.
On 2017/01/03 19:08:00, Luis Héctor Chávez wrote:
> nit: s/defualt/default/

Done.

https://codereview.chromium.org/2601323002/diff/1/chrome/browser/ui/app_list/...
chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc:1318:
HandlePackageRemoved(*package_name);
On 2017/01/03 19:07:59, Luis Héctor Chávez wrote:
> Isn't this a no-op when !GetPackage(*package_name)?

HandlePackageRemoved explicitly marks package uninstalled. that is why this app
won't appear again when user signs out/signs in again. And HandlePackageRemoved
notifies that app has been removed. After this App launcher won't have this
entry.

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

https://codereview.chromium.org/2601323002/diff/1/chrome/browser/ui/app_list/...
chrome/browser/ui/app_list/arc/arc_app_list_prefs.h:293: 
On 2017/01/03 19:08:00, Luis Héctor Chávez wrote:
> nit: remove this blank line to indicate that these methods are still AppHost
> overrides.

Done.

https://codereview.chromium.org/2601323002/diff/1/chrome/browser/ui/app_list/...
chrome/browser/ui/app_list/arc/arc_app_list_prefs.h:365: void
ValidateDefaultAppaAvailability();
On 2017/01/03 19:08:00, Luis Héctor Chávez wrote:
> nit: s/AppaAvailability/AppAvailability/

Done.

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

https://codereview.chromium.org/2601323002/diff/1/chrome/browser/ui/app_list/...
chrome/browser/ui/app_list/arc/arc_app_unittest.cc:1327: // Timeout does not
affect default app availibilty because all installations
On 2017/01/03 19:08:00, Luis Héctor Chávez wrote:
> nit: s/availibility/availability/

Done.

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

https://codereview.chromium.org/2601323002/diff/1/components/arc/common/app.m...
components/arc/common/app.mojom:142: [MinVersion=17] bool success@1);
On 2017/01/03 19:08:00, Luis Héctor Chávez wrote:
> doesn't this also need to be optional? otherwise you'll get 'false' as the
> default value instead of OptionNone.

Bool cannot be optional. So I embedded everything to struct

[Luis Héctor Chávez, 2017-01-04 16:01:41]

lgtm

[dcheng, 2017-01-05 06:54:05]

https://codereview.chromium.org/2601323002/diff/20001/components/arc/common/a...
File components/arc/common/app.mojom (right):

https://codereview.chromium.org/2601323002/diff/20001/components/arc/common/a...
components/arc/common/app.mojom:141: [MinVersion=17] string? package_name@0);
How come the package name is optional here?

[khmel, 2017-01-05 15:24:29]

https://codereview.chromium.org/2601323002/diff/20001/components/arc/common/a...
File components/arc/common/app.mojom (right):

https://codereview.chromium.org/2601323002/diff/20001/components/arc/common/a...
components/arc/common/app.mojom:141: [MinVersion=17] string? package_name@0);
On 2017/01/05 06:54:05, dcheng wrote:
> How come the package name is optional here?

My intention here to support versions < 17. If I declare it as required  , as I
understand, in case Android side version is 16, it would be impossible to make
this call.

[dcheng, 2017-01-05 16:25:50]

mojo lgtm

[khmel, 2017-01-05 20:28:17]

Thank you for your review!

Adding Stefan for c/b/ui/ash/launcher/arc_app_launcher_browsertest.cc
(mechanical parameter addition)

PTAL

[khmel, 2017-01-05 20:30:40]

khmel@chromium.org changed reviewers:
+ skuhne@chromium.org

[khmel, 2017-01-05 20:30:40]

PTAL

[khmel, 2017-01-06 21:33:58]

The CQ bit was checked by khmel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-06 21:34:26]

Dry run: 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, 2017-01-06 22:03:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-06 22:03:10]

Dry run: This issue passed the CQ dry run.

[Mr4D (OOO till 08-26), 2017-01-06 22:12:24]

lgtm

https://codereview.chromium.org/2601323002/diff/40001/chrome/browser/ui/app_l...
File chrome/browser/ui/app_list/arc/arc_app_list_prefs.h (right):

https://codereview.chromium.org/2601323002/diff/40001/chrome/browser/ui/app_l...
chrome/browser/ui/app_list/arc/arc_app_list_prefs.h:369: // some default app is
not available yet.
Eeek - this is weird. A timeout? What happens when there is no network
connection? Or is this about pre-installed "compressed" apk's?

Thinking of starting a session with 50 tabs the system might be busy with itself
for a minute or so alone.

[khmel, 2017-01-06 22:19:08]

https://codereview.chromium.org/2601323002/diff/40001/chrome/browser/ui/app_l...
File chrome/browser/ui/app_list/arc/arc_app_list_prefs.h (right):

https://codereview.chromium.org/2601323002/diff/40001/chrome/browser/ui/app_l...
chrome/browser/ui/app_list/arc/arc_app_list_prefs.h:369: // some default app is
not available yet.
On 2017/01/06 22:12:24, Mr4D wrote:
> Eeek - this is weird. A timeout? What happens when there is no network
> connection? Or is this about pre-installed "compressed" apk's?
> 
> Thinking of starting a session with 50 tabs the system might be busy with
itself
> for a minute or so alone.

Timeout is 15 seconds. In my tests install session starts in 1-2 seconds  after
refreshing app list. No network connection is relative rare case because we just
passed sign-in stage which requires network connection. Of cause problem may
happen after sign-in and before PAI installation started. In worse case app
disappears from App launcher and appear again once app is installed by PAI or by
user manually.
PlayStore app is very closed system and it is impossible currently to get
notification what happening inside PAI process. I spent many hours trying to
find the way to get notification from PlayStore but ....

[khmel, 2017-01-06 22:20:06]

The CQ bit was checked by khmel@chromium.org

[khmel, 2017-01-06 22:20:07]

The patchset sent to the CQ was uploaded after l-g-t-m from xiyuan@chromium.org,
lhchavez@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2601323002/#ps40001
(title: "rebase")

[commit-bot: I haz the power, 2017-01-06 22:20:28]

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, 2017-01-06 22:24:53]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1483741206973690,
"parent_rev": "3180362167493d3261e31d0cb4a7fa1f5180683f", "commit_rev":
"d9d2b1f6a2243c1aa3fbda163d69c6e257e3a154"}

[commit-bot: I haz the power, 2017-01-06 22:25:22]

Description was changed from

==========
arc: Handle default app not availble case.

This CL handles the situation when default app is not available
after Arc starts. In this case, default app entry is removed from
the app launcher.

BUG=b/31934494
TEST=Extended unit tests
TEST=Manually on device.
     Normal flow: Start Arc by clicking default app icon. Spinning
     item appears in shelf for the starting app. Play Store starts,
     spiining item stil exists and installtion stars (view in
     notification). Once default app is installed, it automatically
     pops up and spinning item is replaced with normal icon.
     App is not available flow: Repeat steps above. Installation
     for default app does not appear and spinning item is removed
     from shelf soon after. Default app is also removed from the
     app launcher.
     Re-sign in. In both cases default app state is preserved.
==========

to

==========
arc: Handle default app not availble case.

This CL handles the situation when default app is not available
after Arc starts. In this case, default app entry is removed from
the app launcher.

BUG=b/31934494
TEST=Extended unit tests
TEST=Manually on device.
     Normal flow: Start Arc by clicking default app icon. Spinning
     item appears in shelf for the starting app. Play Store starts,
     spiining item stil exists and installtion stars (view in
     notification). Once default app is installed, it automatically
     pops up and spinning item is replaced with normal icon.
     App is not available flow: Repeat steps above. Installation
     for default app does not appear and spinning item is removed
     from shelf soon after. Default app is also removed from the
     app launcher.
     Re-sign in. In both cases default app state is preserved.

Review-Url: https://codereview.chromium.org/2601323002
Cr-Commit-Position: refs/heads/master@{#442073}
Committed:
https://chromium.googlesource.com/chromium/src/+/d9d2b1f6a2243c1aa3fbda163d69...
==========

[commit-bot: I haz the power, 2017-01-06 22:25:23]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/d9d2b1f6a2243c1aa3fbda163d69...