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

Issue 636218 link

Starred by 1 user
Status: Started
Owner:
hidehiko@chromium.org
Cc:
█ mitsuji@chromium.org
khmel@chromium.org
elijahtaylor@chromium.org
█ dspaid@chromium.org
█ uekawa@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Unexpected data removal on crash.

Project Member Reported by hidehiko@chromium.org, Aug 10 2016 
There is a bug about unexpected user data removal in ARC++ start up sequence.

Branched from  crbug.com/633258 , to mention about the specific bug case clearly.

- Enable ARC.
  - Setting kArcEnabled: false -> true
- Cancel to start ARC, during ToS is showining.
  - Setting kArcEnabled: true -> false
    - Along with that, ArcUserDataService set a flag to remove the
      user data on ABS stop.
- Re-enable ARC.
  - Continue to authorization phase.
- On ARC crash.
  - ARC stops here. The notification is sent to ArcUserDataService.
  - ArcUserDataService removes the user data.

crrev.com/2209173002 is the short-term workaround.
The proper fix will be a part of  crbug.com/633258 

I think this is M53 cherry-pick target.
Hiro, Yoshi, could you check if this is serious enough to be fixed in M53 from your PM perspective?

Comment 1 by hidehiko@chromium.org, Aug 15 2016

Labels: Merge-Request-53
https://codereview.chromium.org/2209173002/ is the fix.
(the bug id was dropped by mistake).

Merge request. Hiro-san, please double check.

Comment 2 by mitsuji@chromium.org, Aug 16 2016 

Yes will ping via email. Thanks!

Comment 3 by keta...@chromium.org, Aug 16 2016

Labels: -Merge-Request-53 Merge-Approved-53
Approving merge to M53 cros.

Comment 4 by mitsuji@chromium.org, Aug 16 2016 

Yes! Thank you Ketaki!!
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 18 2016

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

commit d7ca35a59c7bd8c7ddd78d7682458e48e6b4e934
Author: Hidehiko Abe <hidehiko@chromium.org>
Date: Thu Aug 18 04:20:51 2016

Migrate ArcUserDataService into ArcAuthService.

There are race problems around data removing.
One of the cause is that state management is done in
ArcAuthService and ArcUserDataService separately.

This CL migrates ArcUserDataService into ArcAuthService
so that we can remove the data when necessary.

BUG= 633258 , 636218
TEST=Ran manually.
     - "Enable -> ARC boot wait -> Disable" triggers data removal after
       ARC stop.
     - "Enable -> Disable before ARC boot" triggers immediate data
       removal.
     - "Enable -> ARC boot wait -> Kill ARC instance" does not trigger
       data removal.
R=dspaid@chromium.org, lhchavez@chromium.org
TBR=khmel@chromium.org, lhchavez@chromium.org, stevenjb@chromium.org

