arc: Fix duplicate icons in app launcher in case of crash.

arc: Fix duplicate icons in app launcher in case of crash.

This CL initializes tracking items by existing app set on start in
order to prevent race condition that leads to arc app created is
called twice for some apps.

TBR=xiyuan@chromium.org

TEST=Extended unit_tests
TEST=Manually on device with chrome://inducebrowsercrashforrealz
BUG=b/32765086
BUG=667529
Committed: https://crrev.com/4e1e8774f3b147906917dbd4e50f30bfc7372ba6
Cr-Commit-Position: refs/heads/master@{#433980}

[khmel, 2016-11-21 23:49:22]

khmel@chromium.org changed reviewers:
+ lhchavez@lhchavez.com

[khmel, 2016-11-21 23:49:23]

Hi Luis,

PTAL

[Luis Héctor Chávez, 2016-11-22 16:55:23]

lhchavez@chromium.org changed reviewers:
+ lhchavez@chromium.org
- lhchavez@lhchavez.com

[Luis Héctor Chávez, 2016-11-22 16:55:24]

-the wrong lhchavez

[Luis Héctor Chávez, 2016-11-22 17:14:14]

lgtm with nits

https://codereview.chromium.org/2520173002/diff/20001/chrome/browser/ui/app_l...
File chrome/browser/ui/app_list/arc/arc_app_test.cc (right):

https://codereview.chromium.org/2520173002/diff/20001/chrome/browser/ui/app_l...
chrome/browser/ui/app_list/arc/arc_app_test.cc:170:
arc_session_manager_.reset();
This was already destroyed a few lines above.

https://codereview.chromium.org/2520173002/diff/20001/chrome/browser/ui/app_l...
chrome/browser/ui/app_list/arc/arc_app_test.cc:171: app_instance_.reset();
You should try to delete stuff in the reverse order it was initialized. This
should be the first line in TearDown().

https://codereview.chromium.org/2520173002/diff/20001/chrome/browser/ui/app_l...
chrome/browser/ui/app_list/arc/arc_app_test.cc:172: bridge_service_.reset();
And this should be the second line.

https://codereview.chromium.org/2520173002/diff/20001/chrome/browser/ui/app_l...
File chrome/browser/ui/app_list/arc/arc_app_unittest.cc (right):

https://codereview.chromium.org/2520173002/diff/20001/chrome/browser/ui/app_l...
chrome/browser/ui/app_list/arc/arc_app_unittest.cc:437:
ArcAppModelBuilderRecreate() {}
nit: = default. Same below.

https://codereview.chromium.org/2520173002/diff/20001/chrome/browser/ui/app_l...
chrome/browser/ui/app_list/arc/arc_app_unittest.cc:1064: // No apps on fist
initial start.
nit: "No apps on first initial start." (also, first and initial are redundant,
so maybe drop one?)

[khmel, 2016-11-22 18:43:06]

Thank you Luis for review.

I updated the code.
I additionally moved AppModelRestart test above all test for default apps.
Default apps tests activate using of test directory via UseTestAppsDirectory.
Once activated it affects all other tests, executing in batch mode (this causes
tests retry). I will address this issue in separate CL. For now I moved new test
up to solve it for batch mode.

PTAL

https://codereview.chromium.org/2520173002/diff/20001/chrome/browser/ui/app_l...
File chrome/browser/ui/app_list/arc/arc_app_test.cc (right):

https://codereview.chromium.org/2520173002/diff/20001/chrome/browser/ui/app_l...
chrome/browser/ui/app_list/arc/arc_app_test.cc:170:
arc_session_manager_.reset();
On 2016/11/22 17:14:14, Luis Héctor Chávez wrote:
> This was already destroyed a few lines above.

Done.

https://codereview.chromium.org/2520173002/diff/20001/chrome/browser/ui/app_l...
chrome/browser/ui/app_list/arc/arc_app_test.cc:171: app_instance_.reset();
On 2016/11/22 17:14:14, Luis Héctor Chávez wrote:
> You should try to delete stuff in the reverse order it was initialized. This
> should be the first line in TearDown().

Done.

https://codereview.chromium.org/2520173002/diff/20001/chrome/browser/ui/app_l...
chrome/browser/ui/app_list/arc/arc_app_test.cc:172: bridge_service_.reset();
On 2016/11/22 17:14:14, Luis Héctor Chávez wrote:
> And this should be the second line.

Done,
As I see this is third line, arc_session_manager_ is created before.

https://codereview.chromium.org/2520173002/diff/20001/chrome/browser/ui/app_l...
File chrome/browser/ui/app_list/arc/arc_app_unittest.cc (right):

https://codereview.chromium.org/2520173002/diff/20001/chrome/browser/ui/app_l...
chrome/browser/ui/app_list/arc/arc_app_unittest.cc:437:
ArcAppModelBuilderRecreate() {}
On 2016/11/22 17:14:14, Luis Héctor Chávez wrote:
> nit: = default. Same below.

Done.

https://codereview.chromium.org/2520173002/diff/20001/chrome/browser/ui/app_l...
chrome/browser/ui/app_list/arc/arc_app_unittest.cc:1064: // No apps on fist
initial start.
On 2016/11/22 17:14:14, Luis Héctor Chávez wrote:
> nit: "No apps on first initial start." (also, first and initial are redundant,
> so maybe drop one?)

Done.

[Luis Héctor Chávez, 2016-11-22 19:04:24]

lgtm if trybots are happy as long as you address the issue soon.

[khmel, 2016-11-22 19:05:06]

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

[commit-bot: I haz the power, 2016-11-22 19:05:30]

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-11-22 19:50:42]

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

