Implement chrome.experimental.downloads.search()

content/browser/download/download_query.h

Index: content/browser/download/download_query.h
diff --git a/content/browser/download/download_query.h b/content/browser/download/download_query.h
new file mode 100644
index 0000000000000000000000000000000000000000..142685219abea7179924162b288de139e4a9b779
--- /dev/null
+++ b/content/browser/download/download_query.h
@@ -0,0 +1,110 @@
+// 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 CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_QUERY_H_
+#define CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_QUERY_H_
+#pragma once
+
+#include <string>
+#include <vector>
+
+#include "base/callback.h"
+#include "base/stl_util.h"
+#include "content/browser/download/download_item.h"
+
+namespace download_util {
+
+// Filter and sort a vector of DownloadItem*s. See also the macros at the bottom
+// of this file.
+//
+// DownloadQuery query;
+// CHECK(query.Set("startTime", 0));

[Randy Smith (Not in Mondays), 2011/10/18 23:58:46]

Is this the ideal name for it?  "Set" is a generic verb, and doesn't in and of
itself say "and we'll be filtering for DownloadItems where the field matches
this value.

+// bool FilterOutOddDownloads(DownloadItem* item) {
+//   return 0 == (item->id() % 2);
+// }
+// CHECK(query.Set("filter", base::Bind(&FilterOutOddDownloads)));

[Randy Smith (Not in Mondays), 2011/10/18 23:58:46]

I'd be inclined to make filter a separate method; it's doing something
noticeably different from Set.  Set looks layered on top of filter, but (for
instnace) it makes sense to call filter with several different values, and each
new set value will presumably just override the old one.

+// CHECK(query.OrderBy("-startTime id error"));
+// query.Limit(20);
+// DownloadItems all_items, results;
+// query.Search(all_items, &results);
+class DownloadQuery {
+ public:
+  typedef std::vector<DownloadItem*> DownloadItems;
+
+  DownloadQuery();
+  ~DownloadQuery();
+
+  // Returns false if |value| is the wrong type or if |name| is unrecognized or
+  // if |value| is malformed. Returns true and sets the filter field named
+  // |name| to |value| otherwise. See the implementation for valid |name|s and
+  // their types. (DRY.)
+  template <typename ValueType>
+  bool Set(const char* name, const ValueType& value);
+
+  // Returns false if |order_by| is not a space-separated sequence of sort field
+  // names, each optionally prefixed by a hyphen. See the implementation for
+  // valid sort field names.
+  bool OrderBy(const std::string& order_by);

[Randy Smith (Not in Mondays), 2011/10/18 23:58:46]

So one of my thoughts about suggesting that we go with field enums rather than
names was making this more programmatic.  I could imagine something like Set in
that some method takes a specific field name and a forward/reverse boolean, and
that specifies the nth sort key.  This means you can't reset the DownoadQuery,
but I think that's fine; these'll be constructed pretty much on the fly anyway.

+
+  // Remember to limit the size of search results to |limit|.
+  void Limit(uint32 limit) { limit_ = limit; }
+
+  // Sorts, filters, and limits |all_items| into |results|, which must be
+  // non-NULL.
+  void Search(const DownloadItems& all_items, DownloadItems* results) const;
+
+  // Returns true if |item| matches |this| settings, false otherwise.
+  bool Matches(const DownloadItem& item) const;
+
+  // Ignore these mostly-private interfaces.
+  class SortFieldInterface;
+  class FilterFieldInterface;
+  typedef base::hash_map<std::string, SortFieldInterface*> SortFields;
+  typedef base::hash_map<std::string, FilterFieldInterface*> FilterFields;
+  typedef std::vector<std::string> Strings;
+
+ private:
+  void SetSortField(const char* name, SortFieldInterface* field);
+  void SetFilterField(const char* name, FilterFieldInterface* field);
+
+  SortFields sort_fields_;
+  FilterFields filter_fields_;
+  Strings order_by_fields_;
+  uint32 limit_;
+  STLValueDeleter<SortFields> delete_sort_fields_;
+  STLValueDeleter<FilterFields> delete_filter_fields_;
+
+  DISALLOW_COPY_AND_ASSIGN(DownloadQuery);
+};
+}  // namespace download_util
+
+
+// The following macros facilitate using anonymous expressions (lambdas) to
+// implement arbitrary filters for DownloadQuery. The parameters passed to the
+// second and third forms MUST be variable names and cannot be expressions or
+// even lvalues.

