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

chrome/browser/chromeos/extensions/file_browser_private_api.cc

 // 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.
 
 #include "chrome/browser/chromeos/extensions/file_browser_private_api.h"
 
 #include <sys/stat.h>
 #include <sys/statvfs.h>
 #include <sys/types.h>
 #include <utime.h>
@@ skipping 104 matching lines @@
 const char kDriveConnectionTypeMetered[] = "metered";
 const char kDriveConnectionTypeOnline[] = "online";
 
 /**
  * List of reasons of kDriveConnectionType*.
  *
  * Keep this in sync with the DriveConnectionReason in file_manager.js.
  */
 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 =
     net::UnescapeRule::SPACES |
     net::UnescapeRule::URL_SPECIAL_CHARS |
     net::UnescapeRule::REPLACE_PLUS_WITH_SPACE;
 
 const DiskMountManager::Disk* GetVolumeAsDisk(const std::string& mount_path) {
   DiskMountManager* disk_mount_manager = DiskMountManager::GetInstance();
 
@@ skipping 2940 matching lines @@
 }
 
 bool GetDriveConnectionStateFunction::RunImpl() {
   scoped_ptr<DictionaryValue> value(new DictionaryValue());
   scoped_ptr<ListValue> reasons(new ListValue());
 
   std::string type_string;
   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);
   value->Set("reasons", reasons.release());
   SetResult(value.release());
 
   return true;
 }
 
 bool RequestDirectoryRefreshFunction::RunImpl() {
@@ skipping 141 matching lines @@
                  this, name.size()));
   return true;
 }
 
 void ValidatePathNameLengthFunction::OnFilePathLimitRetrieved(
     size_t current_length,
     size_t max_length) {
   SetResult(new base::FundamentalValue(current_length <= max_length));
   SendResponse(true);
 }