Removed file access from UI thread for mobile config file loading.

chrome/browser/ui/webui/chromeos/mobile_setup_ui.cc

 // Copyright (c) 2011 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/ui/webui/chromeos/mobile_setup_ui.h"
 
 #include <algorithm>
 #include <map>
 #include <string>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/file_util.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "base/logging.h"
 #include "base/memory/weak_ptr.h"
 #include "base/metrics/histogram.h"
 #include "base/string_piece.h"
 #include "base/string_util.h"
-#include "base/threading/thread_restrictions.h"
 #include "base/timer.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/cros/cros_library.h"
 #include "chrome/browser/chromeos/cros/network_library.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/webui/chrome_url_data_manager.h"
@@ skipping 124 matching lines @@
   }
 
   void OnFrameLoadCompleted() {
     webui_->CallJavascriptFunction(kJsPortalFrameLoadCompletedCallback);
   }
 
   WebUI* webui_;
   DISALLOW_COPY_AND_ASSIGN(PortalFrameLoadObserver);
 };
 
-class CellularConfigDocument {
+class CellularConfigDocument
+    : public base::RefCountedThreadSafe<CellularConfigDocument> {
  public:
   CellularConfigDocument() {}
 
   // Return error message for a given code.
   std::string GetErrorMessage(const std::string& code);
+  void LoadCellularConfigFile();
   const std::string& version() { return version_; }
 
+ private:
+  void SetErrorMap(const std::map<std::string, std::string>& map);

[achuithb, 2011/10/29 01:42:44]

You could consider a typedef like
typedef std::map<std::string, std::string> ErrorMap;

[zel, 2011/10/31 15:12:47]

Done.

   bool LoadFromFile(const FilePath& config_path);
 
- private:
   std::string version_;
   std::map<std::string, std::string> error_map_;
+  base::Lock config_lock_;
 
   DISALLOW_COPY_AND_ASSIGN(CellularConfigDocument);
 };
 
 static std::map<std::string, std::string> error_messages_;
 
 class MobileSetupUIHTMLSource : public ChromeURLDataManager::DataSource {
  public:
   explicit MobileSetupUIHTMLSource(const std::string& service_path);
 
@@ skipping 18 matching lines @@
   : public WebUIMessageHandler,
     public chromeos::NetworkLibrary::NetworkManagerObserver,
     public chromeos::NetworkLibrary::NetworkObserver,
     public base::SupportsWeakPtr<MobileSetupHandler> {
  public:
   explicit MobileSetupHandler(const std::string& service_path);
   virtual ~MobileSetupHandler();
 
   // Init work after Attach.
   void Init(TabContents* contents);
+  void StartActivationOnUIThread();
 
   // WebUIMessageHandler implementation.
   virtual WebUIMessageHandler* Attach(WebUI* web_ui) OVERRIDE;
   virtual void RegisterMessages() OVERRIDE;
 
   // NetworkLibrary::NetworkManagerObserver implementation.
   virtual void OnNetworkManagerChanged(chromeos::NetworkLibrary* obj) OVERRIDE;
   // NetworkLibrary::NetworkObserver implementation.
   virtual void OnNetworkChanged(chromeos::NetworkLibrary* obj,
                                 const chromeos::Network* network) OVERRIDE;
@@ skipping 10 matching lines @@
     PLAN_ACTIVATION_SHOWING_PAYMENT         = 6,
     PLAN_ACTIVATION_RECONNECTING_PAYMENT    = 7,
     PLAN_ACTIVATION_DELAY_OTASP             = 8,
     PLAN_ACTIVATION_START_OTASP             = 9,
     PLAN_ACTIVATION_OTASP                   = 10,
     PLAN_ACTIVATION_RECONNECTING_OTASP      = 11,
     PLAN_ACTIVATION_DONE                    = 12,
     PLAN_ACTIVATION_ERROR                   = 0xFF,
   } PlanActivationState;
 
-  class TaskProxy : public base::RefCountedThreadSafe<TaskProxy> {
-   public:
-    TaskProxy(const base::WeakPtr<MobileSetupHandler>& handler, int delay)
-        : handler_(handler), delay_(delay) {
-    }
-    TaskProxy(const base::WeakPtr<MobileSetupHandler>& handler,
-              const std::string& status)
-        : handler_(handler), status_(status) {
-    }
-    void HandleStartActivation() {
-      if (handler_)
-        handler_->StartActivation();
-    }
-    void HandleSetTransactionStatus() {
-      if (handler_)
-        handler_->SetTransactionStatus(status_);
-    }
-    void ContinueConnecting() {
-      if (handler_)
-        handler_->ContinueConnecting(delay_);
-    }
-    void RetryOTASP() {
-      if (handler_)
-        handler_->RetryOTASP();
-    }
-   private:
-    base::WeakPtr<MobileSetupHandler> handler_;
-    std::string status_;
-    int delay_;
-    DISALLOW_COPY_AND_ASSIGN(TaskProxy);
-  };
-
   // Handlers for JS WebUI messages.
   void HandleSetTransactionStatus(const ListValue* args);
   void HandleStartActivation(const ListValue* args);
   void HandlePaymentPortalLoad(const ListValue* args);
   void SetTransactionStatus(const std::string& status);
-  // Schedules activation process via task proxy.
-  void InitiateActivation();
   // Starts activation.
   void StartActivation();
   // Retried OTASP.
   void RetryOTASP();
   // Continues activation process. This method is called after we disconnect
   // due to detected connectivity issue to kick off reconnection.
   void ContinueConnecting(int delay);
 
   // Sends message to host registration page with system/user info data.
   void SendDeviceInfo();
@@ skipping 30 matching lines @@
   void SetupActivationProcess(chromeos::CellularNetwork* network);
   // Disables ethernet and wifi newtorks since they interefere with
   // detection of restricted pool on cellular side.
   void DisableOtherNetworks();
   // Resets network devices after cellular activation process.
   // |network| should be NULL if the activation process failed.
   void CompleteActivation(chromeos::CellularNetwork* network);
   // Control routines for handling other types of connections during
   // cellular activation.
   void ReEnableOtherConnections();
+  // Return error message for a given code.
+  std::string GetErrorMessage(const std::string& code);
 
   // Converts the currently active CellularNetwork device into a JS object.
   static void GetDeviceInfo(chromeos::CellularNetwork* network,
                             DictionaryValue* value);
   static bool ShouldReportDeviceState(std::string* state, std::string* error);
 
   // Performs activation state cellular device evaluation.
   // Returns false if device activation failed. In this case |error|
   // will contain error message to be reported to Web UI.
   static bool EvaluateCellularDeviceState(bool* report_status,
                                           std::string* state,
                                           std::string* error);
 
-  // Return error message for a given code.
-  static std::string GetErrorMessage(const std::string& code);
-  static void LoadCellularConfig();
-
   // Returns next reconnection state based on the current activation phase.
   static PlanActivationState GetNextReconnectState(PlanActivationState state);
   static const char* GetStateDescription(PlanActivationState state);
 
-  static scoped_ptr<CellularConfigDocument> cellular_config_;
-
+  scoped_refptr<CellularConfigDocument> cellular_config_;
   TabContents* tab_contents_;
   // Internal handler state.
   PlanActivationState state_;
   std::string service_path_;
   // Flags that control if wifi and ethernet connection needs to be restored
   // after the activation of cellular network.
   bool reenable_wifi_;
   bool reenable_ethernet_;
   bool reenable_cert_check_;
   bool evaluating_;
   // True if we think that another tab is already running activation.
   bool already_running_;
   // Connection retry counter.
   int connection_retry_count_;
   // Post payment reconnect wait counters.
   int reconnect_wait_count_;
   // Payment portal reload/reconnect attempt count.
   int payment_reconnect_count_;
   // Activation retry attempt count;
   int activation_attempt_;
   // Connection start time.
   base::Time connection_start_time_;
   // Timer that monitors reconnection timeouts.
   base::RepeatingTimer<MobileSetupHandler> reconnect_timer_;
 
   DISALLOW_COPY_AND_ASSIGN(MobileSetupHandler);
 };
 
-scoped_ptr<CellularConfigDocument> MobileSetupHandler::cellular_config_;
-
 ////////////////////////////////////////////////////////////////////////////////
 //
 // CellularConfigDocument
 //
 ////////////////////////////////////////////////////////////////////////////////
 
 std::string CellularConfigDocument::GetErrorMessage(const std::string& code) {
+  base::AutoLock create(config_lock_);
   std::map<std::string, std::string>::iterator iter = error_map_.find(code);
   if (iter == error_map_.end())
     return code;
   return iter->second;
 }
 
+void CellularConfigDocument::LoadCellularConfigFile() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+  // Load partner customization startup manifest if it is available.
+  FilePath config_path(kCellularConfigPath);
+  if (!file_util::PathExists(config_path))
+    return;
+
+  if (LoadFromFile(config_path)) {
+    LOG(INFO) << "Cellular config file loaded: " << kCellularConfigPath;

[achuithb, 2011/10/29 01:42:44]

Did you intend to land these log stmts or are these for debugging?

[zel, 2011/10/31 15:12:47]

Done.

+  } else {
+    LOG(ERROR) << "Error loading cellular config file: " <<
+        kCellularConfigPath;
+  }
+}
+
 bool CellularConfigDocument::LoadFromFile(const FilePath& config_path) {
-  error_map_.clear();
-
   std::string config;
-  {
-    // Reading config file causes us to do blocking IO on UI thread.
-    // Temporarily allow it until we fix http://crosbug.com/11535
-    base::ThreadRestrictions::ScopedAllowIO allow_io;
-    if (!file_util::ReadFileToString(config_path, &config))
-      return false;
-  }
+  if (!file_util::ReadFileToString(config_path, &config))
+    return false;
 
   scoped_ptr<Value> root(base::JSONReader::Read(config, true));
   DCHECK(root.get() != NULL);
   if (!root.get() || root->GetType() != Value::TYPE_DICTIONARY) {
     LOG(WARNING) << "Bad cellular config file";
     return false;
   }
 
   DictionaryValue* root_dict = static_cast<DictionaryValue*>(root.get());
   if (!root_dict->GetString(kVersionField, &version_)) {
     LOG(WARNING) << "Cellular config file missing version";
     return false;
   }
+  std::map<std::string, std::string> error_map;
   DictionaryValue* errors = NULL;
   if (!root_dict->GetDictionary(kErrorsField, &errors))
     return false;
   for (DictionaryValue::key_iterator keys = errors->begin_keys();
        keys != errors->end_keys();
        ++keys) {
     std::string value;
     if (!errors->GetString(*keys, &value)) {
       LOG(WARNING) << "Bad cellular config error value";
-      error_map_.clear();
       return false;
     }
+    error_map.insert(std::pair<std::string, std::string>(*keys, value));

[achuithb, 2011/10/29 01:42:44]

with the ErrorMap typedef, this would become ErrorMap::value_type(*keys, value)

[zel, 2011/10/31 15:12:47]

Done.

+  }
+  SetErrorMap(error_map);
+  return true;
+}
 
