arc: Implement safe access to ArcAuthService.

chrome/browser/chromeos/arc/arc_auth_service.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/arc/arc_auth_service.h"
 
 #include <utility>
 
 #include "ash/shelf/shelf_delegate.h"
 #include "ash/shell.h"
@@ skipping 323 matching lines @@
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   return profile_ != nullptr;
 }
 
 void ArcAuthService::OnPrimaryUserProfilePrepared(Profile* profile) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(profile && profile != profile_);
 
   Shutdown();
 
-  profile_ = profile;

[khmel, 2016/07/14 23:17:10]

Actually this incorrect. It was changed while ago (by myself :)). I am fixing
current logic now. There is only ArcHostSupport who uses profile() from
ArcAuthService. I updated ArcHostSupport for safe work.

-  SetState(State::STOPPED);
-
   if (!IsAllowedForProfile(profile))
     return;
 
   // TODO(khmel): Move this to IsAllowedForProfile.
   if (IsArcDisabledForEnterprise() && IsAccountManaged(profile)) {
     VLOG(2) << "Enterprise users are not supported in ARC.";
     return;
   }
 
+  profile_ = profile;
+  SetState(State::STOPPED);
+
   PrefServiceSyncableFromProfile(profile_)->AddSyncedPrefObserver(
       prefs::kArcEnabled, this);
 
   context_.reset(new ArcAuthContext(this, profile_));
 
   // In case UI is disabled we assume that ARC is opted-in.
   if (IsOptInVerificationDisabled()) {
     auth_code_.clear();
     StartArc();
     return;
@@ skipping 263 matching lines @@
 }
 
 bool ArcAuthService::IsArcManaged() const {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(profile_);
   return profile_->GetPrefs()->IsManagedPreference(prefs::kArcEnabled);
 }
 
 bool ArcAuthService::IsArcEnabled() const {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  if (!IsAllowed())

[khmel, 2016/07/14 23:17:11]

There are several calls to IsArcEnabled without proper call to IsArcAllowed. I
think usage is confusing and it probably makes sense to add extra check
IsAllowed here instead of patching every usage. Logically if arc is not allowed
it is not enabled as well.

[xiyuan, 2016/07/15 17:12:52]

Makes sense.

+    return false;
+
   DCHECK(profile_);
   return profile_->GetPrefs()->GetBoolean(prefs::kArcEnabled);
 }
 
 void ArcAuthService::EnableArc() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(profile_);
 
   if (IsArcEnabled()) {
     OnOptInPreferenceChanged();
@@ skipping 101 matching lines @@
       return os << kStateFetchingCode;
     case ArcAuthService::State::ACTIVE:
       return os << kStateActive;
     default:
       NOTREACHED();
       return os;
   }
 }
 
 }  // namespace arc