[ios] Adds OmahaServiceProvider::Stop().

This will be used to properly remove observers during browser shutdown,
so that the omaha service will not DCHECK() due to leaving observers
registered.

Additionally renames methods from Initialize/ShutDown to Start/Stop.

BUG=996148

Change-Id: Ia0e23834f2ec3e11f4e04db2038cc2e144caa2b0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1933735
Commit-Queue: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: Mark Cogan <marq@chromium.org>
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#747302}

ios/chrome/app/main_controller.mm

@@ -93,6 +93,7 @@
 #import "ios/chrome/browser/metrics/previous_session_info.h"
 #import "ios/chrome/browser/net/cookie_util.h"
 #include "ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.h"
+#import "ios/chrome/browser/omaha/omaha_service.h"
 #include "ios/chrome/browser/pref_names.h"
 #import "ios/chrome/browser/reading_list/reading_list_download_service.h"
 #import "ios/chrome/browser/reading_list/reading_list_download_service_factory.h"
@@ -728,6 +729,8 @@ void MainControllerAuthenticationServiceDelegate::ClearBrowsingData(
 
   // End of per-window code.
 
+  OmahaService::Stop();
+
   [_spotlightManager shutdown];
   _spotlightManager = nil;
 

ios/chrome/browser/omaha/omaha_service.h

@@ -44,6 +44,9 @@ class OmahaService {
                         pending_url_loader_factory,
                     const UpgradeRecommendedCallback& callback);
 
+  // Stops the service in preparation for browser shutdown.
+  static void Stop();
+
   // Returns debug information about the omaha service.
   static void GetDebugInformation(
       const base::Callback<void(base::DictionaryValue*)> callback);
@@ -77,13 +80,20 @@ class OmahaService {
     USAGE_PING,
   };
 
-  // Initialize the timer. Used on startup.
-  void Initialize();
+  // Starts the service. Called on startup.
+  void StartInternal();
+
+  // Stops the service in preparation for browser shutdown.
+  void StopInternal();
 
+  // URL loader completion callback.
   void OnURLLoadComplete(std::unique_ptr<std::string> response_body);
 
-  // Raw GetInstance method. Necessary for using singletons. Returns nullptr if
-  // Omaha should not be enabled for this build variant.
+  // Returns whether Omaha is enabled for this build variant.
+  static bool IsEnabled();
+
+  // Raw GetInstance method. Necessary for using singletons. This method must
+  // only be called if |IsEnabled()| returns true.
   static OmahaService* GetInstance();
 
   // Private constructor, only used by the singleton.
@@ -167,7 +177,7 @@ class OmahaService {
   // Whether to schedule pings. This is only false for tests.
   const bool schedule_;
 
-  // The install date of the application.  This is fetched in |Initialize| on
+  // The install date of the application.  This is fetched in |StartInternal| on
   // the main thread and cached for use on the IO thread.
   int64_t application_install_date_;
 

ios/chrome/browser/omaha/omaha_service.mm

@@ -281,18 +281,27 @@ class XmlWrapper : public OmahaXmlWriter {
 @end
 
 // static
-OmahaService* OmahaService::GetInstance() {
+bool OmahaService::IsEnabled() {
 #if BUILDFLAG(GOOGLE_CHROME_BRANDING)
-  static base::NoDestructor<OmahaService> instance;
-
-  if (tests_hook::DisableUpdateService())
-    return nullptr;
-  return instance.get();
+  return !tests_hook::DisableUpdateService();
 #else
-  return nullptr;
+  return false;
 #endif
 }
 
+// static
+OmahaService* OmahaService::GetInstance() {
+  // base::NoDestructor creates its OmahaService as soon as this method is
+  // entered for the first time. In build variants where Omaha is disabled, that
+  // can lead to a scenario where the OmahaService is started but never
+  // stopped. Guard against this by ensuring that GetInstance() can only be
+  // called when Omaha is enabled.
+  DCHECK(IsEnabled());
+
+  static base::NoDestructor<OmahaService> instance;
+  return instance.get();
+}
+
 // static
 void OmahaService::Start(std::unique_ptr<network::PendingSharedURLLoaderFactory>
                              pending_url_loader_factory,
@@ -300,11 +309,11 @@ void OmahaService::Start(std::unique_ptr<network::PendingSharedURLLoaderFactory>
   DCHECK(pending_url_loader_factory);
   DCHECK(!callback.is_null());
 
-  OmahaService* service = GetInstance();
-  if (!service) {
+  if (!OmahaService::IsEnabled()) {
     return;
   }
 
+  OmahaService* service = GetInstance();
   service->set_upgrade_recommended_callback(callback);
   // This should only be called once.
   DCHECK(!service->pending_url_loader_factory_ ||
@@ -316,25 +325,35 @@ void OmahaService::Start(std::unique_ptr<network::PendingSharedURLLoaderFactory>
                                 base::Unretained(service)));
 }
 
+// static
+void OmahaService::Stop() {
+  if (!OmahaService::IsEnabled()) {
+    return;
+  }
+
+  OmahaService* service = GetInstance();
+  service->StopInternal();
+}
+
 OmahaService::OmahaService()
     : schedule_(true),
       application_install_date_(0),
       sending_install_event_(false) {
-  Initialize();
+  StartInternal();
 }
 
 OmahaService::OmahaService(bool schedule)
     : schedule_(schedule),
       application_install_date_(0),
       sending_install_event_(false) {
-  Initialize();
+  StartInternal();
 }
 
 OmahaService::~OmahaService() {}
 
-void OmahaService::Initialize() {
-  // Initialize the provider at the same time as the rest of the service.
-  ios::GetChromeBrowserProvider()->GetOmahaServiceProvider()->Initialize();
+void OmahaService::StartInternal() {
+  // Start the provider at the same time as the rest of the service.
+  ios::GetChromeBrowserProvider()->GetOmahaServiceProvider()->Start();
 
   NSUserDefaults* defaults = [NSUserDefaults standardUserDefaults];
   next_tries_time_ = base::Time::FromCFAbsoluteTime(
@@ -390,20 +409,24 @@ void OmahaService::Initialize() {
     PersistStates();
 }
 
+void OmahaService::StopInternal() {
+  ios::GetChromeBrowserProvider()->GetOmahaServiceProvider()->Stop();
+}
+
 // static
 void OmahaService::GetDebugInformation(
     const base::Callback<void(base::DictionaryValue*)> callback) {
-  OmahaService* service = GetInstance();
+  if (OmahaService::IsEnabled()) {
+    OmahaService* service = GetInstance();
+    base::PostTask(FROM_HERE, {web::WebThread::IO},
+                   base::BindOnce(&OmahaService::GetDebugInformationOnIOThread,
+                                  base::Unretained(service), callback));
 
-  if (!service) {
+  } else {
     auto result = std::make_unique<base::DictionaryValue>();
     // Invoke the callback with an empty response.
     base::PostTask(FROM_HERE, {web::WebThread::UI},
                    base::BindOnce(callback, base::Owned(result.release())));
-  } else {
-    base::PostTask(FROM_HERE, {web::WebThread::IO},
-                   base::BindOnce(&OmahaService::GetDebugInformationOnIOThread,
-                                  base::Unretained(service), callback));
   }
 }
 

ios/public/provider/chrome/browser/omaha/omaha_service_provider.h

@@ -19,9 +19,16 @@ class OmahaServiceProvider {
   OmahaServiceProvider();
   virtual ~OmahaServiceProvider();
 
-  // Initializes the provider.  This method will be called on the UI thread.
+  // Starts the provider.  This method will be called on the UI thread.
+  virtual void Start();
+  // DEPRECATED, use Start().
   virtual void Initialize();
 
+  // Stops the provider.  This method will be called on the UI thread.
+  virtual void Stop();
+  // DEPRECATED, use Stop().
+  virtual void ShutDown();
+
   // Returns the URL to use for update checks.
   virtual GURL GetUpdateServerURL() const;
 

ios/public/provider/chrome/browser/omaha/omaha_service_provider.mm

@@ -12,8 +12,18 @@ OmahaServiceProvider::OmahaServiceProvider() {}
 
 OmahaServiceProvider::~OmahaServiceProvider() {}
 
+void OmahaServiceProvider::Start() {
+  Initialize();
+}
+
 void OmahaServiceProvider::Initialize() {}
 
+void OmahaServiceProvider::Stop() {
+  ShutDown();
+}
+
+void OmahaServiceProvider::ShutDown() {}
+
 GURL OmahaServiceProvider::GetUpdateServerURL() const {
   return GURL();
 }

ios/public/provider/chrome/browser/omaha/test_omaha_service_provider.h

@@ -13,7 +13,8 @@ class TestOmahaServiceProvider : public OmahaServiceProvider {
   ~TestOmahaServiceProvider() override;
 
   // OmahaServiceProvider.
-  void Initialize() override;
+  void Start() override;
+  void Stop() override;
   GURL GetUpdateServerURL() const override;
   std::string GetApplicationID() const override;
   std::string GetBrandCode() const override;

ios/public/provider/chrome/browser/omaha/test_omaha_service_provider.mm

@@ -23,7 +23,9 @@ TestOmahaServiceProvider::TestOmahaServiceProvider() {}
 
 TestOmahaServiceProvider::~TestOmahaServiceProvider() {}
 
-void TestOmahaServiceProvider::Initialize() {}
+void TestOmahaServiceProvider::Start() {}
+
+void TestOmahaServiceProvider::Stop() {}
 
 GURL TestOmahaServiceProvider::GetUpdateServerURL() const {
   return GURL(kTestUpdateServerURL);