Implement chrome.experimental.downloads.search()

chrome/browser/extensions/extension_downloads_api.h

 // 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.
 
 #ifndef CHROME_BROWSER_EXTENSIONS_EXTENSION_DOWNLOADS_API_H_
 #define CHROME_BROWSER_EXTENSIONS_EXTENSION_DOWNLOADS_API_H_
 #pragma once
 
 #include <map>
 #include <set>
 #include <string>
 
 #include "base/file_path.h"
 #include "base/memory/singleton.h"
 #include "base/string16.h"
 #include "chrome/browser/extensions/extension_function.h"
 #include "content/browser/download/download_item.h"
+#include "content/browser/download/download_query.h"

[asanka, 2011/10/20 19:29:16]

Nit: ordering.

 #include "content/browser/download/download_manager.h"
 
 namespace base {
 class DictionaryValue;
 }
 class ResourceDispatcherHost;
 class TabContents;
 namespace content {
 class ResourceContext;
 }
@@ skipping 94 matching lines @@
   };
   void BeginDownloadOnIOThread();
   void OnStarted(DownloadId dl_id, net::Error error);
   void RespondOnUIThread(int dl_id, net::Error error);
 
   scoped_ptr<IOData> iodata_;
 
   DISALLOW_COPY_AND_ASSIGN(DownloadsDownloadFunction);
 };
 
 class DownloadsSearchFunction : public SyncDownloadsFunction {

[cbentzel, 2011/10/20 18:18:53]

Should this be an AsyncDownloadsFunction function? Do we know how long this is
going to take? I'd worry about this blocking the UI thread, although I suppose
it should be only accessing the in-memory representation instead of hitting the
db/disk.

  public:
   DownloadsSearchFunction();
   virtual ~DownloadsSearchFunction();
   DECLARE_EXTENSION_FUNCTION_NAME("experimental.downloads.search");
 
  protected:
   virtual bool ParseArgs() OVERRIDE;

[cbentzel, 2011/10/20 18:18:53]

For testing purposes, it might be nice to have a 

bool ParseArgsIntoQuery(DownloadQuery* query) 

function to validate that the parser works correctly. 

ParseArgs could just be a simple wrapper around that.

   virtual void RunInternal() OVERRIDE;
 
  private:
+  template <typename ValueType>
+  bool Parse(base::DictionaryValue* json, const char* json_name,
+             download_util::DownloadQuery::FilterFieldName filter_name);
+
+  download_util::DownloadQuery query_;

[cbentzel, 2011/10/20 18:18:53]

Could you make this a scoped_ptr so you can just forward declare DownloadQuery?
It does mean that FilterFieldName would need to move out into a separate
header/class, although I think that Parse could be done separately.

+  int get_id_;
+  bool has_get_id_;
+
   DISALLOW_COPY_AND_ASSIGN(DownloadsSearchFunction);
 };
 
 class DownloadsPauseFunction : public SyncDownloadsFunction {
  public:
   DownloadsPauseFunction();
   virtual ~DownloadsPauseFunction();
   DECLARE_EXTENSION_FUNCTION_NAME("experimental.downloads.pause");
 
  protected:
@@ skipping 115 matching lines @@
   typedef base::hash_map<int, DownloadItem*> ItemMap;
   typedef std::set<int> DownloadIdSet;
 
   Profile* profile_;
   DownloadManager* manager_;
   DownloadIdSet downloads_;
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionDownloadsEventRouter);
 };
 #endif  // CHROME_BROWSER_EXTENSIONS_EXTENSION_DOWNLOADS_API_H_