[Hotword] Delay checking for audio devices until needed.

chrome/browser/search/hotword_service_factory.cc

 // Copyright 2013 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/search/hotword_service_factory.h"
 
 #include "base/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search/hotword_service.h"
 #include "chrome/common/pref_names.h"
@@ skipping 34 matching lines @@
   if (!hotword_service)
     return 0;
   return hotword_service->error_message();
 }
 
 // static
 bool HotwordServiceFactory::IsMicrophoneAvailable() {
   return GetInstance()->microphone_available();
 }
 
+// static
+bool HotwordServiceFactory::IsAudioDeviceStateUpdated() {
+  return GetInstance()->audio_device_state_updated();
+}
+
 HotwordServiceFactory::HotwordServiceFactory()
     : BrowserContextKeyedServiceFactory(
         "HotwordService",
         BrowserContextDependencyManager::GetInstance()),
-      microphone_available_(false) {
+      microphone_available_(false),
+      audio_device_state_updated_(false) {
   // No dependencies.
 
   // Register with the device observer list to update the microphone
   // availability.
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       base::Bind(&HotwordServiceFactory::InitializeMicrophoneObserver,
                  base::Unretained(this)));
 }
 
 HotwordServiceFactory::~HotwordServiceFactory() {
 }
 
 void HotwordServiceFactory::InitializeMicrophoneObserver() {
   MediaCaptureDevicesDispatcher::GetInstance()->AddObserver(this);
 }
 
 void HotwordServiceFactory::OnUpdateAudioDevices(
     const content::MediaStreamDevices& devices) {
   microphone_available_ = !devices.empty();
+  audio_device_state_updated_ = true;
 }
 
 void HotwordServiceFactory::UpdateMicrophoneState() {
   // In order to trigger the monitor, just call getAudioCaptureDevices.
   content::MediaStreamDevices devices =
-      MediaCaptureDevicesDispatcher::GetInstance()->GetAudioCaptureDevices();
-
-  // If the monitor had not previously been started, there may be 0 devices
-  // even if that is not accurate. However, we can update the microphone
-  // availability state now. Either the number of devices will be correct or
-  // we know that the call above will start the monitor and the microphone
-  // state will be updated very soon and call OnUpdateAudioDevices.
-  OnUpdateAudioDevices(devices);
+    MediaCaptureDevicesDispatcher::GetInstance()->GetAudioCaptureDevices();

[Anand Mistry (off Chromium), 2014/10/20 07:09:05]

I think you've introduced a race here. If this factory is created after devices
have been enumerated (Singletons are lazy), then the microphone state won't be
updated until it changes (since the observer is registered after enumeration),
which may be never. I think you need the call to OnUpdateAudioDevices(). It also
looks like there's no way to distinguish between the "no devices" and "not yet
enumerated" states, so the whole state_updated_ thing is moot.

[rpetterson, 2014/10/20 17:25:07]

I disagree. The factory is initialized on startup with a call to GetInstance()
from chrome_browser_main_extra_parts_profiles.cc which gets called from
PreMainMessageLoopRunImpl(). Since the factory is global, this will cover all
profiles. Because of the startup delay this call causes (the root of this bug),
the microphone state is not updated during startup so it must happen after
factory initialization.

The alternative is to return the call to OnUpdateAudioDevices and leave my
existing logic in hotword_service. This will work as well because on the first
call (before we have updated services) |audio_device_state_updated_| will be
false and we won't include the audio device state in the logic calculation for
IsServiceAvailable. After that we will have kicked off the microphone state
update, so the next time the user loads chrome://settings, it should be fine.
The first time this is called -- before the update has occurred -- is the reason
for putting the check for IsAudioDeviceStateUpdated first and then kicking off
the update.

[Anand Mistry (off Chromium), 2014/10/20 21:22:54]

Thanks for the explanation. This makes a lot more sense.

 }
 
 void HotwordServiceFactory::RegisterProfilePrefs(
     user_prefs::PrefRegistrySyncable* prefs) {
   prefs->RegisterBooleanPref(prefs::kHotwordSearchEnabled,
                              false,
                              user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
   // Although this is default true, users will not send back information to
   // Google unless they have opted-in to Hotwording at which point they must
   // also confirm that they wish this preference to be true or opt out of it.
   prefs->RegisterBooleanPref(prefs::kHotwordAudioLoggingEnabled,
                              true,
                              user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
   prefs->RegisterStringPref(prefs::kHotwordPreviousLanguage,
                             std::string(),
                             user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
   prefs->RegisterBooleanPref(prefs::kHotwordAlwaysOnSearchEnabled,
                              false,
                              user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
 }
 
 KeyedService* HotwordServiceFactory::BuildServiceInstanceFor(
     BrowserContext* context) const {
   return new HotwordService(Profile::FromBrowserContext(context));
 }