-    error_map_.insert(std::pair<std::string, std::string>(*keys, value));
-  }
-  return true;
+void CellularConfigDocument::SetErrorMap(
+    const std::map<std::string, std::string>& map) {
+  base::AutoLock create(config_lock_);
+  error_map_.clear();
+  error_map_.insert(map.begin(), map.end());
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 //
 // MobileSetupUIHTMLSource
 //
 ////////////////////////////////////////////////////////////////////////////////
 
 MobileSetupUIHTMLSource::MobileSetupUIHTMLSource(
     const std::string& service_path)
@@ skipping 46 matching lines @@
 
   SendResponse(request_id, base::RefCountedString::TakeString(&full_html));
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 //
 // MobileSetupHandler
 //
 ////////////////////////////////////////////////////////////////////////////////
 MobileSetupHandler::MobileSetupHandler(const std::string& service_path)
-    : tab_contents_(NULL),
+    : cellular_config_(new CellularConfigDocument()),
+      tab_contents_(NULL),
       state_(PLAN_ACTIVATION_PAGE_LOADING),
       service_path_(service_path),
       reenable_wifi_(false),
       reenable_ethernet_(false),
       reenable_cert_check_(false),
       evaluating_(false),
       already_running_(false),
       connection_retry_count_(0),
       reconnect_wait_count_(0),
       payment_reconnect_count_(0),
@@ skipping 10 matching lines @@
     lib->Unlock();
   ReEnableOtherConnections();
 }
 
 WebUIMessageHandler* MobileSetupHandler::Attach(WebUI* web_ui) {
   return WebUIMessageHandler::Attach(web_ui);
 }
 
 void MobileSetupHandler::Init(TabContents* contents) {
   tab_contents_ = contents;
-  chromeos::CellularNetwork* network = GetCellularNetwork(service_path_);
-  if (!network || !network->SupportsActivation())
-    return;
-  LoadCellularConfig();
-  if (!chromeos::CrosLibrary::Get()->GetNetworkLibrary()->IsLocked())
-    SetupActivationProcess(network);
-  else
-    already_running_ = true;
 }
 
 void MobileSetupHandler::RegisterMessages() {
   web_ui_->RegisterMessageCallback(kJsApiStartActivation,
       base::Bind(&MobileSetupHandler::HandleStartActivation,
                  base::Unretained(this)));
   web_ui_->RegisterMessageCallback(kJsApiSetTransactionStatus,
       base::Bind(&MobileSetupHandler::HandleSetTransactionStatus,
                  base::Unretained(this)));
   web_ui_->RegisterMessageCallback(kJsApiPaymentPortalLoad,
@@ skipping 14 matching lines @@
 
 void MobileSetupHandler::OnNetworkChanged(chromeos::NetworkLibrary* cros,
                                           const chromeos::Network* network) {
   if (state_ == PLAN_ACTIVATION_PAGE_LOADING)
     return;
   DCHECK(network && network->type() == chromeos::TYPE_CELLULAR);
   EvaluateCellularNetwork(GetCellularNetwork(network->service_path()));
 }
 
 void MobileSetupHandler::HandleStartActivation(const ListValue* args) {
-  InitiateActivation();
-  UMA_HISTOGRAM_COUNTS("Cellular.MobileSetupStart", 1);
+  BrowserThread::PostTaskAndReply(BrowserThread::FILE, FROM_HERE,
+      base::Bind(&CellularConfigDocument::LoadCellularConfigFile,
+                 cellular_config_.get()),
+      base::Bind(&MobileSetupHandler::StartActivationOnUIThread, AsWeakPtr()));
 }
 
 void MobileSetupHandler::HandleSetTransactionStatus(const ListValue* args) {
   const size_t kSetTransactionStatusParamCount = 1;
   if (args->GetSize() != kSetTransactionStatusParamCount)
     return;
   // Get change callback function name.
   std::string status;
   if (!args->GetString(0, &status))
     return;
-  scoped_refptr<TaskProxy> task = new TaskProxy(AsWeakPtr(), status);
   BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-      base::Bind(&TaskProxy::HandleSetTransactionStatus, task.get()));
+      base::Bind(&MobileSetupHandler::SetTransactionStatus, AsWeakPtr(),
+                 status));
 }
 
 void MobileSetupHandler::HandlePaymentPortalLoad(const ListValue* args) {
   const size_t kPaymentPortalLoadParamCount = 1;
   if (args->GetSize() != kPaymentPortalLoadParamCount)
     return;
   // Get change callback function name.
   std::string result;
   if (!args->GetString(0, &result))
     return;
@@ skipping 18 matching lines @@
       // Disconnect now, this should force reconnection and we will retry to
       // load the frame containing payment portal again.
       DisconnectFromNetwork(network);
     }
   } else {
     NOTREACHED() << "Called paymentPortalLoad while in unexpected state: "
                  << GetStateDescription(state_);
   }
 }
 