Review-Url: https://codereview.chromium.org/2209173002
Cr-Commit-Position: refs/heads/master@{#411923}
(cherry picked from commit 6d95ad318e992261c45f5a88a0a599663a6d66ae)

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

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

[modify] https://crrev.com/d7ca35a59c7bd8c7ddd78d7682458e48e6b4e934/chrome/browser/chromeos/arc/arc_auth_service.cc
[modify] https://crrev.com/d7ca35a59c7bd8c7ddd78d7682458e48e6b4e934/chrome/browser/chromeos/arc/arc_auth_service.h
[modify] https://crrev.com/d7ca35a59c7bd8c7ddd78d7682458e48e6b4e934/chrome/browser/chromeos/arc/arc_auth_service_unittest.cc
[modify] https://crrev.com/d7ca35a59c7bd8c7ddd78d7682458e48e6b4e934/chrome/browser/chromeos/arc/arc_enterprise_reporting_service.cc
[modify] https://crrev.com/d7ca35a59c7bd8c7ddd78d7682458e48e6b4e934/chrome/browser/chromeos/arc/arc_enterprise_reporting_service.h
[modify] https://crrev.com/d7ca35a59c7bd8c7ddd78d7682458e48e6b4e934/chrome/browser/ui/app_list/arc/arc_app_test.cc
[modify] https://crrev.com/d7ca35a59c7bd8c7ddd78d7682458e48e6b4e934/chrome/browser/ui/app_list/arc/arc_app_test.h
[modify] https://crrev.com/d7ca35a59c7bd8c7ddd78d7682458e48e6b4e934/components/arc/arc_service_manager.cc
[modify] https://crrev.com/d7ca35a59c7bd8c7ddd78d7682458e48e6b4e934/components/arc/arc_service_manager.h

Comment 6 by hidehiko@chromium.org, Aug 19 2016

Labels: -merge-merged-2785 Merge-Approved-53
Unfortunately, the cherry-pick was reverted, due to the build failure because of the diverse between release branch head and current ToT.
Relanding now, so manually reverted the label to merge-approved for tracking purpose.
Project Member

Comment 7 by sheriffbot@chromium.org, Aug 20 2016 

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 8 by sheriffbot@chromium.org, Aug 23 2016 

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 9 by bugdroid1@chromium.org, Aug 25 2016

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

commit 61bc6c8dc688551cc51ef7c32515816d4f031bc0
Author: hidehiko <hidehiko@chromium.org>
Date: Thu Aug 25 02:05:22 2016

Migrate ArcUserDataService into ArcAuthService.

There are race problems around data removing.
One of the cause is that state management is done in
ArcAuthService and ArcUserDataService separately.

This CL migrates ArcUserDataService into ArcAuthService
so that we can remove the data when necessary.

BUG= 633258 , 636218
TEST=Ran manually.
     - "Enable -> ARC boot wait -> Disable" triggers data removal after
       ARC stop.
     - "Enable -> Disable before ARC boot" triggers immediate data
       removal.
     - "Enable -> ARC boot wait -> Kill ARC instance" does not trigger
       data removal.
NOTRY=true
NOPRESUBMIT=true
TBR=dspaid@chromium.org, lhchavez@chromium.org, stevenjb@chromium.org

Review-Url: https://codereview.chromium.org/2209173002
Cr-Commit-Position: refs/heads/master@{#411923}
(cherry picked from commit 6d95ad318e992261c45f5a88a0a599663a6d66ae)

Review-Url: https://codereview.chromium.org/2261653002
Cr-Commit-Position: refs/branch-heads/2785@{#746}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

[modify] https://crrev.com/61bc6c8dc688551cc51ef7c32515816d4f031bc0/chrome/browser/chromeos/arc/arc_auth_service.cc
[modify] https://crrev.com/61bc6c8dc688551cc51ef7c32515816d4f031bc0/chrome/browser/chromeos/arc/arc_auth_service.h
[modify] https://crrev.com/61bc6c8dc688551cc51ef7c32515816d4f031bc0/chrome/browser/chromeos/arc/arc_auth_service_unittest.cc
[modify] https://crrev.com/61bc6c8dc688551cc51ef7c32515816d4f031bc0/chrome/browser/chromeos/arc/arc_enterprise_reporting_service.cc
[modify] https://crrev.com/61bc6c8dc688551cc51ef7c32515816d4f031bc0/chrome/browser/chromeos/arc/arc_enterprise_reporting_service.h
[modify] https://crrev.com/61bc6c8dc688551cc51ef7c32515816d4f031bc0/chrome/browser/ui/app_list/arc/arc_app_test.cc
[modify] https://crrev.com/61bc6c8dc688551cc51ef7c32515816d4f031bc0/chrome/browser/ui/app_list/arc/arc_app_test.h
[modify] https://crrev.com/61bc6c8dc688551cc51ef7c32515816d4f031bc0/components/arc/arc_service_manager.cc
[modify] https://crrev.com/61bc6c8dc688551cc51ef7c32515816d4f031bc0/components/arc/arc_service_manager.h
Project Member

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

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

commit e0eb7942096fb4945f5d5d3ed23632148d5348bd
Author: hidehiko <hidehiko@chromium.org>
Date: Tue Oct 25 04:56:29 2016

Extract ArcSupportMessageHost.

Currently ArcSupportHost is the ARC's implementation of
NativeMessageHost. Because of the system structure, the instance
is created and managed by the module outside of ARC.

This CL extracts the implementation of that part into
ArcSupportMessageHost so that the ArcSupportHost, which will
be the main interface to show UI things for ARC, can be managed
along with the ARC's life-time.

There is a plan to extract non authorization related part from
ArcAuthSerivce. Meanwhile, the instance is managed by
ArcAuthService, temporarily.

BUG=657687, 636218,  633258 , b/31079732
TEST=Ran on test device. Ran trybots.

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

[modify] https://crrev.com/e0eb7942096fb4945f5d5d3ed23632148d5348bd/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/e0eb7942096fb4945f5d5d3ed23632148d5348bd/chrome/browser/chromeos/arc/arc_auth_service.cc
[modify] https://crrev.com/e0eb7942096fb4945f5d5d3ed23632148d5348bd/chrome/browser/chromeos/arc/arc_auth_service.h
[modify] https://crrev.com/e0eb7942096fb4945f5d5d3ed23632148d5348bd/chrome/browser/chromeos/arc/arc_auth_service_unittest.cc
[modify] https://crrev.com/e0eb7942096fb4945f5d5d3ed23632148d5348bd/chrome/browser/chromeos/arc/arc_support_host.cc
[modify] https://crrev.com/e0eb7942096fb4945f5d5d3ed23632148d5348bd/chrome/browser/chromeos/arc/arc_support_host.h
[add] https://crrev.com/e0eb7942096fb4945f5d5d3ed23632148d5348bd/chrome/browser/chromeos/arc/extensions/arc_support_message_host.cc
[add] https://crrev.com/e0eb7942096fb4945f5d5d3ed23632148d5348bd/chrome/browser/chromeos/arc/extensions/arc_support_message_host.h
[modify] https://crrev.com/e0eb7942096fb4945f5d5d3ed23632148d5348bd/chrome/browser/extensions/api/messaging/native_message_host_chromeos.cc
Project Member

Comment 11 by bugdroid1@chromium.org, Mar 1 2017 

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

commit ae82e3ad7862628f263303d750ee7e1ac5775219
Author: hidehiko <hidehiko@chromium.org>
Date: Wed Mar 01 20:05:39 2017

Do nothing on OnSessionStopped if ARC is being restarted.

ArcSessionManager::OnSessionStopped() implements operations
running on ARC session is stopped.
However, ArcSessionRunner supports automatic restarting,
and the function does not expect such cases, so that state
machine gets into a wrong state.

Along with the behavior change, unifies the observers
for ArcSessionManager.

BUG=657687
BUG=636218
BUG= 633258 
TEST=Ran bots.

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

[modify] https://crrev.com/ae82e3ad7862628f263303d750ee7e1ac5775219/chrome/browser/chromeos/arc/arc_session_manager.cc
[modify] https://crrev.com/ae82e3ad7862628f263303d750ee7e1ac5775219/chrome/browser/chromeos/arc/arc_session_manager.h
[modify] https://crrev.com/ae82e3ad7862628f263303d750ee7e1ac5775219/chrome/browser/chromeos/arc/notification/arc_boot_error_notification.cc
[modify] https://crrev.com/ae82e3ad7862628f263303d750ee7e1ac5775219/chrome/browser/chromeos/arc/notification/arc_boot_error_notification.h
[modify] https://crrev.com/ae82e3ad7862628f263303d750ee7e1ac5775219/components/arc/BUILD.gn
[modify] https://crrev.com/ae82e3ad7862628f263303d750ee7e1ac5775219/components/arc/arc_session.cc
[modify] https://crrev.com/ae82e3ad7862628f263303d750ee7e1ac5775219/components/arc/arc_session.h
[delete] https://crrev.com/b2d4d4ba555e4658e5b2ea2ba4cbd9b95ac84442/components/arc/arc_session_observer.cc
[delete] https://crrev.com/b2d4d4ba555e4658e5b2ea2ba4cbd9b95ac84442/components/arc/arc_session_observer.h
[modify] https://crrev.com/ae82e3ad7862628f263303d750ee7e1ac5775219/components/arc/arc_session_runner.cc
[modify] https://crrev.com/ae82e3ad7862628f263303d750ee7e1ac5775219/components/arc/arc_session_runner.h
[modify] https://crrev.com/ae82e3ad7862628f263303d750ee7e1ac5775219/components/arc/arc_session_runner_unittest.cc
[add] https://crrev.com/ae82e3ad7862628f263303d750ee7e1ac5775219/components/arc/arc_stop_reason.cc
[add] https://crrev.com/ae82e3ad7862628f263303d750ee7e1ac5775219/components/arc/arc_stop_reason.h
[modify] https://crrev.com/ae82e3ad7862628f263303d750ee7e1ac5775219/components/arc/test/fake_arc_session.cc
[modify] https://crrev.com/ae82e3ad7862628f263303d750ee7e1ac5775219/components/arc/test/fake_arc_session.h

Comment 12 by yoshi@chromium.org, Jan 18 2018

Cc: -yoshiat@chromium.org

Comment 13 by zalcorn@chromium.org, Sep 28 

Triage nag: This Chrome OS bug has an owner but no component. Please add a component so that this can be tracked by the relevant team.

Comment 14 by weifangsun@chromium.org, Nov 8 

<UI triage> Bug owners, please add the appropriate component to your bug. Thanks!

Comment 15 by weifangsun@chromium.org, Nov 8 

<UI triage> Bug owners, please add the appropriate component to your bug. Thanks!

Comment 16 by uekawa@google.com, Nov 12

Components: Platform>Apps>ARC
This looks like we should be able to close this now?

Comment 17 by lhchavez@chromium.org, Jan 7

Cc: -lhchavez@chromium.org
►

Sign in to add a comment