Skip to content

Commit

Permalink
This implements half of the activityLogPrivate API. The API is only a…
Browse files Browse the repository at this point in the history
…vailable to a single whitelisted extension. It includes the schema and events. It is missing the implementation of its function, which I'll add as a separate CL.

BUG=241672

Review URL: https://chromiumcodereview.appspot.com/16061002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@204796 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
felt@chromium.org committed Jun 7, 2013
1 parent 9e3000b commit a765cc1
Show file tree
Hide file tree
Showing 33 changed files with 678 additions and 79 deletions.
10 changes: 7 additions & 3 deletions chrome/browser/extensions/activity_log/activity_actions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,22 @@
#include <string>
#include "base/logging.h"
#include "base/stringprintf.h"
#include "chrome/browser/extensions/activity_log/api_actions.h"
#include "chrome/browser/extensions/activity_log/activity_actions.h"

namespace extensions {

using api::activity_log_private::ExtensionActivity;

const char* Action::kTableBasicFields =
"extension_id LONGVARCHAR NOT NULL, "
"time INTEGER NOT NULL";

Action::Action(const std::string& extension_id,
const base::Time& time)
const base::Time& time,
ExtensionActivity::ActivityType activity_type)
: extension_id_(extension_id),
time_(time) {}
time_(time),
activity_type_(activity_type) {}

