New architecture of the activity logging: Policies for summarization (and compression)

chrome/browser/extensions/activity_log/fullstream_ui_policy.cc

+// Copyright $YEAR 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 "base/files/file_path.h"
+#include "base/json/json_string_value_serializer.h"
+#include "base/logging.h"
+#include "base/string16.h"
+#include "chrome/browser/extensions/activity_log/activity_database.h"
+#include "chrome/browser/extensions/activity_log/api_actions.h"
+#include "chrome/browser/extensions/activity_log/blocked_actions.h"
+#include "chrome/browser/extensions/activity_log/dom_actions.h"
+#include "chrome/browser/extensions/activity_log/fullstream_ui_policy.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/common/chrome_constants.h"
+#include "chrome/common/extensions/extension.h"
+#include "googleurl/src/gurl.h"
+#include "sql/error_delegate_util.h"
+
+using base::Callback;
+using base::FilePath;
+using base::Time;
+using base::Unretained;
+using content::BrowserThread;
+
+namespace {
+
+// Key strings for passing parameters to the ProcessAction member function.
+const char kKeyReason[] =         "fsuip.reason";
+const char kKeyDomainAction[] =   "fsuip.domact";
+const char kKeyURLTitle[] =       "fsuip.urltitle";
+const char kKeyDetailsString[] =  "fsuip.details";
+
+}  // End of namespace anonymous
+
+namespace extensions {
+
+// TODO(dbabic) This would be a fine error handler for all sql-based policies,
+// so it would make sense to introduce another class in the hierarchy,
+// SQLiteBasedPolicy as a super class of FullStreamUIPolicy and move this
+// error handler (as well as other SQLite-related functionality) there.
+
+// This handles errors from the database.
+class KillActivityDatabaseErrorDelegate : public sql::ErrorDelegate {
+ public:
+  explicit KillActivityDatabaseErrorDelegate(FullStreamUIPolicy* backend)
+      : backend_(backend),
+        scheduled_death_(false) {}
+
+  virtual int OnError(int error,
+                      sql::Connection* connection,
+                      sql::Statement* stmt) OVERRIDE {
+    if (!scheduled_death_ && sql::IsErrorCatastrophic(error)) {
+      ScheduleDeath();
+    }
+    return error;
+  }
+
+  // Schedules death if an error wasn't already reported.
+  void ScheduleDeath() {
+    if (!scheduled_death_) {
+      scheduled_death_ = true;
+      backend_->KillActivityLogDatabase();
+    }
+  }
+
+  bool scheduled_death() const {
+    return scheduled_death_;
+  }
+
+ private:
+  FullStreamUIPolicy* backend_;
+  bool scheduled_death_;
+
+  DISALLOW_COPY_AND_ASSIGN(KillActivityDatabaseErrorDelegate);
+};
+
+FullStreamUIPolicy::FullStreamUIPolicy(
+    Profile* profile,
+    content::BrowserThread::ID thread_id)
+    : ActivityLogPolicy(profile, thread_id) {
+  // We normally dispatch DB requests to the DB thread, but the thread might
+  // not exist if we are under test conditions. Substitute the UI thread for
+  // this case.
+  if (BrowserThread::IsMessageLoopValid(BrowserThread::DB)) {
+    dispatch_thread_ = BrowserThread::DB;
+  } else {
+    LOG(ERROR) << "BrowserThread::DB does not exist, running on UI thread!";
+    dispatch_thread_ = BrowserThread::UI;
+  }
+
+  db_ = new ActivityDatabase();
+  FilePath database_name = profile_base_path_.Append(
+      chrome::kExtensionActivityLogFilename);
+  KillActivityDatabaseErrorDelegate* error_delegate =
+      new KillActivityDatabaseErrorDelegate(this);
+  db_->SetErrorDelegate(error_delegate);
+  ScheduleAndForget(db_, &ActivityDatabase::Init, database_name);
+}
+
+FullStreamUIPolicy::~FullStreamUIPolicy() {
+  ScheduleAndForget(db_, &ActivityDatabase::Close);
+}
+
+// Get data as a set of key-value pairs.  The keys are policy-specific.
+void FullStreamUIPolicy::ReadData(
+    const std::string& extension_id,
+    const int day,
+    const Callback
+        <void(scoped_ptr<std::vector<scoped_refptr<Action> > >)>& callback)
+    const {
+  BrowserThread::PostTaskAndReplyWithResult(
+      dispatch_thread_,
+      FROM_HERE,
+      base::Bind(&ActivityDatabase::GetActions, Unretained(db_),
+                 extension_id, day),
+      callback);
+}
+
+void FullStreamUIPolicy::SetSaveStateOnRequestOnly() {
+  db_->SetBatchModeForTesting(false);
+  ActivityLogPolicy::SetSaveStateOnRequestOnly();
+}
+
+void FullStreamUIPolicy::GetKey(ActivityLogPolicy::KeyType key_ty,
+    std::string* key) const {
+  DCHECK(key && "Unexpected NULL pointer.");
+  switch (key_ty) {
+    case PARAM_KEY_REASON:
+      *key = kKeyReason;
+      break;
+    case PARAM_KEY_DOM_ACTION:
+      *key = kKeyDomainAction;
+      break;
+    case PARAM_KEY_URL_TITLE:
+      *key = kKeyURLTitle;
+      break;
+    case PARAM_KEY_DETAILS_STRING:
+      *key = kKeyDetailsString;
+      break;
+    default:
+      *key = "";
+  }
+}
+
+void FullStreamUIPolicy::KillActivityLogDatabase() {
+  ScheduleAndForget(db_, &ActivityDatabase::KillDatabase);
+}
+
+void FullStreamUIPolicy::ProcessArguments(
+    ActionType action_type,
+    const std::string& name,
+    const ListValue* args,
+    std::stringstream& processed_args) const {
+  if (args) {
+    ListValue::const_iterator it = args->begin();
+    for (; it != args->end(); ++it) {
+      std::string arg;
+      JSONStringValueSerializer serializer(&arg);
+      if (serializer.SerializeAndOmitBinaryValues(**it)) {
+        if (it != args->begin()) {
+          processed_args << ", ";
+        }
+        processed_args << arg;

[Matt Perry, 2013/05/30 18:55:32]

I doubt the efficiency of stringstream vs string matters much here.
- It's not called that frequently.
- Serializing a JSON value is likely more intensive than concatenating a string.
- std::string append is amortized linear time, not quadratic. Most
implementations will double in size each time they reach capacity.

[dbabic, 2013/05/30 21:51:25]

I checked the GCC 4.9 source, and string::append doesn't seem to be amortized
linear time there.  Namely, the call to _M_clone (called from reserve, called
from append) allocates each time exactly size_of_old_string + size_of_new_string
memory.  Feel free to check it yourself
(./libstdc++-v3/include/bits/basic_string.tcc).  Thus, unless I'm missing
something, running append in a loop will be quadratic.

Now, whether that matters is a completely different question.  At this point, I
don't know how many arguments we might have (Adrienne might know better).  Would
you still prefer to go with string::append?  I guess it's an
efficiency-vs-conformance question.

[Matt Perry, 2013/05/30 22:00:27]

That's surprising - the SGI docs [ http://www.sgi.com/tech/stl/basic_string.html
] claim that string concat is O(N), and they're generally trustworthy.
I still think it doesn't matter in this case. It's not something that will
happen 100s of times per second, so performance is unlikely to matter.

[dbabic, 2013/05/30 22:15:45]

I believe that SGI docs are correct, it's O(N) for a single append, but we are
doing appends in a loop.

All args will be logged only in Navitron.  In deployment, all args will be
logged only for DOM and whitelisted (privacy-irrelevant) actions.  I don't know
how many of those will get executed per second.

With that info, do you still prefer string::apply?

[Matt Perry, 2013/05/30 22:46:21]

Then it depends on what N refers to. If N is the length of the appended data,
then the full loop is still O(length of final string).
+= or append, yes.

[dbabic, 2013/05/30 22:54:41]

Done.

[dbabic, 2013/05/30 23:01:18]

Just noticed this... At least the way it's implemented in GCC, N has to refer to
the size of the length of the concatenated string, as the prefix is reallocated
and copied on each append.  Thus, complexity, at least in GCC, has to be
quadratic.  Anyways, I've changed it to string::append.

+      }

[Matt Perry, 2013/05/30 22:00:27]

On second thought - can I ask why you don't just run
SerializeAndOmitBinaryValues over args directly? Why serialize each argument
independently?

[dbabic, 2013/05/30 22:15:45]

I don't know the answer to that. I copied the previously existing code and made
it a bit more efficient by changing the implementation to stringstream.  If you
don't mind, I'll add a TODO for that --- since my code is out of sync with the
master (from this morning), I can't run tests and would prefer to avoid all
non-trivial code changes until I can run "git cl try" again.

[Matt Perry, 2013/05/30 22:46:21]

Sounds good. It looks like the JSONSerializer will add additional [] to surround
the arguments. Not sure if that's a dealbreaker for you.

[dbabic, 2013/05/30 22:54:41]

Done.

+    }
+  }
+}
+
+void FullStreamUIPolicy::ProcessWebRequestModifications(
+    DictionaryValue& details,
+    std::string& details_string) const {
+  JSONStringValueSerializer serializer(&details_string);
+  serializer.Serialize(details);
+}
+
+void FullStreamUIPolicy::ProcessAction(
+    ActionType action_type,
+    const Extension& extension,
+    const std::string& name,
+    const GURL* url_param,
+    const ListValue* args,
+    const DictionaryValue* details) {
+  std::stringstream concatenated_args;
+  ProcessArguments(action_type, name, args, concatenated_args);
+  const Time now = Time::Now();
+  // TODO(dbabic,felt) Drop the dummy string in the next revision
+  const std::string dummy;
+  GURL url_obj;
+  if (url_param) {
+    url_obj = *url_param;
+  }
+  scoped_refptr<Action> action;
+
+  switch (action_type) {
+    case ACTION_API: {
+      action = new APIAction(
+          extension.id(),
+          now,
+          APIAction::CALL,
+          name,
+          concatenated_args.str(),
+          dummy); // TODO(dbabic,felt) Drop in the next revision
+      break;
+    }
+    case ACTION_EVENT: {
+      action = new APIAction(
+          extension.id(),
+          now,
+          APIAction::EVENT_CALLBACK,
+          name,
+          concatenated_args.str(),
+          dummy); // TODO(dbabic,felt) Drop in the next revision
+      break;
+    }
+    case ACTION_BLOCKED: {
+      std::string key;
+      int reason = 0;
+      if (details) {
+        GetKey(PARAM_KEY_REASON, &key);
+        details->GetInteger(key, &reason);
+      }
+
+      action = new BlockedAction(
+          extension.id(),
+          now,
+          name,
+          concatenated_args.str(),
+          static_cast<BlockedAction::Reason>(reason),
+          dummy); // TODO(dbabic,felt) Drop in the next revision
+      break;
+    }
+    case ACTION_DOM: {
+      std::string key;
+      string16 value;
+      DOMAction::DOMActionType action_type = DOMAction::MODIFIED;
+
+      if (details) {
+        int action_id = 0;
+        GetKey(PARAM_KEY_DOM_ACTION, &key);
+        details->GetInteger(key, &action_id);
+        action_type = static_cast<DOMAction::DOMActionType>(action_id);
+        GetKey(PARAM_KEY_URL_TITLE, &key);
+        details->GetString(key, &value);
+      }
+
+      action = new DOMAction(
+          extension.id(),
+          now,
+          action_type,
+          url_obj,
+          value,
+          name,
+          concatenated_args.str(),
+          dummy); // TODO(dbabic,felt) Drop in the next revision
+      break;
+    }
+    case ACTION_WEB_REQUEST: {
+      std::string key;
+      std::string details_string;
+      if (details) {
+        scoped_ptr<DictionaryValue> copy_of_details(details->DeepCopy());
+        GetKey(PARAM_KEY_DETAILS_STRING, &key);
+        ProcessWebRequestModifications(*copy_of_details.get(), details_string);
+      }
+
+      action = new DOMAction(
+          extension.id(),
+          now,
+          DOMAction::WEBREQUEST,
+          url_obj,
+          string16(),
+          name,
+          details_string,
+          dummy); // TODO(dbabic,felt) Drop in the next revision
+      break;
+    }
+    default:
+      NOTREACHED();
+  }
+
+  ScheduleAndForget(db_, &ActivityDatabase::RecordAction, action);
+}
+
+}  // End of namespace extensions