Added asynchronous notification of readiness to the StatisticsProvider, and

chrome/browser/metrics/metrics_service.h

 // Copyright (c) 2012 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.
 
 // This file defines a service that collects information about the user
 // experience in order to help improve future versions of the app.
 
 #ifndef CHROME_BROWSER_METRICS_METRICS_SERVICE_H_
 #define CHROME_BROWSER_METRICS_METRICS_SERVICE_H_
 #pragma once
@@ skipping 134 matching lines @@
     INITIAL_LOG_READY,      // Initial log generated, and waiting for reply.
     SENDING_OLD_LOGS,       // Sending unsent logs from previous session.
     SENDING_CURRENT_LOGS,   // Sending standard current logs as they acrue.
   };
 
   enum ShutdownCleanliness {
     CLEANLY_SHUTDOWN = 0xdeadbeef,
     NEED_TO_SHUTDOWN = ~CLEANLY_SHUTDOWN
   };
 
-  // First part of the init task. Called on the FILE thread to load hardware
-  // class information.
-  static void InitTaskGetHardwareClass(base::WeakPtr<MetricsService> self,
-                                       base::MessageLoopProxy* target_loop);
+  // First part of the init task. Starts retrieving the hardware class and
+  // proceeds to OnInitTaskGotHardwareClass() or waits for
+  // OnStatisticsProviderReady().
+  static void InitTaskGetHardwareClass(base::WeakPtr<MetricsService> self);
+
+#if defined(OS_CHROMEOS)
+  // Callback for the StatisticsProvider to signal that hardware data has been
+  // loaded. Retrieves the hardware class and passes it to
+  // OnInitTaskGotHardwareClass().
+  void OnStatisticsProviderReady();

[jar (doing other things), 2012/04/20 00:37:39]

Rather than conditionally defining a method... and having the comments that
seems very confused... 

How about always having the methods all the time (all OS versions)... always
having the comment that described the flow... and then having the interior of
the method *optionally* do work work, depending on whether we are in CrOS or
not?

The comments on the init tasks are confusing to start with (since we need to set
several things up... with several callbacks)... and merging in the OS options to
cause different flows across these methods seems extra confusing.

[Joao da Silva, 2012/04/20 11:53:35]

Good points; I thought it was acceptable in this class because there were 2
precedents (StartExternalMetrics and LogChromeOSCrash).

The last patch set removes the hardware_class lookup step out of the init flow
completely, and the StatisticsProvider callback has an internal #ifdef as
suggested.

+#endif
 
   // Callback from InitTaskGetHardwareClass() that continues the init task by
   // loading plugin information.
   void OnInitTaskGotHardwareClass(const std::string& hardware_class);
 
   // Callback from PluginService::GetPlugins() that moves the state to
   // INIT_TASK_DONE.
   void OnInitTaskGotPluginInfo(
       const std::vector<webkit::WebPluginInfo>& plugins);
 
@@ skipping 263 matching lines @@
   friend bool prerender::IsOmniboxEnabled(Profile* profile);
   friend class extensions::ExtensionDownloader;
 
   // Returns true if prefs::kMetricsReportingEnabled is set.
   static bool IsMetricsReportingEnabled();
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(MetricsServiceHelper);
 };
 
 #endif  // CHROME_BROWSER_METRICS_METRICS_SERVICE_H_