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

Issue 637058 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Aug 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

arc: Sync service initialization issue.

Project Member Reported by lgcheng@google.com, Aug 11 2016

Issue description

ArcPackageSyncableService should not be started before Arc service is
ready.

Cause crash:
http://b/30699487

 

Comment 1 by lgcheng@google.com, Aug 11 2016

Labels: Merge-Request-53

Comment 2 by gov...@chromium.org, Aug 11 2016

Cc: keta...@chromium.org
Labels: OS-Chrome

Comment 3 by lgcheng@google.com, Aug 11 2016

Labels: Merge-Request-54
Project Member

Comment 4 by sheriffbot@chromium.org, Aug 12 2016

Labels: Hotlist-Google

Comment 6 by lgcheng@google.com, Aug 12 2016

Labels: -Merge-Request-54
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 12 2016

Labels: merge-merged-2824
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c24c03bb322e34b235fa663d6085a8c3c36db9e1

commit c24c03bb322e34b235fa663d6085a8c3c36db9e1
Author: Elijah Taylor <elijahtaylor@google.com>
Date: Fri Aug 12 21:34:51 2016

arc: Fix ArcPackageSyncableService initialization issue. ArcPackageSyncableService should not be started before Arc service is ready.

BUG= 637058 

TEST=Pass sync integration test.
TEST=Manual test. No crash when add second user.

Review-Url: https://codereview.chromium.org/2231593004
Cr-Commit-Position: refs/heads/master@{#411696}
(cherry picked from commit a0627b05a72e3ddd8b9fbdba6ab93717868a9921)

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

Cr-Commit-Position: refs/branch-heads/2824@{#7}
Cr-Branched-From: facabd3224aecbcab4bea9daadad31c67488d78c-refs/heads/master@{#410520}

[modify] https://crrev.com/c24c03bb322e34b235fa663d6085a8c3c36db9e1/chrome/browser/sync/chrome_sync_client.cc
[modify] https://crrev.com/c24c03bb322e34b235fa663d6085a8c3c36db9e1/chrome/browser/sync/test/integration/migration_test.cc
[add] https://crrev.com/c24c03bb322e34b235fa663d6085a8c3c36db9e1/chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc
[add] https://crrev.com/c24c03bb322e34b235fa663d6085a8c3c36db9e1/chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.h
[modify] https://crrev.com/c24c03bb322e34b235fa663d6085a8c3c36db9e1/chrome/browser/ui/app_list/arc/arc_package_syncable_service.cc
[modify] https://crrev.com/c24c03bb322e34b235fa663d6085a8c3c36db9e1/chrome/chrome_browser_ui.gypi

Cc: gov...@chromium.org

Comment 9 by gov...@chromium.org, Aug 12 2016

ketakid@ for M53 approval as it is Chrome OS change.
Cc: -gov...@chromium.org

Comment 11 by dimu@chromium.org, Aug 13 2016

Labels: -Merge-Request-53 Merge-Review-53 Hotlist-Merge-Review
[Automated comment] There appears to be on-going work (i.e. bugroid changes), needs manual review.
govind@ should this be approved by you since this is a chrome change?
Cc: gov...@chromium.org
Before we approve merge to M53, Could you please confirm whether this change is baked/verified in Canary and safe to merge?

Comment 15 by lgcheng@google.com, Aug 15 2016

It is verified in M54 branch. It can not be verified in Canary because no successful PFQ roll after the patch is landed. Details can be seen at:

http://b/30699487
Labels: -Merge-Review-53 Merge-Approved-53
Approving merge to M53 branch 2785 based on comment #15 and per chat with ketakid@ this imp merge for CrOS. Please merge ASAP. Thank you.
Project Member

Comment 17 by bugdroid1@chromium.org, Aug 16 2016

Labels: -merge-approved-53 merge-merged-2785
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d7e92bc01d0b627a4d66d5ffce4d686f50771075

commit d7e92bc01d0b627a4d66d5ffce4d686f50771075
Author: Josh Horwich <jhorwich@chromium.org>
Date: Tue Aug 16 18:13:12 2016

arc: Fix ArcPackageSyncableService initialization issue.

ArcPackageSyncableService should not be started before Arc service is
ready.

BUG= 637058 

TEST=Pass sync integration test.
TEST=Manual test. No crash when add second user.

TBR=stevenjb@chromium.org, pavely@chromium.org

Review-Url: https://codereview.chromium.org/2231593004
Cr-Commit-Position: refs/heads/master@{#411696}
(cherry picked from commit a0627b05a72e3ddd8b9fbdba6ab93717868a9921)

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

Cr-Commit-Position: refs/branch-heads/2785@{#626}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

[modify] https://crrev.com/d7e92bc01d0b627a4d66d5ffce4d686f50771075/chrome/browser/sync/chrome_sync_client.cc
[modify] https://crrev.com/d7e92bc01d0b627a4d66d5ffce4d686f50771075/chrome/browser/sync/test/integration/migration_test.cc
[add] https://crrev.com/d7e92bc01d0b627a4d66d5ffce4d686f50771075/chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc
[add] https://crrev.com/d7e92bc01d0b627a4d66d5ffce4d686f50771075/chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.h
[modify] https://crrev.com/d7e92bc01d0b627a4d66d5ffce4d686f50771075/chrome/browser/ui/app_list/arc/arc_package_syncable_service.cc
[modify] https://crrev.com/d7e92bc01d0b627a4d66d5ffce4d686f50771075/chrome/chrome_browser_ui.gypi

Comment 18 by lgcheng@google.com, Aug 16 2016

Status: Fixed (was: Assigned)
Labels: VerifyIn-54
Status: Verified (was: Fixed)
Chrome OS 54.0.2840.39/8743.42.0

Sign in to add a comment