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

Issue 652470 link

Starred by 2 users

Issue metadata

Status: Archived
Owner:
Closed: Oct 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Fix race when Arc package sync service starts.

Project Member Reported by lgcheng@google.com, Oct 3 2016

Issue description

A race is introduced back between initial packagelist refreshed and sync DataTypeController::OnModelStarted.

Fix the race by changing proper observer.

Inner link:
http://b/31831978


 
Project Member

Comment 1 by sheriffbot@chromium.org, Oct 4 2016

Labels: Hotlist-Google
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 6 2016

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

commit 88a14008509adefe2b764ccb573ba629ba68dac5
Author: lgcheng <lgcheng@google.com>
Date: Thu Oct 06 00:52:42 2016

arc: Fix race when arc package sync service starts.
Fix the race which happens between initial packagelist refreshed and
sync DataTypeController::OnModelStarted by changing the observer of
OnAppInstanceReady() to OnPackageListInitialRefreshed().

BUG= 652470 
Test=Manual Test. Sync service always starts after arc packagelist
refreshes correctly.
Test=Pass sync integration test.

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

[modify] https://crrev.com/88a14008509adefe2b764ccb573ba629ba68dac5/chrome/browser/sync/test/integration/sync_arc_package_helper.cc
[modify] https://crrev.com/88a14008509adefe2b764ccb573ba629ba68dac5/chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc
[modify] https://crrev.com/88a14008509adefe2b764ccb573ba629ba68dac5/chrome/browser/ui/app_list/arc/arc_app_list_prefs.h
[modify] https://crrev.com/88a14008509adefe2b764ccb573ba629ba68dac5/chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc
[modify] https://crrev.com/88a14008509adefe2b764ccb573ba629ba68dac5/chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.h

Comment 3 by lgcheng@google.com, Oct 6 2016

Labels: Merge-Request-54

Comment 4 by dimu@chromium.org, Oct 6 2016

Labels: -Merge-Request-54 Merge-Review-54 Hotlist-Merge-Review
[Automated comment] Less than 2 weeks to go before stable on M54, manual review required.

Comment 5 by lgcheng@google.com, Oct 7 2016

This patch should be merged together with https://bugs.chromium.org/p/chromium/issues/detail?id=650483

Please help review the merge request.

Comment 6 by lgcheng@google.com, Oct 8 2016

Cc: josa...@chromium.org

Comment 7 by lgcheng@google.com, Oct 10 2016

Cc: bhthompson@chromium.org
Labels: -Merge-Review-54 Merge-Approved-54
Please double check this made 55 as well.

Comment 9 by lgcheng@google.com, Oct 12 2016

Re #8 confirm this patch landed in 55.0.2882.0.

M54 merge coming in a moment.
Project Member

Comment 10 by bugdroid1@chromium.org, Oct 12 2016

Labels: -merge-approved-54 merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e24af55b17c3554262658d03395bc48ca05deb4a

commit e24af55b17c3554262658d03395bc48ca05deb4a
Author: Josh Horwich <jhorwich@chromium.org>
Date: Wed Oct 12 00:16:25 2016

[Merge To M54]arc: Fix race when arc package sync service starts.
Fix the race which happens between initial packagelist refreshed and sync
DataTypeController::OnModelStarted by changing the observer of
OnAppInstanceReady() to OnPackageListInitialRefreshed().

BUG= 652470 
TBR=xiyuan@chromium.org, pavely@chromium.org, khmel@chromium.org
Test=Manual Test. Sync service always starts after arc packagelist
refreshes correctly.
Test=Pass sync integration test.

