arc: Fix issue when Arc OptIn is not started on fresh devices.

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 "base/command_line.h"
 #include "base/strings/stringprintf.h"
@@ skipping 210 matching lines @@
   storage_partition_ = content::BrowserContext::GetStoragePartitionForSite(
       profile_, GURL(site_url));
   CHECK(storage_partition_);
 
   // In case UI is disabled we assume that ARC is opted-in.
   if (!IsOptInVerificationDisabled()) {
     if (profile_->GetPrefs()->GetBoolean(prefs::kArcEnabled)) {
       OnOptInPreferenceChanged();
     } else {
       UpdateEnabledStateUMA(false);
-      if (!disable_ui_for_testing && profile_->IsNewProfile()) {
-        PrefServiceSyncableFromProfile(profile_)->AddObserver(this);
-        OnIsSyncingChanged();
-      }
+      PrefServiceSyncableFromProfile(profile_)->AddObserver(this);

[khmel, 2016/03/31 22:19:21]

In one of the recent CLs I changed observer scheme 

from:
PrefChangeRegistrar pref_change_registrar_;
pref_change_registrar_.Add(prefs::kArcEnabled, ...)

To:
PrefServiceSyncableFromProfile(profile_)->
AddSyncedPrefObserver(prefs::kArcEnabled, this);

This was done to handle UMA statics properly in order to filter local vs synced
changes.

However SyncedPrefObserver is not triggered when initial sync occurred (for new
profile).
Workaround is to double check ArcEnabled pref on syncing is ready.

I also tested following case:
Enabled Arc on device, Logout, Disable Arc on another device,
Restart device and change was tracked. In this case, it was synced.

WDYT?

+      OnIsSyncingChanged();
     }
   } else {
     auth_code_.clear();
     StartArc();
   }
 }
 
 void ArcAuthService::OnIsSyncingChanged() {
   syncable_prefs::PrefServiceSyncable* const pref_service_syncable =
       PrefServiceSyncableFromProfile(profile_);
   if (!pref_service_syncable->IsSyncing())
     return;
 
   pref_service_syncable->RemoveObserver(this);
-  if (!profile_->GetPrefs()->HasPrefPath(prefs::kArcEnabled))
+
+  if (profile_->GetPrefs()->GetBoolean(prefs::kArcEnabled))
+    OnOptInPreferenceChanged();
+
+  if (!disable_ui_for_testing && profile_->IsNewProfile() &&

[xiyuan, 2016/03/31 22:32:44]

nit: profile_->IsNewProfile() might not be necessary.

[khmel, 2016/03/31 22:47:34]

I think it might be required.
For example:
User starts first time and we show Notification. User ignores this and
notification is timed out. User starts next time, in this case profile is not
new, but it still has no kArcEnabled path. In this case we would show
notification second time and this is not expected from UI design.

[xiyuan, 2016/03/31 22:50:50]

Okay. That sounds like a valid case.

+      !profile_->GetPrefs()->HasPrefPath(prefs::kArcEnabled)) {
     arc::ArcAuthNotification::Show();
+  }
 }
 
 void ArcAuthService::Shutdown() {
   ShutdownBridgeAndCloseUI();
   if (profile_) {
     syncable_prefs::PrefServiceSyncable* pref_service_syncable =
         PrefServiceSyncableFromProfile(profile_);
     pref_service_syncable->RemoveObserver(this);
     pref_service_syncable->RemoveSyncedPrefObserver(prefs::kArcEnabled, this);
   }
@@ skipping 265 matching lines @@
       return os << kStateFetchingCode;
     case ArcAuthService::State::ACTIVE:
       return os << kStateActive;
     default:
       NOTREACHED();
       return os;
   }
 }
 
 }  // namespace arc