-void MobileSetupHandler::InitiateActivation() {
-  scoped_refptr<TaskProxy> task = new TaskProxy(AsWeakPtr(), 0);
-  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-      base::Bind(&TaskProxy::HandleStartActivation, task.get()));
-}
-
 void MobileSetupHandler::StartActivation() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  UMA_HISTOGRAM_COUNTS("Cellular.MobileSetupStart", 1);
   chromeos::NetworkLibrary* lib =
       chromeos::CrosLibrary::Get()->GetNetworkLibrary();
   chromeos::CellularNetwork* network = GetCellularNetwork(service_path_);
   // Check if we can start activation process.
   if (!network || already_running_) {
     std::string error;
     if (already_running_)
       error = kErrorAlreadyRunning;
     else if (!lib->cellular_available())
       error = kErrorNoDevice;
@@ skipping 125 matching lines @@
     return false;
   if (network)
     LOG(INFO) << "Connecting to: " << network->service_path();
   connection_retry_count_++;
   connection_start_time_ = base::Time::Now();
   if (!network) {
     LOG(WARNING) << "Connect failed."
                  << (network ? network->service_path().c_str() : "no service");
     // If we coudn't connect during reconnection phase, try to reconnect
     // with a delay (and try to reconnect if needed).
-    scoped_refptr<TaskProxy> task = new TaskProxy(AsWeakPtr(),
-                                                  delay);
     BrowserThread::PostDelayedTask(BrowserThread::UI, FROM_HERE,
-        base::Bind(&TaskProxy::ContinueConnecting, task.get()), delay);
+        base::Bind(&MobileSetupHandler::ContinueConnecting, AsWeakPtr(), delay),
+        delay);
     return false;
   }
   chromeos::CrosLibrary::Get()->GetNetworkLibrary()->
       ConnectToCellularNetwork(network);
   return true;
 }
 
 void MobileSetupHandler::ForceReconnect(
     chromeos::CellularNetwork* network,
     int delay) {
   DCHECK(network);
   UMA_HISTOGRAM_COUNTS("Cellular.ActivationRetry", 1);
   // Reset reconnect metrics.
   connection_retry_count_ = 0;
   connection_start_time_ = base::Time();
   // First, disconnect...
   LOG(INFO) << "Disconnecting from " << network->service_path();
   chromeos::CrosLibrary::Get()->GetNetworkLibrary()->
       DisconnectFromNetwork(network);
   // Check the network state 3s after we disconnect to make sure.
-  scoped_refptr<TaskProxy> task = new TaskProxy(AsWeakPtr(),
-                                                delay);
   BrowserThread::PostDelayedTask(BrowserThread::UI, FROM_HERE,
-      base::Bind(&TaskProxy::ContinueConnecting, task.get()), delay);
+      base::Bind(&MobileSetupHandler::ContinueConnecting, AsWeakPtr(), delay),
+      delay);
 }
 
 bool MobileSetupHandler::ConnectionTimeout() {
   return (base::Time::Now() -
             connection_start_time_).InSeconds() > kConnectionTimeoutSeconds;
 }
 
 void MobileSetupHandler::EvaluateCellularNetwork(
     chromeos::CellularNetwork* network) {
   if (!web_ui_)
@@ skipping 382 matching lines @@
 
   // Signal to JS layer that the state is changing.
   UpdatePage(network, error_description);
 
   // Pick action that should happen on entering the new state.
   switch (new_state) {
     case PLAN_ACTIVATION_START:
       break;
     case PLAN_ACTIVATION_DELAY_OTASP: {
       UMA_HISTOGRAM_COUNTS("Cellular.RetryOTASP", 1);
-      scoped_refptr<TaskProxy> task = new TaskProxy(AsWeakPtr(), 0);
       BrowserThread::PostDelayedTask(BrowserThread::UI, FROM_HERE,
-          base::Bind(&TaskProxy::RetryOTASP, task.get()), kOTASPRetryDelay);
+          base::Bind(&MobileSetupHandler::RetryOTASP, AsWeakPtr()),
+          kOTASPRetryDelay);
       break;
     }
     case PLAN_ACTIVATION_INITIATING_ACTIVATION:
     case PLAN_ACTIVATION_TRYING_OTASP:
     case PLAN_ACTIVATION_OTASP:
       DCHECK(network);
       LOG(WARNING) << "Activating service " << network->service_path().c_str();
       UMA_HISTOGRAM_COUNTS("Cellular.ActivationTry", 1);
       if (!network->StartActivation()) {
         UMA_HISTOGRAM_COUNTS("Cellular.ActivationFailure", 1);
@@ skipping 163 matching lines @@
   const chromeos::NetworkDevice* device =
       cros->FindNetworkDeviceByPath(network->device_path());
   if (device) {
     value->SetString("MEID", device->meid());
     value->SetString("IMEI", device->imei());
     value->SetString("MDN", device->mdn());
   }
 }
 
 std::string MobileSetupHandler::GetErrorMessage(const std::string& code) {
-  if (!cellular_config_.get())
-    return "";
   return cellular_config_->GetErrorMessage(code);
 }
 
-void MobileSetupHandler::LoadCellularConfig() {
-  static bool config_loaded = false;
-  if (config_loaded)
+void MobileSetupHandler::StartActivationOnUIThread() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  chromeos::CellularNetwork* network = GetCellularNetwork(service_path_);
+  if (!network || !network->SupportsActivation())
     return;
-  config_loaded = true;
-  // Load partner customization startup manifest if it is available.
-  FilePath config_path(kCellularConfigPath);
-  bool config_exists = false;
-  {
-    // Reading config file causes us to do blocking IO on UI thread.
-    // Temporarily allow it until we fix http://crosbug.com/11535
-    base::ThreadRestrictions::ScopedAllowIO allow_io;
-    config_exists = file_util::PathExists(config_path);
-  }
-  if (config_exists) {
-    scoped_ptr<CellularConfigDocument> config(new CellularConfigDocument());
-    bool config_loaded = config->LoadFromFile(config_path);
-    if (config_loaded) {
-      LOG(INFO) << "Cellular config file loaded: " << kCellularConfigPath;
-      // lock
-      cellular_config_.reset(config.release());
-    } else {
-      LOG(ERROR) << "Error loading cellular config file: " <<
-          kCellularConfigPath;
-    }
-  }
+
+  if (!chromeos::CrosLibrary::Get()->GetNetworkLibrary()->IsLocked())
+    SetupActivationProcess(network);
+  else
+    already_running_ = true;
+
+  StartActivation();
 }
 
-
 ////////////////////////////////////////////////////////////////////////////////
 //
 // MobileSetupUI
 //
 ////////////////////////////////////////////////////////////////////////////////
 
 MobileSetupUI::MobileSetupUI(TabContents* contents) : ChromeWebUI(contents) {
   chromeos::CellularNetwork* network = GetCellularNetwork();
   std::string service_path = network ? network->service_path() : std::string();
   MobileSetupHandler* handler = new MobileSetupHandler(service_path);
   AddMessageHandler((handler)->Attach(this));
   handler->Init(contents);
   MobileSetupUIHTMLSource* html_source =
       new MobileSetupUIHTMLSource(service_path);
 
   // Set up the chrome://mobilesetup/ source.
   Profile* profile = Profile::FromBrowserContext(contents->browser_context());
   profile->GetChromeURLDataManager()->AddDataSource(html_source);
 }
 
 void MobileSetupUI::RenderViewCreated(RenderViewHost* host) {
   ChromeWebUI::RenderViewCreated(host);
   // Destroyed by the corresponding RenderViewHost.
   new PortalFrameLoadObserver(host, tab_contents()->web_ui());
 }