Compressed activity log database storage

chrome/browser/extensions/activity_log/activity_log_policy.h

Index: chrome/browser/extensions/activity_log/activity_log_policy.h
diff --git a/chrome/browser/extensions/activity_log/activity_log_policy.h b/chrome/browser/extensions/activity_log/activity_log_policy.h
index e8421dc8819b055b8664356dc0ca9e5ed4d554eb..bb55aadb6ad71c4e068af3da509c4c01ef24c2fd 100644
--- a/chrome/browser/extensions/activity_log/activity_log_policy.h
+++ b/chrome/browser/extensions/activity_log/activity_log_policy.h
@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_EXTENSIONS_ACTIVITY_LOG_ACTIVITY_LOG_POLICY_H_
 #define CHROME_BROWSER_EXTENSIONS_ACTIVITY_LOG_ACTIVITY_LOG_POLICY_H_
 
+#include <set>
 #include <string>
 
 #include "base/bind.h"
@@ -51,23 +52,10 @@ class ActivityLogPolicy {
  public:
   enum PolicyType {
     POLICY_FULLSTREAM,
-    POLICY_NOARGS,
+    POLICY_COUNTS,
     POLICY_INVALID,
   };
 
-  // For all subclasses, add all the key types they might support here.
-  // The actual key is returned by calling GetKey(KeyType).  The subclasses
-  // are free to return an empty string for keys they don't support.
-  // For every key added here, you should update the GetKey member function
-  // for at least one policy.
-  enum KeyType {
-    PARAM_KEY_REASON,      // Why an action was blocked
-    PARAM_KEY_DOM_ACTION,  // Getter, Setter, Method,...
-    PARAM_KEY_URL_TITLE,
-    PARAM_KEY_DETAILS_STRING,
-    PARAM_KEY_EXTRA,
-  };
-
   // Parameters are the profile and the thread that will be used to execute
   // the callback when ReadData is called.
   // TODO(felt,dbabic)  Since only ReadData uses thread_id, it would be
@@ -99,11 +87,58 @@ class ActivityLogPolicy {
       const base::Callback
           <void(scoped_ptr<Action::ActionVector>)>& callback) = 0;
 
-  virtual std::string GetKey(KeyType key_id) const;
-
   // For unit testing only.
   void SetClockForTesting(base::Clock* clock) { testing_clock_ = clock; }
 
+  // A collection of methods that are useful for implementing policies.  These
+  // are all static methods; the ActivityLogPolicy::Util class cannot be
+  // instantiated.  This is nested within ActivityLogPolicy to make calling
+  // these methods more convenient from within a policy, but they are public.
+  class Util {
+   public:
+    // Serialize a Value as a JSON string.  Returns an empty string if value is
+    // null.
+    static std::string Serialize(const base::Value* value);
+
+    // Removes potentially privacy-sensitive data that should not be logged.
+    // This should generally be called on an Action before logging, unless
+    // debugging flags are enabled.  Modifies the Action object in place; if
+    // the action might be shared with other users, it is up to the caller to
+    // call ->Clone() first.
+    static void SanitizeAction(scoped_refptr<Action> action);

[felt, 2013/08/07 01:09:42]

could you make the name be related to privacy?

[mvrable, 2013/08/07 17:01:19]

Changed to StripPrivacySensitiveFields; if you can think of a better name I can
use that instead.

[felt, 2013/08/08 02:08:50]

Sounds good.

+
+    // Strip arguments from most API actions, preserving actions only for a
+    // whitelisted set.  Modifies the Action object in-place.
+    static void StripArguments(const std::set<std::string>& api_whitelist,

[felt, 2013/08/07 01:09:42]

is this a legacy thing, meant to be replaced by SanitizeAction? (since
StripArguments was my poor man's version?) or do you foresee other uses of it

[mvrable, 2013/08/07 17:01:19]

I was imagining possible use cases where we might want to support a policy that
logs more arguments but still wants to be careful not to log privacy-sensitive
data.  Such a policy could call SanitizeAction, but might not use
StripArguments.

We don't currently have any such policies, though, so maybe this isn't worth
keeping separate?

[felt, 2013/08/08 02:08:50]

Ahh, I see.
After thinking about it more, it does make sense to keep around for now. We
don't need to keep arguments for every API call, purely from a space perspective
it makes sense to have some whitelist for removing arguments we don't think are
important, and I can imagine that differing between policies.

+                               scoped_refptr<Action> action);
+
+    // Serialize a URL and an associated incognito flag into a single string,
+    // to be stored in the database.  If incognito URLs should be hidden then
+    // the URL contents should be stripped (SanitizeAction does this).
+    static std::string UrlToString(const GURL& url, bool incognito_flag);
+
+    // Given a base day (timestamp at local midnight), computes the timestamp
+    // at midnight the given number of days before or after.
+    static base::Time AddDays(const base::Time& base_date, int days);
+
+    // Compute the time bounds that should be used for a database query to
+    // cover a time range days_ago days in the past, relative to the specified
+    // time.
+    static void ComputeDatabaseTimeBounds(const base::Time& now,
+                                          int days_ago,
+                                          int64* early_bound,
+                                          int64* late_bound);
+
+    // Deletes obsolete database tables from an activity log database.  This
+    // can be used in InitDatabase() methods of ActivityLogDatabasePolicy
+    // subclasses to clean up data from old versions of the activity logging
+    // code.  Returns true on success, false on database error.
+    static bool DropObsoleteTables(sql::Connection* db);
+
+   private:
+    DISALLOW_IMPLICIT_CONSTRUCTORS(Util);
+  };
+
  protected:
   // An ActivityLogPolicy is not directly destroyed.  Instead, call Close()
   // which will cause the object to be deleted when it is safe.