filemanager: Fix GetDriveConnectionStateFunctionChange to return "offline" when DriveSystemService …

chrome/browser/chromeos/extensions/file_browser_private_api.cc

Index: chrome/browser/chromeos/extensions/file_browser_private_api.cc
diff --git a/chrome/browser/chromeos/extensions/file_browser_private_api.cc b/chrome/browser/chromeos/extensions/file_browser_private_api.cc
index 85c4c2c851be68267af29ae07046b488d97ee8c7..06d1e0dc7b3088101688f9cf6a0b82b8fd84ffdc 100644
--- a/chrome/browser/chromeos/extensions/file_browser_private_api.cc
+++ b/chrome/browser/chromeos/extensions/file_browser_private_api.cc
@@ -122,6 +122,7 @@ const char kDriveConnectionTypeOnline[] = "online";
  */
 const char kDriveConnectionReasonNotReady[] = "not_ready";
 const char kDriveConnectionReasonNoNetwork[] = "no_network";
+const char kDriveConnectionReasonNoService[] = "no_service";
 
 // Unescape rules used for parsing query parameters.
 const net::UnescapeRule::Type kUnescapeRuleForQueryParameters =
@@ -3082,22 +3083,30 @@ bool GetDriveConnectionStateFunction::RunImpl() {
   drive::DriveSystemService* system_service =
       drive::DriveSystemServiceFactory::GetForProfile(profile_);
 
-  bool ready = system_service->drive_service()->CanStartOperation();
-  bool is_connection_cellular =
-      net::NetworkChangeNotifier::IsConnectionCellular(
-          net::NetworkChangeNotifier::GetConnectionType());
-  if (net::NetworkChangeNotifier::IsOffline() || !ready) {
+  if (system_service) {
+    bool ready = system_service->drive_service()->CanStartOperation();
+    bool is_connection_cellular =
+        net::NetworkChangeNotifier::IsConnectionCellular(
+            net::NetworkChangeNotifier::GetConnectionType());
+    if (net::NetworkChangeNotifier::IsOffline() || !ready) {
+      type_string = kDriveConnectionTypeOffline;
+      if (net::NetworkChangeNotifier::IsOffline())

[hashimoto, 2013/02/12 06:42:05]

nit: It seems redundant to have two IsOffline() checks here and below.
How about modifying |ready|'s value to 'system_service &&
system_service->drive_service()->CanStartOperation()' to avoid code duplication?

[Haruki Sato, 2013/02/12 07:28:49]

Good point!
Refactored as suggested. and it's now clearer that |reasons| actually explains
why connection is "offline".

+        reasons->AppendString(kDriveConnectionReasonNoNetwork);
+      if (!ready)
+        reasons->AppendString(kDriveConnectionReasonNotReady);
+    } else if (
+        is_connection_cellular &&
+        profile_->GetPrefs()->GetBoolean(prefs::kDisableDriveOverCellular)) {
+      type_string = kDriveConnectionTypeMetered;
+    } else {
+      type_string = kDriveConnectionTypeOnline;
+    }
+  } else {
+    // No Drive service. Disabled by user or in Incognito/Guest mode.
     type_string = kDriveConnectionTypeOffline;
     if (net::NetworkChangeNotifier::IsOffline())
       reasons->AppendString(kDriveConnectionReasonNoNetwork);
-    if (!ready)
-      reasons->AppendString(kDriveConnectionReasonNotReady);
-  } else if (
-      is_connection_cellular &&
-      profile_->GetPrefs()->GetBoolean(prefs::kDisableDriveOverCellular)) {
-    type_string = kDriveConnectionTypeMetered;
-  } else {
-    type_string = kDriveConnectionTypeOnline;
+    reasons->AppendString(kDriveConnectionReasonNoService);
   }
 
   value->SetString("type", type_string);