[commit-bot: I haz the power, 2016-11-22 19:50:43]

Dry run: This issue passed the CQ dry run.

[khmel, 2016-11-22 20:54:53]

The CQ bit was checked by khmel@chromium.org

[commit-bot: I haz the power, 2016-11-22 20:55:34]

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-11-22 21:03:18]

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

[commit-bot: I haz the power, 2016-11-22 21:03:19]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[khmel, 2016-11-22 21:07:05]

Description was changed from

==========
arc: Fix duplicate icons in app launcher in case of crash.

This CL initializes tracking items by existing app set on start in
order to prevent race condition that leads to arc app created is
called twice for some apps.

TEST=Extended unit_tests
TEST=Manually on device with chrome://inducebrowsercrashforrealz
BUG=b/32765086
BUG=667529
==========

to

==========
arc: Fix duplicate icons in app launcher in case of crash.

This CL initializes tracking items by existing app set on start in
order to prevent race condition that leads to arc app created is
called twice for some apps.

TBR=xiyuan@chromium.org

TEST=Extended unit_tests
TEST=Manually on device with chrome://inducebrowsercrashforrealz
BUG=b/32765086
BUG=667529
==========

[khmel, 2016-11-22 21:07:26]

Adding Xiyuan TBR.

[khmel, 2016-11-22 21:07:36]

The CQ bit was checked by khmel@chromium.org

[commit-bot: I haz the power, 2016-11-22 21:08:26]

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-11-22 21:13:36]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1479848856146110,
"parent_rev": "8069d21bc107d2aa2363a5ea2a510a016c59a9ed", "commit_rev":
"9095d397abed0bf4e5ccf83dbd1797b703cff9ac"}

[commit-bot: I haz the power, 2016-11-22 21:14:00]

Description was changed from

==========
arc: Fix duplicate icons in app launcher in case of crash.

This CL initializes tracking items by existing app set on start in
order to prevent race condition that leads to arc app created is
called twice for some apps.

TBR=xiyuan@chromium.org

TEST=Extended unit_tests
TEST=Manually on device with chrome://inducebrowsercrashforrealz
BUG=b/32765086
BUG=667529
==========

to

==========
arc: Fix duplicate icons in app launcher in case of crash.

This CL initializes tracking items by existing app set on start in
order to prevent race condition that leads to arc app created is
called twice for some apps.

TBR=xiyuan@chromium.org

TEST=Extended unit_tests
TEST=Manually on device with chrome://inducebrowsercrashforrealz
BUG=b/32765086
BUG=667529
==========

[commit-bot: I haz the power, 2016-11-22 21:14:01]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-11-22 21:16:00]

Description was changed from

==========
arc: Fix duplicate icons in app launcher in case of crash.

This CL initializes tracking items by existing app set on start in
order to prevent race condition that leads to arc app created is
called twice for some apps.

TBR=xiyuan@chromium.org

TEST=Extended unit_tests
TEST=Manually on device with chrome://inducebrowsercrashforrealz
BUG=b/32765086
BUG=667529
==========

to

==========
arc: Fix duplicate icons in app launcher in case of crash.

This CL initializes tracking items by existing app set on start in
order to prevent race condition that leads to arc app created is
called twice for some apps.

TBR=xiyuan@chromium.org

TEST=Extended unit_tests
TEST=Manually on device with chrome://inducebrowsercrashforrealz
BUG=b/32765086
BUG=667529

Committed: https://crrev.com/4e1e8774f3b147906917dbd4e50f30bfc7372ba6
Cr-Commit-Position: refs/heads/master@{#433980}
==========

[commit-bot: I haz the power, 2016-11-22 21:16:01]

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