[Hotword] Sync Audio History pref every 24 hours, when opening chrome://settings and . . .

chrome/browser/ui/webui/options/browser_options_handler.cc

Index: chrome/browser/ui/webui/options/browser_options_handler.cc
diff --git a/chrome/browser/ui/webui/options/browser_options_handler.cc b/chrome/browser/ui/webui/options/browser_options_handler.cc
index 6cd7245380864daf316a1121434b3a04fa3fa538..d59dead61a80ecffe77cabf4f5d2cb5aec4903f4 100644
--- a/chrome/browser/ui/webui/options/browser_options_handler.cc
+++ b/chrome/browser/ui/webui/options/browser_options_handler.cc
@@ -44,6 +44,7 @@
 #include "chrome/browser/profiles/profile_shortcut_manager.h"
 #include "chrome/browser/profiles/profile_window.h"
 #include "chrome/browser/profiles/profiles_state.h"
+#include "chrome/browser/search/hotword_audio_history_handler.h"
 #include "chrome/browser/search/hotword_service.h"
 #include "chrome/browser/search/hotword_service_factory.h"
 #include "chrome/browser/search/search.h"
@@ -1674,6 +1675,15 @@ void BrowserOptionsHandler::ShowCloudPrintDevicesPage(
 
 #endif
 
+void BrowserOptionsHandler::ShowHotwordAudioHistorySection(
+    bool always_on, bool success, bool logging_enabled) {
+  if (success && logging_enabled) {

[Dan Beam, 2014/12/12 20:04:20]

if !success or !logging_enabled should this section be hidden?

[rpetterson, 2014/12/12 20:19:36]

Ideally, if !success we would show an error. I actually have a CL for that ready
to go once I get the final wording on the error from UX.

if (!logging_enabled && success), then we should not show it. Would you
recommend passing that into the function to ensure it stays hidden?

[Dan Beam, 2014/12/12 20:25:46]

i generally ask this type of question whenever I see a show* or hide* function
without a balanced counterpart (e.g. a show without a hide, etc.).

often we optimize for the common case but forget error/uncommon cases (another
reason why I like setVisible instead of show/hide -- because you are likely to
need both if you're "Doing It Right").

[rpetterson, 2014/12/12 21:39:56]

Since I prefer doing things the right way, I've updated the audio history
functions (in browser_options_handler and browser_options) to all be
SetAudioHistorySectionVisible.

+    web_ui()->CallJavascriptFunction("BrowserOptions.showAudioHistorySection",
+                                     base::FundamentalValue(always_on));
+  }
+  // TODO(rlp): Add version with error display.
+}
+
 void BrowserOptionsHandler::HandleRequestHotwordAvailable(
     const base::ListValue* args) {
   Profile* profile = Profile::FromWebUI(web_ui());
@@ -1707,10 +1717,14 @@ void BrowserOptionsHandler::HandleRequestHotwordAvailable(
     // Audio history should be displayed if it's enabled regardless of the
     // hotword error state. An additional message is displayed if always-on
     // hotwording is enabled.
-    if (profile->GetPrefs()->GetBoolean(prefs::kHotwordAudioLoggingEnabled) &&
-        HotwordService::IsExperimentalHotwordingEnabled()) {
-      web_ui()->CallJavascriptFunction("BrowserOptions.showAudioHistorySection",
-                                       base::FundamentalValue(always_on));
+    if (HotwordService::IsExperimentalHotwordingEnabled()) {
+      HotwordService* hotword_service =
+          HotwordServiceFactory::GetForProfile(profile);
+      if (hotword_service)

[Dan Beam, 2014/12/12 20:04:20]

curlies

[rpetterson, 2014/12/12 20:19:36]

Done.

+        hotword_service->GetAudioHistoryHandler()->GetAudioHistoryEnabled(
+            base::Bind(&BrowserOptionsHandler::ShowHotwordAudioHistorySection,
+                       weak_ptr_factory_.GetWeakPtr(),
+                       always_on));
     }
 
     if (!error) {
@@ -1745,8 +1759,6 @@ void BrowserOptionsHandler::HandleLaunchHotwordAudioVerificationApp(
         prefs::kHotwordAudioLoggingEnabled));
 
     launch_mode = HotwordService::RETRAIN;
-    // TODO(rlp): Make sure the Chrome Audio History pref is synced
-    // to the account-level Audio History setting from footprints.
   } else if (profile->GetPrefs()->GetBoolean(
       prefs::kHotwordAudioLoggingEnabled)) {
     DCHECK(!profile->GetPrefs()->GetBoolean(