Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(4636)

Unified Diff: chrome/browser/extensions/activity_log/activity_log.cc

Issue 15573003: New architecture of the activity logging: Policies for summarization (and compression) (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Addressed Adrienne's final comments. Created 7 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/extensions/activity_log/activity_log.cc
diff --git a/chrome/browser/extensions/activity_log/activity_log.cc b/chrome/browser/extensions/activity_log/activity_log.cc
index 7fc3a4d17148d224e48ff6c5888f434d09bb05a5..68f1d33806aff37c9f333c8d692358a6f5f0fc4b 100644
--- a/chrome/browser/extensions/activity_log/activity_log.cc
+++ b/chrome/browser/extensions/activity_log/activity_log.cc
@@ -12,6 +12,7 @@
#include "chrome/browser/extensions/activity_log/activity_log.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/stream_noargs_ui_policy.h"
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/extensions/extension_system.h"
#include "chrome/browser/profiles/incognito_helpers.h"
@@ -86,41 +87,6 @@ void ActivityLog::RecomputeLoggingIsEnabled() {
return LogIsEnabled::GetInstance()->ComputeIsEnabled();
}
-// This handles errors from the database.
-class KillActivityDatabaseErrorDelegate : public sql::ErrorDelegate {
- public:
- explicit KillActivityDatabaseErrorDelegate(ActivityLog* 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:
- ActivityLog* backend_;
- bool scheduled_death_;
-
- DISALLOW_COPY_AND_ASSIGN(KillActivityDatabaseErrorDelegate);
-};
-
// ActivityLogFactory
ActivityLogFactory* ActivityLogFactory::GetInstance() {
@@ -140,52 +106,38 @@ content::BrowserContext* ActivityLogFactory::GetBrowserContextToUse(
// ActivityLog
// Use GetInstance instead of directly creating an ActivityLog.
-ActivityLog::ActivityLog(Profile* profile) : profile_(profile) {
+ActivityLog::ActivityLog(Profile* profile) : policy_(NULL), profile_(profile) {
// enable-extension-activity-logging and enable-extension-activity-ui
log_activity_to_stdout_ = CommandLine::ForCurrentProcess()->HasSwitch(
switches::kEnableExtensionActivityLogging);
- // enable-extension-activity-log-testing
- // This controls whether arguments are collected.
- testing_mode_ = CommandLine::ForCurrentProcess()->HasSwitch(
- switches::kEnableExtensionActivityLogTesting);
- if (!testing_mode_) {
- for (int i = 0; i < APIAction::kSizeAlwaysLog; i++) {
- arg_whitelist_api_.insert(std::string(APIAction::kAlwaysLog[i]));
- }
- }
-
// 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.
+ content::BrowserThread::ID dispatch_thread;
if (BrowserThread::IsMessageLoopValid(BrowserThread::DB)) {
- dispatch_thread_ = BrowserThread::DB;
+ dispatch_thread = BrowserThread::DB;
} else {
LOG(ERROR) << "BrowserThread::DB does not exist, running on UI thread!";
- dispatch_thread_ = BrowserThread::UI;
+ dispatch_thread = BrowserThread::UI;
}
- // If the database cannot be initialized for some reason, we keep
- // chugging along but nothing will get recorded. If the UI is
- // available, things will still get sent to the UI even if nothing
- // is being written to the database.
- db_ = new ActivityDatabase();
- if (!IsLogEnabled()) return;
- base::FilePath base_dir = profile->GetPath();
- base::FilePath database_name = base_dir.Append(
- chrome::kExtensionActivityLogFilename);
- KillActivityDatabaseErrorDelegate* error_delegate =
- new KillActivityDatabaseErrorDelegate(this);
- db_->SetErrorDelegate(error_delegate);
- ScheduleAndForget(&ActivityDatabase::Init, database_name);
+ // TODO(dbabic) In the next iteration, we should support multiple policies,
+ // which are then polled by the ActivityLog object
+ if (IsLogEnabled()) {
+ switch (policy_type_) {
+ case ActivityLogPolicy::POLICY_FULLSTREAM:
+ policy_ = new FullStreamUIPolicy(profile, dispatch_thread);
+ break;
+ case ActivityLogPolicy::POLICY_NOARGS:
+ policy_ = new StreamWithoutArgsUIPolicy(profile, dispatch_thread);
+ break;
+ }
+ }
}
ActivityLog::~ActivityLog() {
- ScheduleAndForget(&ActivityDatabase::Close);
-}
-
-void ActivityLog::SetArgumentLoggingForTesting(bool log_arguments) {
- testing_mode_ = log_arguments;
+ delete policy_;
}
// static
@@ -212,12 +164,33 @@ void ActivityLog::LogAPIActionInternal(const Extension* extension,
ListValue* args,
const std::string& extra,
const APIAction::Type type) {
+ // TODO(felt) This doesn't solve the TOCTOU problem, pass extension ID
+ // instead of a pointer to extension. That will fix the problem.
+ if (!extension)
+ return;
+
std::string verb, manager;
bool matches = RE2::FullMatch(api_call, "(.*?)\\.(.*)", &manager, &verb);
if (matches) {
if (!args->empty() && manager == "tabs") {
APIAction::LookupTabId(api_call, args, profile_);
}
+
+ if (policy_) {
+ DCHECK((type == APIAction::CALL || type == APIAction::EVENT_CALLBACK) &&
+ "Unexpected APIAction call type.");
+ policy_->ProcessAction(
+ type == APIAction::CALL ? ActivityLogPolicy::ACTION_API :
+ ActivityLogPolicy::ACTION_EVENT,
+ *extension,
+ api_call,
+ NULL,
+ args,
+ NULL);
+ }
+
+ // TODO(felt) Logging should be done more efficiently, so that it
+ // doesn't require construction of the action object.
scoped_refptr<APIAction> action = new APIAction(
extension->id(),
base::Time::Now(),
@@ -225,7 +198,6 @@ void ActivityLog::LogAPIActionInternal(const Extension* extension,
api_call,
MakeArgList(args),
extra);
- ScheduleAndForget(&ActivityDatabase::RecordAction, action);
// Display the action.
ObserverMap::const_iterator iter = observers_.find(extension);
@@ -254,10 +226,9 @@ void ActivityLog::LogAPIAction(const Extension* extension,
const std::string& api_call,
ListValue* args,
const std::string& extra) {
- if (!IsLogEnabled()) return;
- if (!testing_mode_ &&
- arg_whitelist_api_.find(api_call) == arg_whitelist_api_.end())
- args->Clear();
+ if (!IsLogEnabled() || !extension)
+ return;
+
LogAPIActionInternal(extension,
api_call,
args,
@@ -273,10 +244,9 @@ void ActivityLog::LogEventAction(const Extension* extension,
const std::string& api_call,
ListValue* args,
const std::string& extra) {
- if (!IsLogEnabled()) return;
- if (!testing_mode_ &&
- arg_whitelist_api_.find(api_call) == arg_whitelist_api_.end())
- args->Clear();
+ if (!IsLogEnabled() || !extension)
+ return;
+
LogAPIActionInternal(extension,
api_call,
args,
@@ -289,17 +259,31 @@ void ActivityLog::LogBlockedAction(const Extension* extension,
ListValue* args,
BlockedAction::Reason reason,
const std::string& extra) {
- if (!IsLogEnabled()) return;
- if (!testing_mode_ &&
- arg_whitelist_api_.find(blocked_call) == arg_whitelist_api_.end())
- args->Clear();
+ if (!IsLogEnabled() || !extension)
+ return;
+
+ if (policy_) {
+ scoped_ptr<base::DictionaryValue> details(new DictionaryValue());
+ std::string key;
+ policy_->GetKey(ActivityLogPolicy::PARAM_KEY_REASON, &key);
+ details->SetInteger(key, static_cast<int>(reason));
+ policy_->ProcessAction(
+ ActivityLogPolicy::ACTION_BLOCKED,
+ *extension,
+ blocked_call,
+ NULL,
+ args,
+ details.get());
+ }
+
+ // TODO(felt) Logging should be done more efficiently, so that it
+ // doesn't require construction of the action object.
scoped_refptr<BlockedAction> action = new BlockedAction(extension->id(),
base::Time::Now(),
blocked_call,
MakeArgList(args),
reason,
extra);
- ScheduleAndForget(&ActivityDatabase::RecordAction, action);
// Display the action.
ObserverMap::const_iterator iter = observers_.find(extension);
if (iter != observers_.end()) {
@@ -320,6 +304,28 @@ void ActivityLog::LogDOMActionInternal(const Extension* extension,
const ListValue* args,
const std::string& extra,
DOMAction::DOMActionType verb) {
+ if (!extension)
+ return;
+
+ if (policy_) {
+ scoped_ptr<base::DictionaryValue> details(new DictionaryValue());
+ std::string key;
+ policy_->GetKey(ActivityLogPolicy::PARAM_KEY_DOM_ACTION, &key);
+ details->SetInteger(key, static_cast<int>(verb));
+ policy_->GetKey(ActivityLogPolicy::PARAM_KEY_URL_TITLE, &key);
+ details->SetString(key, url_title);
+ policy_->ProcessAction(
+ ActivityLogPolicy::ACTION_DOM,
+ *extension,
+ api_call,
+ &url,
+ args,
+ details.get());
+ }
+
+
+ // TODO(felt) Logging should be done more efficiently, so that it
+ // doesn't require construction of the action object.
scoped_refptr<DOMAction> action = new DOMAction(
extension->id(),
base::Time::Now(),
@@ -329,7 +335,6 @@ void ActivityLog::LogDOMActionInternal(const Extension* extension,
api_call,
MakeArgList(args),
extra);
- ScheduleAndForget(&ActivityDatabase::RecordAction, action);
// Display the action.
ObserverMap::const_iterator iter = observers_.find(extension);
@@ -358,7 +363,9 @@ void ActivityLog::LogDOMAction(const Extension* extension,
const std::string& api_call,
const ListValue* args,
const std::string& extra) {
- if (!IsLogEnabled()) return;
+ if (!IsLogEnabled() || !extension)
+ return;
+
DOMAction::DOMActionType action = DOMAction::MODIFIED;
if (extra == "Getter") {
action = DOMAction::GETTER;
@@ -370,6 +377,7 @@ void ActivityLog::LogDOMAction(const Extension* extension,
} else if (extra == "Method") {
action = DOMAction::METHOD;
}
+
LogDOMActionInternal(extension,
url,
url_title,
@@ -385,21 +393,29 @@ void ActivityLog::LogWebRequestAction(const Extension* extension,
scoped_ptr<DictionaryValue> details,
const std::string& extra) {
string16 null_title;
- if (!IsLogEnabled()) return;
+ if (!IsLogEnabled() || !extension)
+ return;
- // Strip details of the web request modifications (for privacy reasons),
- // unless testing is enabled.
- if (!testing_mode_) {
- DictionaryValue::Iterator details_iterator(*details);
- while (!details_iterator.IsAtEnd()) {
- details->SetBoolean(details_iterator.key(), true);
- details_iterator.Advance();
- }
- }
std::string details_string;
+ if (policy_) {
+ scoped_ptr<base::DictionaryValue> details(new DictionaryValue());
+ std::string key;
+ policy_->GetKey(ActivityLogPolicy::PARAM_KEY_DETAILS_STRING, &key);
+ details->SetString(key, details_string);
+ policy_->ProcessAction(
+ ActivityLogPolicy::ACTION_WEB_REQUEST,
+ *extension,
+ api_call,
+ &url,
+ NULL,
+ details.get());
+ }
+
JSONStringValueSerializer serializer(&details_string);
serializer.SerializeAndOmitBinaryValues(*details);
+ // TODO(felt) Logging should be done more efficiently, so that it
+ // doesn't require construction of the action object.
scoped_refptr<DOMAction> action = new DOMAction(
extension->id(),
base::Time::Now(),
@@ -409,7 +425,6 @@ void ActivityLog::LogWebRequestAction(const Extension* extension,
api_call,
details_string,
extra);
- ScheduleAndForget(&ActivityDatabase::RecordAction, action);
// Display the action.
ObserverMap::const_iterator iter = observers_.find(extension);
@@ -428,14 +443,9 @@ void ActivityLog::GetActions(
const int day,
const base::Callback
<void(scoped_ptr<std::vector<scoped_refptr<Action> > >)>& callback) {
- BrowserThread::PostTaskAndReplyWithResult(
- dispatch_thread_,
- FROM_HERE,
- base::Bind(&ActivityDatabase::GetActions,
- base::Unretained(db_),
- extension_id,
- day),
- callback);
+ if (policy_) {
+ policy_->ReadData(extension_id, day, callback);
+ }
}
void ActivityLog::OnScriptsExecuted(
@@ -474,16 +484,12 @@ void ActivityLog::OnScriptsExecuted(
web_contents->GetTitle(),
std::string(), // no api call here
script_names.get(),
- std::string(), // no extras either
- DOMAction::INSERTED);
+ std::string(),
+ DOMAction::INSERTED); // no extras either
}
}
}
-void ActivityLog::KillActivityLogDatabase() {
- ScheduleAndForget(&ActivityDatabase::KillDatabase);
-}
-
// static
const char* ActivityLog::ActivityToString(Activity activity) {
switch (activity) {
@@ -501,4 +507,7 @@ const char* ActivityLog::ActivityToString(Activity activity) {
}
}
+ActivityLogPolicy::PolicyType ActivityLog::policy_type_(
+ ActivityLogPolicy::POLICY_NOARGS);
+
} // namespace extensions

Powered by Google App Engine
This is Rietveld 408576698