[Randy Smith (Not in Mondays), 2011/10/13 23:23:11]

What's the reason for this restriction?  Is there any way to lift it?  It seems
like an uncomfortable restriction on callers.

[benjhayden, 2011/10/14 19:31:02]

The reason is that the macro implementation uses the variable names as both
arguments to base::Bind() and method parameter names. There are a few ways to
lift it:
0. Require the caller to specify both parameter names and values: DQF2(name0,
value0()[], name1, value1()[], item.foo() == name0 && item.bar() == name1)
1. Always use the same parameter names, allow the user to only specify values:
DQF2(value0()[], value1()[], item.foo() == param0 && item.bar() == param1)
2. Use python and its lambda and keyword args. :-)
query.filter = lambda item, name0=otr(): item.otr() == name0

Since some parts of chrome use named variables for function parameters for
clarity instead of using "param/*meaning*/,", I thought that DM's Search methods
could do the same:
bool otr = otr();
query.Set("filter", DQF1(otr, item.otr() == otr));

[Randy Smith (Not in Mondays), 2011/10/18 23:58:46]

Hmmm.  All reasonable options, but I find myself wondering if the macro magic
(here) is worth it.  If the user wants this functionality, they can create a
class with a method and use base::Bind themselves; it requires them to
understand how to use base::Bind, but I don't think the cost in terms of lines
of code is that extreme.  And macros bad :-}.  Maybe just put a couple of lines
of example code near query.Set("filter"...) doc to show people how they should
be thinking if they want to do this?

+//
+// query.Set("filter", DOWNLOAD_QUERY_FILTER2(otr, foo,
+//   item.otr() == otr && item.foo() == foo));
+
+#define SYMCAT(X, Y) X##Y
+#define DOWNLOAD_QUERY_FILTER_IMPL(NAME, expr) \
+  ({struct NAME {static bool Filter(const DownloadItem& item) {return expr;}}; \
+    base::Bind(&NAME::Filter);})
+#define DOWNLOAD_QUERY_FILTER1_IMPL(NAME, var0, expr) \
+  ({template<typename Var0Type>struct NAME {static bool Filter( \
+      Var0Type var0, const DownloadItem& item) {return expr;}}; \
+    base::Bind(&NAME::Filter, var0);})
+#define DOWNLOAD_QUERY_FILTER2_IMPL(NAME, var0, var1, expr) \
+  ({template<typename Var0Type, Var1Type>struct NAME {static bool Filter( \
+      Var0Type var0, Var1Type var1, const DownloadItem& item) {return expr;}}; \
+    base::Bind(&NAME::Filter, var0, var1);})
+#define DOWNLOAD_QUERY_FILTER(expr) DOWNLOAD_QUERY_FILTER_IMPL( \
+  SYMCAT(DownloadQueryFilter, __LINE__), (expr))
+#define DOWNLOAD_QUERY_FILTER1(var0, expr) DOWNLOAD_QUERY_FILTER1_IMPL( \
+  SYMCAT(DownloadQueryFilter, __LINE__), var0, (expr))
+#define DOWNLOAD_QUERY_FILTER2(var0, var1, expr) DOWNLOAD_QUERY_FILTER2_IMPL( \
+  SYMCAT(DownloadQueryFilter, __LINE__), var0, var1, (expr))
+
+#endif  // CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_QUERY_H_