Record the initial state of the Physical Web at startup

ios/chrome/browser/physical_web/start_physical_web_discovery.mm

 // Copyright 2016 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 "ios/chrome/browser/physical_web/start_physical_web_discovery.h"
 
 #import <CoreLocation/CoreLocation.h>
 
 #include "components/physical_web/data_source/physical_web_data_source.h"
 #include "ios/chrome/browser/application_context.h"
 #import "ios/chrome/browser/experimental_flags.h"
 #import "ios/chrome/browser/geolocation/omnibox_geolocation_config.h"
+#import "ios/chrome/browser/physical_web/ios_chrome_physical_web_data_source.h"
 #include "ios/chrome/browser/physical_web/physical_web_constants.h"
+#import "ios/chrome/browser/physical_web/physical_web_initial_state_recorder.h"
 #include "ios/chrome/browser/pref_names.h"
 #include "url/gurl.h"
 
 void StartPhysicalWebDiscovery(PrefService* pref_service, bool is_incognito) {
   // Do not scan if the Physical Web feature is disabled by a command line flag
   // or Chrome Variations experiment.
   if (!experimental_flags::IsPhysicalWebEnabled()) {
     return;
   }
 
@@ skipping 21 matching lines @@
     if (!is_incognito && location_services_enabled && location_authorized &&
         geolocation_eligible) {
       pref_service->SetInteger(prefs::kIosPhysicalWebEnabled,
                                physical_web::kPhysicalWebOn);
       preference_state =
           pref_service->GetInteger(prefs::kIosPhysicalWebEnabled);
     }
   }
 
   // Scan only if the feature is enabled.
+  PhysicalWebDataSource* data_source =
+      GetApplicationContext()->GetPhysicalWebDataSource();
   if (preference_state == physical_web::kPhysicalWebOn) {
-    GetApplicationContext()->GetPhysicalWebDataSource()->StartDiscovery(true);
+    data_source->StartDiscovery(true);
   } else {
-    GetApplicationContext()->GetPhysicalWebDataSource()->StopDiscovery();
+    data_source->StopDiscovery();
   }
-}
+
+  // Configure the data source to record the initial state of settings relevant
+  // to the Physical Web feature.
+  IOSChromePhysicalWebDataSource* ios_chrome_data_source =
+      (IOSChromePhysicalWebDataSource*)data_source;

[mattreynolds, 2016/11/04 00:23:07]

This is safe since we're in iOS-specific code. The ugly cast is only necessary
because I couldn't find a way to access the PrefService from a global context,
any suggestions?

[sdefresne, 2016/11/10 13:15:42]

Well, the reason you could not find a place with access to the PrefService from
a global context is that there are multiple PrefService in that application. So
you need to have your preferences be global preferences (i.e. registered in the
local_state_ in ApplicationContext) and not in the ChromeBrowserState.

If you do, then ApplicationContext has access to both the PrefService and
IOSChromePhysicalWebDataSource (since it creates both, see
ApplicationContextImpl::CreateLocalState and
ApplicationContextImpl::GetPhysicalWebDataSource).

My suggestion:
1. move the preference to be store in local_state,
2. pass the pref service to the constructor of IOSChromePhysicalWebDataSource,
3. use the PrefChangeRegistrar to be notified of pref change if necessary
otherwise just use the value in initialisation.

[mattreynolds, 2016/11/11 02:26:10]

Thanks! This is much cleaner.

+  if (ios_chrome_data_source) {
+    ios_chrome_data_source->RecordInitialState(pref_service);

[Olivier, 2016/11/10 18:08:08]

This method is called on every BVC switch. Is it expected? This does not seem
"initial"

[mattreynolds, 2016/11/11 02:26:10]

There's a check in PhysicalWebInitialStateRecorder to make sure we don't record
more than once. But I agree, this was confusing. In the latest PS I'm recording
the state in the IOSChromePhysicalWebDataSource constructor so it's clearer that
this should only happen once.

+  }
+}