// static
bool Action::InitializeTableInternal(sql::Connection* db,
Expand Down
12 changes: 11 additions & 1 deletion chrome/browser/extensions/activity_log/activity_actions.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "base/memory/ref_counted_memory.h"
#include "base/time.h"
#include "base/values.h"
#include "chrome/common/extensions/api/activity_log_private.h"
#include "sql/connection.h"
#include "sql/statement.h"
#include "sql/transaction.h"
Expand All @@ -27,14 +28,22 @@ class Action : public base::RefCountedThreadSafe<Action> {
// Record the action in the database.
virtual void Record(sql::Connection* db) = 0;

// Flatten the activity's type-specific fields into an ExtensionActivity.
virtual scoped_ptr<api::activity_log_private::ExtensionActivity>
ConvertToExtensionActivity() = 0;

// Print an action as a regular string for debugging purposes.
virtual std::string PrintForDebug() = 0;

const std::string& extension_id() const { return extension_id_; }
const base::Time& time() const { return time_; }
api::activity_log_private::ExtensionActivity::ActivityType activity_type()
const { return activity_type_; }

protected:
Action(const std::string& extension_id, const base::Time& time);
Action(const std::string& extension_id,
const base::Time& time,
api::activity_log_private::ExtensionActivity::ActivityType type);
virtual ~Action() {}

// Initialize the table for a given action type.
Expand All @@ -53,6 +62,7 @@ class Action : public base::RefCountedThreadSafe<Action> {

std::string extension_id_;
base::Time time_;
api::activity_log_private::ExtensionActivity::ActivityType activity_type_;

DISALLOW_COPY_AND_ASSIGN(Action);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,6 @@ class ActivityDatabaseTest : public ChromeRenderViewHostTestHarness {
test_user_manager_.reset(new chromeos::ScopedTestUserManager());
#endif
CommandLine command_line(CommandLine::NO_PROGRAM);
profile_ =
Profile::FromBrowserContext(web_contents()->GetBrowserContext());
extension_service_ = static_cast<TestExtensionSystem*>(
ExtensionSystem::Get(profile_))->CreateExtensionService(
&command_line, base::FilePath(), false);
CommandLine::ForCurrentProcess()->AppendSwitch(
switches::kEnableExtensionActivityLogTesting);
}
Expand All @@ -61,10 +56,6 @@ class ActivityDatabaseTest : public ChromeRenderViewHostTestHarness {
ChromeRenderViewHostTestHarness::TearDown();
}

protected:
ExtensionService* extension_service_;
Profile* profile_;

private:
#if defined OS_CHROMEOS
chromeos::ScopedStubCrosEnabler stub_cros_enabler_;
Expand Down Expand Up @@ -217,9 +208,9 @@ TEST_F(ActivityDatabaseTest, GetTodaysActions) {
activity_db->RecordAction(extra_dom_action);

// Read them back
std::string api_print = "ID: punky, CATEGORY: CALL, "
std::string api_print = "ID: punky, CATEGORY: call, "
"API: brewster, ARGS: woof";
std::string dom_print = "DOM API CALL: lets, ARGS: vamoose, VERB: MODIFIED";
std::string dom_print = "DOM API CALL: lets, ARGS: vamoose, VERB: modified";
scoped_ptr<std::vector<scoped_refptr<Action> > > actions =
activity_db->GetActions("punky", 0);
ASSERT_EQ(2, static_cast<int>(actions->size()));
Expand Down Expand Up @@ -288,9 +279,9 @@ TEST_F(ActivityDatabaseTest, GetOlderActions) {
activity_db->RecordAction(tooold_dom_action);

// Read them back
std::string api_print = "ID: punky, CATEGORY: CALL, "
std::string api_print = "ID: punky, CATEGORY: call, "
"API: brewster, ARGS: woof";
std::string dom_print = "DOM API CALL: lets, ARGS: vamoose, VERB: MODIFIED";
std::string dom_print = "DOM API CALL: lets, ARGS: vamoose, VERB: modified";
scoped_ptr<std::vector<scoped_refptr<Action> > > actions =
activity_db->GetActions("punky", 3);
ASSERT_EQ(2, static_cast<int>(actions->size()));
Expand Down
58 changes: 25 additions & 33 deletions chrome/browser/extensions/activity_log/activity_log.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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/api/activity_log_private/activity_log_private_api.h"
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/extensions/extension_system.h"
#include "chrome/browser/profiles/incognito_helpers.h"
Expand Down Expand Up @@ -41,14 +42,6 @@ std::string MakeArgList(const ListValue* args) {
return call_signature;
}

// Concatenate an API call with its arguments.
std::string MakeCallSignature(const std::string& name, const ListValue* args) {
std::string call_signature = name + "(";
call_signature += MakeArgList(args);
call_signature += ")";
return call_signature;
}

// Computes whether the activity log is enabled in this browser (controlled by
// command-line flags) and caches the value (which is assumed never to change).
class LogIsEnabled {
Expand Down Expand Up @@ -106,12 +99,9 @@ content::BrowserContext* ActivityLogFactory::GetBrowserContextToUse(

// Use GetInstance instead of directly creating an ActivityLog.
ActivityLog::ActivityLog(Profile* profile) : 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.
// It also controls whether logging statements are printed.
testing_mode_ = CommandLine::ForCurrentProcess()->HasSwitch(
switches::kEnableExtensionActivityLogTesting);
if (!testing_mode_) {
Expand All @@ -130,6 +120,8 @@ ActivityLog::ActivityLog(Profile* profile) : profile_(profile) {
dispatch_thread_ = BrowserThread::UI;
}

observers_ = new ObserverListThreadSafe<Observer>;

// 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
Expand Down Expand Up @@ -158,12 +150,11 @@ ActivityLog* ActivityLog::GetInstance(Profile* profile) {
}

void ActivityLog::AddObserver(ActivityLog::Observer* observer) {
if (!IsLogEnabled()) return;
// TODO(felt) Re-implement Observer notification HERE for the API.
observers_->AddObserver(observer);
}

void ActivityLog::RemoveObserver(ActivityLog::Observer* observer) {
// TODO(felt) Re-implement Observer notification HERE for the API.
observers_->RemoveObserver(observer);
}

void ActivityLog::LogAPIActionInternal(const std::string& extension_id,
Expand All @@ -185,9 +176,8 @@ void ActivityLog::LogAPIActionInternal(const std::string& extension_id,
MakeArgList(args),
extra);
ScheduleAndForget(&ActivityDatabase::RecordAction, action);
// TODO(felt) Re-implement Observer notification HERE for the API.
if (log_activity_to_stdout_)
LOG(INFO) << action->PrintForDebug();
observers_->Notify(&Observer::OnExtensionActivity, action);
if (testing_mode_) LOG(INFO) << action->PrintForDebug();
} else {
LOG(ERROR) << "Unknown API call! " << api_call;
}
Expand All @@ -198,7 +188,8 @@ void ActivityLog::LogAPIAction(const std::string& extension_id,
const std::string& api_call,
ListValue* args,
const std::string& extra) {
if (!IsLogEnabled()) return;
if (!IsLogEnabled() ||
ActivityLogAPI::IsExtensionWhitelisted(extension_id)) return;
if (!testing_mode_ &&
arg_whitelist_api_.find(api_call) == arg_whitelist_api_.end())
args->Clear();
Expand All @@ -217,7 +208,8 @@ void ActivityLog::LogEventAction(const std::string& extension_id,
const std::string& api_call,
ListValue* args,
const std::string& extra) {
if (!IsLogEnabled()) return;
if (!IsLogEnabled() ||
ActivityLogAPI::IsExtensionWhitelisted(extension_id)) return;
if (!testing_mode_ &&
arg_whitelist_api_.find(api_call) == arg_whitelist_api_.end())
args->Clear();
Expand All @@ -233,7 +225,8 @@ void ActivityLog::LogBlockedAction(const std::string& extension_id,
ListValue* args,
BlockedAction::Reason reason,
const std::string& extra) {
if (!IsLogEnabled()) return;
if (!IsLogEnabled() ||
ActivityLogAPI::IsExtensionWhitelisted(extension_id)) return;
if (!testing_mode_ &&
arg_whitelist_api_.find(blocked_call) == arg_whitelist_api_.end())
args->Clear();
Expand All @@ -244,9 +237,8 @@ void ActivityLog::LogBlockedAction(const std::string& extension_id,
reason,
extra);
ScheduleAndForget(&ActivityDatabase::RecordAction, action);
// TODO(felt) Re-implement Observer notification HERE for the API.
if (log_activity_to_stdout_)
LOG(INFO) << action->PrintForDebug();
observers_->Notify(&Observer::OnExtensionActivity, action);
if (testing_mode_) LOG(INFO) << action->PrintForDebug();
}

void ActivityLog::LogDOMAction(const std::string& extension_id,
Expand All @@ -256,7 +248,8 @@ void ActivityLog::LogDOMAction(const std::string& extension_id,
const ListValue* args,
DomActionType::Type call_type,
const std::string& extra) {
if (!IsLogEnabled()) return;
if (!IsLogEnabled() ||
ActivityLogAPI::IsExtensionWhitelisted(extension_id)) return;
if (call_type == DomActionType::METHOD && api_call == "XMLHttpRequest.open")
call_type = DomActionType::XHR;
scoped_refptr<DOMAction> action = new DOMAction(
Expand All @@ -269,9 +262,8 @@ void ActivityLog::LogDOMAction(const std::string& extension_id,
MakeArgList(args),
extra);
ScheduleAndForget(&ActivityDatabase::RecordAction, action);
// TODO(felt) Re-implement Observer notification HERE for the API.
if (log_activity_to_stdout_)
LOG(INFO) << action->PrintForDebug();
observers_->Notify(&Observer::OnExtensionActivity, action);
if (testing_mode_) LOG(INFO) << action->PrintForDebug();
}

void ActivityLog::LogWebRequestAction(const std::string& extension_id,
Expand All @@ -280,7 +272,8 @@ void ActivityLog::LogWebRequestAction(const std::string& extension_id,
scoped_ptr<DictionaryValue> details,
const std::string& extra) {
string16 null_title;
if (!IsLogEnabled()) return;
if (!IsLogEnabled() ||
ActivityLogAPI::IsExtensionWhitelisted(extension_id)) return;

// Strip details of the web request modifications (for privacy reasons),
// unless testing is enabled.
Expand All @@ -305,9 +298,8 @@ void ActivityLog::LogWebRequestAction(const std::string& extension_id,
details_string,
extra);
ScheduleAndForget(&ActivityDatabase::RecordAction, action);
// TODO(felt) Re-implement Observer notification HERE for the API.
if (log_activity_to_stdout_)
LOG(INFO) << action->PrintForDebug();
observers_->Notify(&Observer::OnExtensionActivity, action);
if (testing_mode_) LOG(INFO) << action->PrintForDebug();
}

void ActivityLog::GetActions(
Expand Down Expand Up @@ -340,7 +332,7 @@ void ActivityLog::OnScriptsExecuted(
for (ExecutingScriptsMap::const_iterator it = extension_ids.begin();
it != extension_ids.end(); ++it) {
const Extension* extension = extensions->GetByID(it->first);
if (!extension)
if (!extension || ActivityLogAPI::IsExtensionWhitelisted(extension->id()))
continue;

// If OnScriptsExecuted is fired because of tabs.executeScript, the list
Expand Down
12 changes: 6 additions & 6 deletions chrome/browser/extensions/activity_log/activity_log.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ class Extension;
class ActivityLog : public BrowserContextKeyedService,
public TabHelper::ScriptExecutionObserver {
public:
// Observers can listen for activity events.
// Observers can listen for activity events. There is probably only one
// observer: the activityLogPrivate API.
class Observer {
public:
virtual void OnExtensionActivity(scoped_refptr<Action> activity) = 0;
Expand All @@ -57,7 +58,8 @@ class ActivityLog : public BrowserContextKeyedService,
// really intended for use by unit tests.
static void RecomputeLoggingIsEnabled();

// Add/remove observer.
// Add/remove observer: the activityLogPrivate API only listens when the
// ActivityLog extension is registered for an event.
void AddObserver(Observer* observer);
void RemoveObserver(Observer* observer);

Expand Down Expand Up @@ -170,6 +172,7 @@ class ActivityLog : public BrowserContextKeyedService,
}

typedef ObserverListThreadSafe<Observer> ObserverList;
scoped_refptr<ObserverList> observers_;

// The database wrapper that does the actual database I/O.
// We initialize this on the same thread as the ActivityLog, but then
Expand All @@ -182,14 +185,11 @@ class ActivityLog : public BrowserContextKeyedService,
// we dispatch to the UI thread.
BrowserThread::ID dispatch_thread_;

// Whether to log activity to stdout or the UI. These are set by switches.
bool log_activity_to_stdout_;
bool log_activity_to_ui_;

// testing_mode_ controls whether to log API call arguments. By default, we
// don't log most arguments to avoid saving too much data. In testing mode,
// argument collection is enabled. We also whitelist some arguments for
// collection regardless of whether this bool is true.
// When testing_mode_ is enabled, we also print to the console.
bool testing_mode_;
base::hash_set<std::string> arg_whitelist_api_;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class ActivityLogExtensionTest : public ExtensionApiTest {
virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
ExtensionBrowserTest::SetUpCommandLine(command_line);
command_line->AppendSwitch(switches::kEnableExtensionActivityLogging);
command_line->AppendSwitch(switches::kEnableExtensionActivityLogTesting);
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ class ActivityLogTest : public testing::Test {
scoped_refptr<Action> last = i->front();
std::string id(kExtensionId);
std::string noargs = "ID: " + id + ", CATEGORY: "
"CALL, API: tabs.testMethod, ARGS: ";
"call, API: tabs.testMethod, ARGS: ";
ASSERT_EQ(noargs, last->PrintForDebug());
}

Expand All @@ -85,7 +85,7 @@ class ActivityLogTest : public testing::Test {
scoped_refptr<Action> last = i->front();
std::string id(kExtensionId);
std::string args = "ID: " + id + ", CATEGORY: "
"CALL, API: extension.connect, ARGS: \"hello\", \"world\"";
"call, API: extension.connect, ARGS: \"hello\", \"world\"";
ASSERT_EQ(args, last->PrintForDebug());
}

Expand Down
Loading

0 comments on commit a765cc1

Please sign in to comment.