Review-Url: https://codereview.chromium.org/2389763003
Cr-Commit-Position: refs/heads/master@{#423362}
(cherry picked from commit 88a14008509adefe2b764ccb573ba629ba68dac5)

Review URL: https://codereview.chromium.org/2409253004 .

Cr-Commit-Position: refs/branch-heads/2840@{#725}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/e24af55b17c3554262658d03395bc48ca05deb4a/chrome/browser/sync/test/integration/sync_arc_package_helper.cc
[modify] https://crrev.com/e24af55b17c3554262658d03395bc48ca05deb4a/chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc
[modify] https://crrev.com/e24af55b17c3554262658d03395bc48ca05deb4a/chrome/browser/ui/app_list/arc/arc_app_list_prefs.h
[modify] https://crrev.com/e24af55b17c3554262658d03395bc48ca05deb4a/chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc
[modify] https://crrev.com/e24af55b17c3554262658d03395bc48ca05deb4a/chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.h

Comment 11 by lgcheng@google.com, Oct 17 2016

Status: Fixed (was: Started)
Project Member

Comment 12 by bugdroid1@chromium.org, Oct 27 2016

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

commit 88a14008509adefe2b764ccb573ba629ba68dac5
Author: lgcheng <lgcheng@google.com>
Date: Thu Oct 06 00:52:42 2016

arc: Fix race when arc package sync service starts.
Fix the race which happens between initial packagelist refreshed and
sync DataTypeController::OnModelStarted by changing the observer of
OnAppInstanceReady() to OnPackageListInitialRefreshed().

BUG= 652470 
Test=Manual Test. Sync service always starts after arc packagelist
refreshes correctly.
Test=Pass sync integration test.

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

[modify] https://crrev.com/88a14008509adefe2b764ccb573ba629ba68dac5/chrome/browser/sync/test/integration/sync_arc_package_helper.cc
[modify] https://crrev.com/88a14008509adefe2b764ccb573ba629ba68dac5/chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc
[modify] https://crrev.com/88a14008509adefe2b764ccb573ba629ba68dac5/chrome/browser/ui/app_list/arc/arc_app_list_prefs.h
[modify] https://crrev.com/88a14008509adefe2b764ccb573ba629ba68dac5/chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc
[modify] https://crrev.com/88a14008509adefe2b764ccb573ba629ba68dac5/chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.h

Project Member

Comment 13 by bugdroid1@chromium.org, Oct 27 2016

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

commit e24af55b17c3554262658d03395bc48ca05deb4a
Author: Josh Horwich <jhorwich@chromium.org>
Date: Wed Oct 12 00:16:25 2016

[Merge To M54]arc: Fix race when arc package sync service starts.
Fix the race which happens between initial packagelist refreshed and sync
DataTypeController::OnModelStarted by changing the observer of
OnAppInstanceReady() to OnPackageListInitialRefreshed().

BUG= 652470 
TBR=xiyuan@chromium.org, pavely@chromium.org, khmel@chromium.org
Test=Manual Test. Sync service always starts after arc packagelist
refreshes correctly.
Test=Pass sync integration test.

Review-Url: https://codereview.chromium.org/2389763003
Cr-Commit-Position: refs/heads/master@{#423362}
(cherry picked from commit 88a14008509adefe2b764ccb573ba629ba68dac5)

Review URL: https://codereview.chromium.org/2409253004 .

Cr-Commit-Position: refs/branch-heads/2840@{#725}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/e24af55b17c3554262658d03395bc48ca05deb4a/chrome/browser/sync/test/integration/sync_arc_package_helper.cc
[modify] https://crrev.com/e24af55b17c3554262658d03395bc48ca05deb4a/chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc
[modify] https://crrev.com/e24af55b17c3554262658d03395bc48ca05deb4a/chrome/browser/ui/app_list/arc/arc_app_list_prefs.h
[modify] https://crrev.com/e24af55b17c3554262658d03395bc48ca05deb4a/chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc
[modify] https://crrev.com/e24af55b17c3554262658d03395bc48ca05deb4a/chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.h

Comment 14 by dchan@google.com, Jan 21 2017

Labels: VerifyIn-57

Comment 15 by dchan@google.com, Mar 4 2017

Labels: VerifyIn-58

Comment 16 by dchan@google.com, Apr 17 2017

Labels: VerifyIn-59

Comment 17 by dchan@google.com, May 30 2017

Labels: VerifyIn-60
Labels: VerifyIn-61

Comment 19 by dchan@chromium.org, Oct 14 2017

Status: Archived (was: Fixed)

Sign in to add a comment