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/common/extensions/extension.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 "googleurl/src/gurl.h"
+#include "sql/error_delegate_util.h"
+
+using namespace base;
+using namespace content;
+
+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)

[felt, 2013/05/24 18:43:38]

Is there enough room to indent the callback another 4 so we can see it's
continued from previous line? (I can't tell in the code review dashboard)

[dbabic, 2013/05/28 21:11:49]

Done (coppied your formatting).

+    const {
+  BrowserThread::PostTaskAndReplyWithResult(
+      dispatch_thread_,
+      FROM_HERE,
+      base::Bind(&ActivityDatabase::GetActions, Unretained(db_),
+                 extension_id, day),
+      callback);
+}
+
+void FullStreamUIPolicy::GetKey(ActivityLogPolicy::KeyType key_ty,
+    std::string& key) const {
+  switch (key_ty) {
+    case PARAM_KEY_REASON:
+      key = "fsuip.reason";

[felt, 2013/05/24 18:43:38]

Can you make these k constants? Since they're only used within the class, they
can be in an anonymous namespace in this file.

[dbabic, 2013/05/28 21:11:49]

Done.

+      break;
+    case PARAM_KEY_DOM_ACTION:
+      key = "fsuip.domact";
+      break;
+    case PARAM_KEY_URL_TITLE:
+      key = "fsuip.urltitle";
+      break;
+    case PARAM_KEY_DETAILS_STRING:
+      key = "fsuip.details";
+      break;
+    default:
+      key = "";
+  }
+}
+
+void FullStreamUIPolicy::KillActivityLogDatabase() {
+  ScheduleAndForget(db_, &ActivityDatabase::KillDatabase);
+}
+
+void FullStreamUIPolicy::ProcessArguments(

[felt, 2013/05/24 18:43:38]

Try checking out base/strings classes to see if you can find anything to replace
the stringstream:
https://code.google.com/p/chromium/codesearch#chromium/src/base/strings/&q=pa...

[dbabic, 2013/05/28 21:11:49]

I checked all the classes and macros, there doesn't seem to be anything for
efficient string concatenation.

+    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;
+      }
+    }
+  }
+}
+
+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 */);

[felt, 2013/05/24 18:43:38]

use // for consistency, here and in the other parts of the switch statement

P.S. Petr might have found a use for extras, so we might not want to drop them.
(Keep the comment though so we remember to discuss it.)

[dbabic, 2013/05/28 21:11:49]

Done.

+      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 the extensions namespace

[felt, 2013/05/24 18:43:38]

"namespace extensions"

[dbabic, 2013/05/28 21:11:49]

Done.