register voice interaction preference.

chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc

Index: chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc
diff --git a/chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc b/chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc
index 7d2d7cd4f0a666cbc541fcbf515c37d222d8f31d..47cd5d67fb9a9ef5f99f295afbe6312ba5e88f97 100644
--- a/chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc
+++ b/chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc
@@ -168,10 +168,12 @@ ArcVoiceInteractionFrameworkService::ArcVoiceInteractionFrameworkService(
     ArcBridgeService* bridge_service)
     : ArcService(bridge_service), binding_(this) {
   arc_bridge_service()->voice_interaction_framework()->AddObserver(this);
+  ArcSessionManager::Get()->AddObserver(this);
 }
 
 ArcVoiceInteractionFrameworkService::~ArcVoiceInteractionFrameworkService() {
   arc_bridge_service()->voice_interaction_framework()->RemoveObserver(this);
+  ArcSessionManager::Get()->RemoveObserver(this);
 }
 
 void ArcVoiceInteractionFrameworkService::OnInstanceReady() {
@@ -319,6 +321,13 @@ void ArcVoiceInteractionFrameworkService::HideMetalayer() {
   SetMetalayerVisibility(false);
 }
 
+void ArcVoiceInteractionFrameworkService::OnArcPlayStoreEnabledChanged(
+    bool enabled) {
+  if (!enabled)

[Luis Héctor Chávez, 2017/07/10 16:53:02]

nit: you can't elide braces since the body is not a one-liner.

alternatively, use the guard-clause pattern if it makes sense:

if (enabled)
  return;
// rest

[Muyuan, 2017/07/14 20:52:13]

Done.

+    ProfileManager::GetActiveUserProfile()->GetPrefs()->SetBoolean(
+        prefs::kVoiceInteractionPrefSynced, false);
+}

[Luis Héctor Chávez, 2017/07/10 16:53:02]

Should we also clear kVoiceInteraction{Enabled,ContextEnabled} here? If so,
maybe you can have the same function mentioned in L399 and call it here, too.

[Muyuan, 2017/07/14 20:52:13]

It does not matter as kVoiceInteractionPrefSynced is the master switch. But I
modified the code to keep consistency. 

+
 void ArcVoiceInteractionFrameworkService::StartVoiceInteractionSetupWizard() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   arc::mojom::VoiceInteractionFrameworkInstance* framework_instance =
@@ -377,6 +386,30 @@ void ArcVoiceInteractionFrameworkService::SetVoiceInteractionContextEnabled(
   framework_instance->SetVoiceInteractionContextEnabled(enable);
 }
 
+void ArcVoiceInteractionFrameworkService::UpdateVoiceInteractionPrefs() {
+  if (!ProfileManager::GetActiveUserProfile()->GetPrefs()->GetBoolean(

[Luis Héctor Chávez, 2017/07/10 16:53:02]

nit: use guard-clause pattern:

if (ProfileManager::GetActiveUserProfile()->GetPrefs()->GetBoolean(
        prefs::kVoiceInteractionPrefSynced)) {
  return;
}
// rest.

[Muyuan, 2017/07/14 20:52:13]

Done.

+          prefs::kVoiceInteractionPrefSynced)) {
+    mojom::VoiceInteractionFrameworkInstance* framework_instance =
+        ARC_GET_INSTANCE_FOR_METHOD(
+            arc_bridge_service()->voice_interaction_framework(),
+            GetVoiceInteractionSettings);
+    if (!framework_instance)
+      return;
+    framework_instance->GetVoiceInteractionSettings(
+        base::Bind([](mojom::VoiceInteractionStatusPtr status) {

[Luis Héctor Chávez, 2017/07/10 16:53:02]

nit: make this a standalone function in the anonymous namespace.

[stevenjb, 2017/07/10 17:57:07]

+1. We generally only use function objects like this for trivial functions (e.g.
for sorting or finding).

[Muyuan, 2017/07/14 20:52:13]

Done.

+          auto* prefs = ProfileManager::GetActiveUserProfile()->GetPrefs();

[stevenjb, 2017/07/10 17:57:07]

Prefer explicit types when the context isn't completely clear (i.e. |prefs| is
actually 'PrefService', not 'Prefs' as implied by the getter). Also, even if the
getter were GetPrefService we typically provide the type for clarity and reserve
auto for iterators and places where the type is very clear like with
MakeUnique<>).

[Muyuan, 2017/07/14 20:52:14]

Done.

+          prefs->SetBoolean(prefs::kVoiceInteractionPrefSynced,
+                            status->defined);
+          if (!status->defined)
+            return;
+          prefs->SetBoolean(prefs::kVoiceInteractionEnabled,
+                            status->voice_interaction_enabled);
+          prefs->SetBoolean(prefs::kVoiceInteractionContextEnabled,
+                            status->context_enabled);
+        }));
+  }
+}
+
 void ArcVoiceInteractionFrameworkService::StartSessionFromUserInteraction(
     const gfx::Rect& rect) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);