Cleanup: separate WAPI and Drive API code in gdata_documents_service.

chrome/browser/chromeos/gdata/gdata_file_system.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/gdata/gdata_file_system.h"
 
 #include <set>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/file_util.h"
 #include "base/json/json_file_value_serializer.h"
 #include "base/message_loop.h"
 #include "base/message_loop_proxy.h"
 #include "base/metrics/histogram.h"
 #include "base/platform_file.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "base/values.h"
+#include "chrome/browser/chromeos/gdata/documents_service_interface.h"
 #include "chrome/browser/chromeos/gdata/drive_api_parser.h"
 #include "chrome/browser/chromeos/gdata/drive_webapps_registry.h"
 #include "chrome/browser/chromeos/gdata/gdata.pb.h"
-#include "chrome/browser/chromeos/gdata/gdata_documents_service.h"
 #include "chrome/browser/chromeos/gdata/gdata_download_observer.h"
 #include "chrome/browser/chromeos/gdata/gdata_files.h"
 #include "chrome/browser/chromeos/gdata/gdata_protocol_handler.h"
 #include "chrome/browser/chromeos/gdata/gdata_system_service.h"
 #include "chrome/browser/chromeos/gdata/gdata_uploader.h"
 #include "chrome/browser/chromeos/gdata/gdata_util.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/pref_names.h"
@@ skipping 2201 matching lines @@
                                ui_weak_ptr_,
                                CreateRelayCallback(callback)));
 }
 
 void GDataFileSystem::GetAvailableSpaceOnUIThread(
     const GetAvailableSpaceCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(!callback.is_null());
 
   if (gdata::util::IsDriveV2ApiEnabled()) {
-    documents_service_->GetAboutResource(
+    documents_service_->GetAccountMetadata(

[satorux1, 2012/08/17 12:05:48]

since you are now using the same interface, can you get rid of
gdata::util::IsDriveV2ApiEnabled()? Please check other places too.

You can do it in a separate patch. If so, please file a bug and add a TODO here.

[kochi, 2012/08/20 09:17:30]

As we have different callbacks for Drive API and WAPI,
we have to use IsDriveV2ApiEnabled() even though
we call the same interface here.

Changed the code here to use tertiary operator for now.

I think we should have better layering of these
file system/documents service/operations, and the higher
level users would get the result in same format whichever
API is used.

Filed http://crbug.com/143653

On 2012/08/17 12:05:48, satorux1 wrote:

[satorux1, 2012/08/20 16:47:19]

Could you make this if-else? Ternary operators are harder to read, and this will
result in a harder-to-read compilation errors if we mess up something here.


Looking at the two callback functions, these look fairly similar. Rather than
using separate callbacks, it may be simpler to use the same callback, then do
things differently in the callback per gdata::util::IsDriveV2ApiEnabled().

       base::Bind(&GDataFileSystem::OnGetAboutResource,
                  ui_weak_ptr_,
                  callback));
     return;
   }
 
   documents_service_->GetAccountMetadata(
       base::Bind(&GDataFileSystem::OnGetAvailableSpace,
                  ui_weak_ptr_,
                  callback));
@@ skipping 1200 matching lines @@
     return;
   }
 
   PlatformFileInfoProto entry_file_info;
   GDataEntry::ConvertPlatformFileInfoToProto(*file_info, &entry_file_info);
   *entry_proto->mutable_file_info() = entry_file_info;
   callback.Run(GDATA_FILE_OK, entry_proto.Pass());
 }
 
 }  // namespace gdata