New issue
Advanced search Search tips

Issue 822459 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Possible race between ArcSettingsService::OnConnectionReady and ArcSessionManager::OnProvisioningFinished

Project Member Reported by jhorwich@chromium.org, Mar 15 2018

Issue description

ArcSettingsServiceImpl's ctor attemtps to register an observer for ArcSessionManager - to get OnArcInitialStart callback.

Said callback is typically called from ArcSessionManager::OnProvisioningFinished, e.g. on first sign-in of a new ARC++ instance.

ArcSettingsServiceImpl's ctor is called in response to ArcSettingsService::OnConnectionReady() - I haven't dug into what exactly triggers this.

I've been seeing cases where ArcSessionManager::OnProvisioningFinished is called before ArcSettingsService::OnConnectionReady() is happening, which results in ArcSettingsServiceImpl::OnArcInitialStart not being called for the ARC++ instance.

I've seen these cases when turning ARC++ off then on again within the same Chrome OS user session (e.g. login to chrome os, turn on ARC++, turn it off, turn it on again), or in cases where ARC++ was off when logging in an existing Chrome OS user, then turning ARC++ on.
 

Comment 1 by khmel@chromium.org, Mar 16 2018

Status: Started (was: Untriaged)

Comment 2 by khmel@chromium.org, Mar 16 2018

crrev.com/c/967069
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 9 2018

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

commit 0d31fd73f0972395661f1a47e1b5a7019f65ebdf
Author: khmel@google.com <khmel@google.com>
Date: Mon Apr 09 17:07:44 2018

arc: Fix race condition applying location and B&R settings.

This fix race condition when settings are driven by mojo instance
creation for intent helper and onArcInitialStart is called due the
communication via auth mojom. That leads to case when onArcInitialStart
is not called for arc settings and these settings are not applied on
Android side.

Bug:  822459 
Test: Unit tests added. Manually on device. Simulate race condition,
observer that all consumers of onArcInitialStart are called. Only
settings component has such race. Other consumers are created on
session start and do not have this race.

Change-Id: Iab0a5fe53d55c3e84fbe532cf7c8d5b720e67bcb
Reviewed-on: https://chromium-review.googlesource.com/967069
Commit-Queue: Yury Khmel <khmel@google.com>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549204}
[modify] https://crrev.com/0d31fd73f0972395661f1a47e1b5a7019f65ebdf/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/0d31fd73f0972395661f1a47e1b5a7019f65ebdf/chrome/browser/chromeos/arc/arc_session_manager.h
[modify] https://crrev.com/0d31fd73f0972395661f1a47e1b5a7019f65ebdf/chrome/browser/chromeos/arc/arc_session_manager_unittest.cc
[modify] https://crrev.com/0d31fd73f0972395661f1a47e1b5a7019f65ebdf/chrome/browser/chromeos/arc/intent_helper/arc_settings_service.cc
[modify] https://crrev.com/0d31fd73f0972395661f1a47e1b5a7019f65ebdf/chrome/browser/chromeos/arc/intent_helper/arc_settings_service.h
[modify] https://crrev.com/0d31fd73f0972395661f1a47e1b5a7019f65ebdf/chrome/browser/chromeos/arc/intent_helper/arc_settings_service_browsertest.cc
[add] https://crrev.com/0d31fd73f0972395661f1a47e1b5a7019f65ebdf/chrome/browser/chromeos/arc/intent_helper/arc_settings_service_unittest.cc
[modify] https://crrev.com/0d31fd73f0972395661f1a47e1b5a7019f65ebdf/components/arc/arc_prefs.cc
[modify] https://crrev.com/0d31fd73f0972395661f1a47e1b5a7019f65ebdf/components/arc/arc_prefs.h
[modify] https://crrev.com/0d31fd73f0972395661f1a47e1b5a7019f65ebdf/components/arc/test/fake_backup_settings_instance.cc
[modify] https://crrev.com/0d31fd73f0972395661f1a47e1b5a7019f65ebdf/components/arc/test/fake_backup_settings_instance.h
[modify] https://crrev.com/0d31fd73f0972395661f1a47e1b5a7019f65ebdf/components/arc/test/fake_intent_helper_instance.cc
[modify] https://crrev.com/0d31fd73f0972395661f1a47e1b5a7019f65ebdf/components/arc/test/fake_intent_helper_instance.h

Comment 4 by khmel@chromium.org, Apr 11 2018

Status: Fixed (was: Started)


Sign in to add a comment