From 4e6625f47ba486747e0994c372c7dc803e26a724 Mon Sep 17 00:00:00 2001 From: "rlp@chromium.org" Date: Wed, 26 Mar 2014 04:28:15 +0000 Subject: [Hotword] Adding wrapper for audio logging preference to correctly return desired value. Since the default value for the audio logging preference is "true", just getting the preference would return "true" (duh). But we only want to return true if the user has set the preference -- that is, they have actually opted in. This adds a wrapper around the call to get the value as used in the private api so that we check first if the pref has been set. BUG=345811 Review URL: https://codereview.chromium.org/208963003 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@259478 0039d316-1c4b-4281-b951-d872f2087c98 --- .../extensions/api/hotword_private/hotword_private_api.cc | 5 +++-- chrome/browser/search/hotword_service.cc | 7 +++++++ chrome/browser/search/hotword_service.h | 4 ++++ chrome/browser/search/hotword_service_unittest.cc | 15 +++++++++++++++ 4 files changed, 29 insertions(+), 2 deletions(-) diff --git a/chrome/browser/extensions/api/hotword_private/hotword_private_api.cc b/chrome/browser/extensions/api/hotword_private/hotword_private_api.cc index e139e85..5b95a55 100644 --- a/chrome/browser/extensions/api/hotword_private/hotword_private_api.cc +++ b/chrome/browser/extensions/api/hotword_private/hotword_private_api.cc @@ -89,8 +89,9 @@ bool HotwordPrivateGetStatusFunction::RunImpl() { PrefService* prefs = GetProfile()->GetPrefs(); result.enabled_set = prefs->HasPrefPath(prefs::kHotwordSearchEnabled); result.enabled = prefs->GetBoolean(prefs::kHotwordSearchEnabled); - result.audio_logging_enabled = - prefs->GetBoolean(prefs::kHotwordAudioLoggingEnabled); + result.audio_logging_enabled = false; + if (hotword_service) + result.audio_logging_enabled = hotword_service->IsOptedIntoAudioLogging(); SetResult(result.ToValue().release()); return true; diff --git a/chrome/browser/search/hotword_service.cc b/chrome/browser/search/hotword_service.cc index ce6d841..e2f16b2 100644 --- a/chrome/browser/search/hotword_service.cc +++ b/chrome/browser/search/hotword_service.cc @@ -207,6 +207,13 @@ bool HotwordService::IsHotwordAllowed() { DoesHotwordSupportLanguage(profile_); } +bool HotwordService::IsOptedIntoAudioLogging() { + // Do not opt the user in if the preference has not been set. + return + profile_->GetPrefs()->HasPrefPath(prefs::kHotwordAudioLoggingEnabled) && + profile_->GetPrefs()->GetBoolean(prefs::kHotwordAudioLoggingEnabled); +} + bool HotwordService::RetryHotwordExtension() { ExtensionService* extension_service = GetExtensionService(profile_); if (!extension_service) diff --git a/chrome/browser/search/hotword_service.h b/chrome/browser/search/hotword_service.h index 9e401d1..def0df0 100644 --- a/chrome/browser/search/hotword_service.h +++ b/chrome/browser/search/hotword_service.h @@ -54,6 +54,10 @@ class HotwordService : public content::NotificationObserver, // and language. virtual bool IsHotwordAllowed(); + // Checks if the user has opted into audio logging. Returns true if the user + // is opted in, false otherwise.. + bool IsOptedIntoAudioLogging(); + // Used in the case of an error with the current hotword extension. Tries // to reload the extension or in the case of failure, tries to re-download it. // Returns true upon successful attempt at reload or if the extension has diff --git a/chrome/browser/search/hotword_service_unittest.cc b/chrome/browser/search/hotword_service_unittest.cc index 4dca993..3749c35 100644 --- a/chrome/browser/search/hotword_service_unittest.cc +++ b/chrome/browser/search/hotword_service_unittest.cc @@ -151,3 +151,18 @@ TEST_F(HotwordServiceTest, IsHotwordAllowedLocale) { SetApplicationLocale(static_cast(otr_profile.get()), "en"); EXPECT_FALSE(HotwordServiceFactory::IsHotwordAllowed(otr_profile.get())); } + +TEST_F(HotwordServiceTest, AudioLoggingPrefSetCorrectly) { + TestingProfile::Builder profile_builder; + scoped_ptr profile = profile_builder.Build(); + + HotwordServiceFactory* hotword_service_factory = + HotwordServiceFactory::GetInstance(); + HotwordService* hotword_service = + hotword_service_factory->GetForProfile(profile.get()); + EXPECT_TRUE(hotword_service != NULL); + + // If it's a fresh profile, although the default value is true, + // it should return false if the preference has never been set. + EXPECT_FALSE(hotword_service->IsOptedIntoAudioLogging()); +} -- cgit v1.1