Refactor GDataWapiFeedLoader::LoadFromServer() parameters

chrome/browser/chromeos/gdata/gdata_wapi_feed_loader.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.
 
 #ifndef CHROME_BROWSER_CHROMEOS_GDATA_GDATA_WAPI_FEED_LOADER_H_
 #define CHROME_BROWSER_CHROMEOS_GDATA_GDATA_WAPI_FEED_LOADER_H_
 
 #include "base/callback.h"
 #include "base/file_path.h"
 #include "base/observer_list.h"
 #include "chrome/browser/chromeos/gdata/gdata_directory_service.h"
 #include "chrome/browser/chromeos/gdata/gdata_errorcode.h"
-
-class GURL;
+#include "googleurl/src/gurl.h"
 
 namespace base {
 class Value;
 }
 
 namespace gdata {
 
 class DriveWebAppsRegistryInterface;
 class DocumentFeed;
 class DocumentsServiceInterface;
@@ skipping 42 matching lines @@
   // Time when filesystem began to be loaded from disk.
   base::Time load_start_time;
   const FileOperationCallback callback;
 };
 
 // Callback run as a response to LoadFromServer.
 typedef base::Callback<void(GetDocumentsParams* params,
                             GDataFileError error)>
     LoadDocumentFeedCallback;
 
+// Defines set of parameters for calling GDataWapiFeedLoader::LoadFromServer().
+// Value of |start_changestamp| determines the type of feed to load - 0 means
+// root feed, every other value would trigger delta feed.
+// In the case of loading the root feed we use |root_feed_changestamp| as its
+// initial changestamp value since it does not come with that info.
+//
+// When all feeds are loaded, |feed_load_callback| is invoked with the retrieved
+// feeds. Then |load_finished_callback| is invoked with the error code.
+//
+// |should_fetch_multiple_feeds| is true iff don't want to stop feed loading
+// after we retrieve first feed chunk.
+// If invoked as a part of content search, query will be set in |search_query|.
+// If |feed_to_load| is set, this is feed url that will be used to load feed.
+//
+// |load_finished_callback| may be null.
+// |feed_load_callback| must not be null.
+struct LoadFeedParams {
+  LoadFeedParams(ContentOrigin initial_origin,
+                 const LoadDocumentFeedCallback& feed_load_callback);
+  ~LoadFeedParams();
+
+  ContentOrigin initial_origin;
+  int64 start_changestamp;
+  int64 root_feed_changestamp;
+  bool should_fetch_multiple_feeds;
+  FilePath search_file_path;
+  std::string search_query;
+  GURL feed_to_load;
+  std::string directory_resource_id;
+  FileOperationCallback load_finished_callback;
+  const LoadDocumentFeedCallback feed_load_callback;
+};

[satorux1, 2012/08/16 10:56:29]

hmm, rather than just putting a bunch of parameters in a struct, can we do a
better job?

Would it be possible to redesign the class so we don't need to deal with this
many parameters in LoadFromServer()?

For instance, Seems to me that |search_query| is awkward. We might want to have
a separate function for search, like SearchFromServer().

+
 // GDataWapiFeedLoader is used to load feeds from WAPI (codename for
 // Documents List API) and load the cached proto file.
 class GDataWapiFeedLoader {
  public:
   // Used to notify events from the loader.
   // All events are notified on UI thread.
   class Observer {
    public:
     // Triggered when a content of a directory has been changed.
     // |directory_path| is a virtual directory path representing the
@@ skipping 29 matching lines @@
   // Then, it will initiate retrieval of the root feed from the server unless
   // |should_load_from_server| is set to false. |should_load_from_server| is
   // false only for testing. If loading from the server is successful, runs
   // |callback| if it was not previously run (i.e. loading from the cache was
   // successful).
   //
   // |callback| may be null.
   void LoadFromCache(bool should_load_from_server,
                      const FileOperationCallback& callback);
 
-  // Starts root feed load from the server. Value of |start_changestamp|
-  // determines the type of feed to load - 0 means root feed, every other
-  // value would trigger delta feed.
-  // In the case of loading the root feed we use |root_feed_changestamp| as its
-  // initial changestamp value since it does not come with that info.
-  //
-  // When all feeds are loaded, |feed_load_callback| is invoked with the
-  // retrieved feeds. Then |load_finished_callback| is invoked with the error
-  // code.
-  //
-  // |should_fetch_multiple_feeds| is true iff don't want to stop feed loading
-  // after we retrieve first feed chunk.
-  // If invoked as a part of content search, query will be set in
-  // |search_query|.
-  // If |feed_to_load| is set, this is feed url that will be used to load feed.
-  //
-  // |load_finished_callback| may be null.
-  // |feed_load_callback| must not be null.
-  void LoadFromServer(
-      ContentOrigin initial_origin,
-      int64 start_changestamp,
-      int64 root_feed_changestamp,
-      bool should_fetch_multiple_feeds,
-      const std::string& search_query,
-      const GURL& feed_to_load,
-      const std::string& directory_resource_id,
-      const FileOperationCallback& load_finished_callback,
-      const LoadDocumentFeedCallback& feed_load_callback);
+  // Starts root feed load from the server, with detail specified in |param|.
+  void LoadFromServer(const LoadFeedParams& param);
 
   // Retrieves account metadata and determines from the last change timestamp
   // if the feed content loading from the server needs to be initiated.
   void ReloadFromServerIfNeeded(
       ContentOrigin initial_origin,
       int64 local_changestamp,
       const FileOperationCallback& callback);
 
   // Updates whole directory structure feeds collected in |feed_list|.
   // On success, returns PLATFORM_FILE_OK. Record file statistics as UMA
@@ skipping 88 matching lines @@
 
   // Note: This should remain the last member so it'll be destroyed and
   // invalidate its weak pointers before any other members are destroyed.
   base::WeakPtrFactory<GDataWapiFeedLoader> weak_ptr_factory_;
   DISALLOW_COPY_AND_ASSIGN(GDataWapiFeedLoader);
 };
 
 }  // namespace gdata
 
 #endif  // CHROME_BROWSER_CHROMEOS_GDATA_GDATA_WAPI_FEED_LOADER_H_