diff options
Diffstat (limited to 'chrome/browser')
6 files changed, 180 insertions, 60 deletions
diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc index 6653d99..f11e26f 100644 --- a/chrome/browser/chrome_content_browser_client.cc +++ b/chrome/browser/chrome_content_browser_client.cc @@ -673,9 +673,9 @@ content::WebContentsViewDelegate* // Check if the extension activity log is enabled for the profile. static bool IsExtensionActivityLogEnabledForProfile(Profile* profile) { - extensions::ActivityLog* activity_log = - extensions::ActivityLog::GetInstance(profile); - return activity_log->IsLogEnabled(); + // crbug.com/247908 - This should be IsLogEnabled except for an issue + // in chrome_frame_net_tests + return extensions::ActivityLog::IsLogEnabledOnAnyProfile(); } void ChromeContentBrowserClient::GuestWebContentsAttached( diff --git a/chrome/browser/extensions/activity_log/activity_log.cc b/chrome/browser/extensions/activity_log/activity_log.cc index 9403318e2..4be4e77 100644 --- a/chrome/browser/extensions/activity_log/activity_log.cc +++ b/chrome/browser/extensions/activity_log/activity_log.cc @@ -15,10 +15,13 @@ #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/extensions/extension_system_factory.h" +#include "chrome/browser/extensions/install_tracker_factory.h" #include "chrome/browser/profiles/incognito_helpers.h" #include "chrome/common/chrome_constants.h" #include "chrome/common/chrome_switches.h" #include "chrome/common/extensions/extension.h" +#include "components/browser_context_keyed_service/browser_context_dependency_manager.h" #include "content/public/browser/web_contents.h" #include "googleurl/src/gurl.h" #include "third_party/re2/re2/re2.h" @@ -41,16 +44,21 @@ std::string MakeArgList(const ListValue* args) { 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). +// This is a hack for AL callers who don't have access to a profile object +// when deciding whether or not to do the work required for logging. The state +// is accessed through the static ActivityLog::IsLogEnabledOnAnyProfile() +// method. It returns true if --enable-extension-activity-logging is set on the +// command line OR *ANY* profile has the activity log whitelisted extension +// installed. class LogIsEnabled { public: - LogIsEnabled() { - ComputeIsEnabled(); + LogIsEnabled() : any_profile_enabled_(false) { + ComputeIsFlagEnabled(); } - void ComputeIsEnabled() { - enabled_ = CommandLine::ForCurrentProcess()-> + void ComputeIsFlagEnabled() { + base::AutoLock auto_lock(lock_); + cmd_line_enabled_ = CommandLine::ForCurrentProcess()-> HasSwitch(switches::kEnableExtensionActivityLogging); } @@ -58,10 +66,20 @@ class LogIsEnabled { return Singleton<LogIsEnabled>::get(); } - bool enabled() { return enabled_; } + bool IsEnabled() { + base::AutoLock auto_lock(lock_); + return cmd_line_enabled_ || any_profile_enabled_; + } + + void SetProfileEnabled(bool any_profile_enabled) { + base::AutoLock auto_lock(lock_); + any_profile_enabled_ = any_profile_enabled; + } private: - bool enabled_; + base::Lock lock_; + bool any_profile_enabled_; + bool cmd_line_enabled_; }; } // namespace @@ -69,13 +87,14 @@ class LogIsEnabled { namespace extensions { // static -bool ActivityLog::IsLogEnabled() { - return LogIsEnabled::GetInstance()->enabled(); +bool ActivityLog::IsLogEnabledOnAnyProfile() { + return LogIsEnabled::GetInstance()->IsEnabled(); } // static -void ActivityLog::RecomputeLoggingIsEnabled() { - return LogIsEnabled::GetInstance()->ComputeIsEnabled(); +void ActivityLog::RecomputeLoggingIsEnabled(bool profile_enabled) { + LogIsEnabled::GetInstance()->ComputeIsFlagEnabled(); + LogIsEnabled::GetInstance()->SetProfileEnabled(profile_enabled); } // ActivityLogFactory @@ -94,10 +113,26 @@ content::BrowserContext* ActivityLogFactory::GetBrowserContextToUse( return chrome::GetBrowserContextRedirectedInIncognito(context); } +ActivityLogFactory::ActivityLogFactory() + : BrowserContextKeyedServiceFactory( + "ActivityLog", + BrowserContextDependencyManager::GetInstance()) { + DependsOn(ExtensionSystemFactory::GetInstance()); + DependsOn(InstallTrackerFactory::GetInstance()); +} + +ActivityLogFactory::~ActivityLogFactory() { +} + // ActivityLog // Use GetInstance instead of directly creating an ActivityLog. -ActivityLog::ActivityLog(Profile* profile) : profile_(profile) { +ActivityLog::ActivityLog(Profile* profile) + : profile_(profile), + first_time_checking_(true), + has_threads_(true) { + enabled_ = IsLogEnabledOnAnyProfile(); + // enable-extension-activity-log-testing // This controls whether arguments are collected. // It also controls whether logging statements are printed. @@ -109,35 +144,80 @@ ActivityLog::ActivityLog(Profile* profile) : profile_(profile) { } } - // 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; + // Check that the right threads exist. If not, we shouldn't try to do things + // that require them. + if (!BrowserThread::IsMessageLoopValid(BrowserThread::DB) || + !BrowserThread::IsMessageLoopValid(BrowserThread::FILE) || + !BrowserThread::IsMessageLoopValid(BrowserThread::IO)) { + LOG(ERROR) << "Missing threads, disabling Activity Logging!"; + has_threads_ = false; } 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 + // We initialize the database whether or not the AL is enabled, since we might + // be enabled later on. 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; + if (!has_threads_) return; base::FilePath base_dir = profile->GetPath(); base::FilePath database_name = base_dir.Append( chrome::kExtensionActivityLogFilename); ScheduleAndForget(&ActivityDatabase::Init, database_name); } +void ActivityLog::Shutdown() { + if (tracker_ && !first_time_checking_) tracker_->RemoveObserver(this); +} + ActivityLog::~ActivityLog() { - if (dispatch_thread_ == BrowserThread::UI) // Cleanup fast in a unittest. - db_->Close(); - else + if (has_threads_) ScheduleAndForget(&ActivityDatabase::Close); + else + db_->Close(); +} + +// We can't register for the InstallTrackerFactory events or talk to the +// extension service in the constructor, so we do that here the first time +// this is called (as identified by first_time_checking_). +bool ActivityLog::IsLogEnabled() { + if (!first_time_checking_) return enabled_; + if (!has_threads_) return false; + tracker_ = InstallTrackerFactory::GetForProfile(profile_); + tracker_->AddObserver(this); + const ExtensionService* extension_service = + ExtensionSystem::Get(profile_)->extension_service(); + if (extension_service->IsExtensionEnabled( + kActivityLogExtensionId)) { + enabled_ = true; + LogIsEnabled::GetInstance()->SetProfileEnabled(true); + } + first_time_checking_ = false; + return enabled_; +} + +void ActivityLog::OnExtensionInstalled(const Extension* extension) { + if (extension->id() != kActivityLogExtensionId) return; + enabled_ = true; + LogIsEnabled::GetInstance()->SetProfileEnabled(true); +} + +void ActivityLog::OnExtensionUninstalled(const Extension* extension) { + if (extension->id() != kActivityLogExtensionId) return; + if (!CommandLine::ForCurrentProcess()->HasSwitch( + switches::kEnableExtensionActivityLogging)) + enabled_ = false; +} + +// Counter-intuitively, OnExtensionInstalled is also called when an extension +// is reenabled. +void ActivityLog::OnExtensionDisabled(const Extension* extension) { + if (extension->id() != kActivityLogExtensionId) return; + if (!CommandLine::ForCurrentProcess()->HasSwitch( + switches::kEnableExtensionActivityLogging)) + enabled_ = false; } void ActivityLog::SetArgumentLoggingForTesting(bool log_arguments) { @@ -307,8 +387,9 @@ void ActivityLog::GetActions( const int day, const base::Callback <void(scoped_ptr<std::vector<scoped_refptr<Action> > >)>& callback) { + if (!has_threads_) return; BrowserThread::PostTaskAndReplyWithResult( - dispatch_thread_, + BrowserThread::DB, FROM_HERE, base::Bind(&ActivityDatabase::GetActions, base::Unretained(db_), diff --git a/chrome/browser/extensions/activity_log/activity_log.h b/chrome/browser/extensions/activity_log/activity_log.h index 2aa84bb..9b6fb77 100644 --- a/chrome/browser/extensions/activity_log/activity_log.h +++ b/chrome/browser/extensions/activity_log/activity_log.h @@ -19,6 +19,8 @@ #include "base/threading/thread.h" #include "chrome/browser/extensions/activity_log/activity_actions.h" #include "chrome/browser/extensions/activity_log/activity_database.h" +#include "chrome/browser/extensions/install_observer.h" +#include "chrome/browser/extensions/install_tracker.h" #include "chrome/browser/extensions/tab_helper.h" #include "chrome/browser/profiles/profile.h" #include "chrome/common/extensions/dom_action_types.h" @@ -36,7 +38,8 @@ class Extension; // A utility for tracing interesting activity for each extension. // It writes to an ActivityDatabase on a separate thread to record the activity. class ActivityLog : public BrowserContextKeyedService, - public TabHelper::ScriptExecutionObserver { + public TabHelper::ScriptExecutionObserver, + public InstallObserver { public: // Observers can listen for activity events. There is probably only one // observer: the activityLogPrivate API. @@ -49,14 +52,15 @@ class ActivityLog : public BrowserContextKeyedService, // use GetInstance instead. static ActivityLog* GetInstance(Profile* profile); - // Currently, we only want to record actions if the user has opted in to the - // ActivityLog feature. - static bool IsLogEnabled(); + // Provides up-to-date information about whether the AL is enabled for a + // profile. The AL is enabled if the user has installed the whitelisted + // AL extension *or* set the --enable-extension-activity-logging flag. + bool IsLogEnabled(); - // Recompute whether logging should be enabled (the value of IsLogEnabled is - // normally cached). WARNING: This may not be thread-safe, and is only - // really intended for use by unit tests. - static void RecomputeLoggingIsEnabled(); + // If you want to know whether the log is enabled but DON'T have a profile + // object yet, use this method. However, it's preferable for the caller to + // use IsLogEnabled when possible. + static bool IsLogEnabledOnAnyProfile(); // Add/remove observer: the activityLogPrivate API only listens when the // ActivityLog extension is registered for an event. @@ -115,8 +119,35 @@ class ActivityLog : public BrowserContextKeyedService, <void(scoped_ptr<std::vector<scoped_refptr<Action> > >)>& callback); + // Extension::InstallObserver + // We keep track of whether the whitelisted extension is installed; if it is, + // we want to recompute whether to have logging enabled. + virtual void OnExtensionInstalled( + const extensions::Extension* extension) OVERRIDE; + virtual void OnExtensionUninstalled( + const extensions::Extension* extension) OVERRIDE; + virtual void OnExtensionDisabled( + const extensions::Extension* extension) OVERRIDE; + // We also have to list the following from InstallObserver. + virtual void OnBeginExtensionInstall(const std::string& extension_id, + const std::string& extension_name, + const gfx::ImageSkia& installing_icon, + bool is_app, + bool is_platform_app) OVERRIDE {} + virtual void OnDownloadProgress(const std::string& extension_id, + int percent_downloaded) OVERRIDE {} + virtual void OnInstallFailure(const std::string& extension_id) OVERRIDE {} + virtual void OnAppsReordered() OVERRIDE {} + virtual void OnAppInstalledToAppList( + const std::string& extension_id) OVERRIDE {} + virtual void OnShutdown() OVERRIDE {} + // For unit tests only. void SetArgumentLoggingForTesting(bool log_arguments); + static void RecomputeLoggingIsEnabled(bool profile_enabled); + + // BrowserContextKeyedService + virtual void Shutdown() OVERRIDE; private: friend class ActivityLogFactory; @@ -146,21 +177,24 @@ class ActivityLog : public BrowserContextKeyedService, // exist, which should only happen in tests where there is no DB thread. template<typename DatabaseFunc> void ScheduleAndForget(DatabaseFunc func) { - BrowserThread::PostTask(dispatch_thread_, + if (!has_threads_) return; + BrowserThread::PostTask(BrowserThread::DB, FROM_HERE, base::Bind(func, base::Unretained(db_))); } template<typename DatabaseFunc, typename ArgA> void ScheduleAndForget(DatabaseFunc func, ArgA a) { - BrowserThread::PostTask(dispatch_thread_, + if (!has_threads_) return; + BrowserThread::PostTask(BrowserThread::DB, FROM_HERE, base::Bind(func, base::Unretained(db_), a)); } template<typename DatabaseFunc, typename ArgA, typename ArgB> void ScheduleAndForget(DatabaseFunc func, ArgA a, ArgB b) { - BrowserThread::PostTask(dispatch_thread_, + if (!has_threads_) return; + BrowserThread::PostTask(BrowserThread::DB, FROM_HERE, base::Bind(func, base::Unretained(db_), a, b)); } @@ -175,10 +209,6 @@ class ActivityLog : public BrowserContextKeyedService, // commits suicide. extensions::ActivityDatabase* db_; - // Normally the DB thread. In some cases (tests), it might not exist - // we dispatch to the UI thread. - BrowserThread::ID dispatch_thread_; - // 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 @@ -188,6 +218,14 @@ class ActivityLog : public BrowserContextKeyedService, base::hash_set<std::string> arg_whitelist_api_; Profile* profile_; + bool enabled_; + bool first_time_checking_; + InstallTracker* tracker_; + + // We need the DB, FILE, and IO threads to operate. In some cases (tests), + // these threads might not exist, so we avoid dispatching anything to the + // ActivityDatabase to prevent things from exploding. + bool has_threads_; DISALLOW_COPY_AND_ASSIGN(ActivityLog); }; @@ -205,11 +243,8 @@ class ActivityLogFactory : public BrowserContextKeyedServiceFactory { private: friend struct DefaultSingletonTraits<ActivityLogFactory>; - ActivityLogFactory() - : BrowserContextKeyedServiceFactory( - "ActivityLog", - BrowserContextDependencyManager::GetInstance()) {} - virtual ~ActivityLogFactory() {} + ActivityLogFactory(); + virtual ~ActivityLogFactory(); virtual BrowserContextKeyedService* BuildServiceInstanceFor( content::BrowserContext* profile) const OVERRIDE; diff --git a/chrome/browser/extensions/activity_log/activity_log_unittest.cc b/chrome/browser/extensions/activity_log/activity_log_unittest.cc index dacdfc5..b4c8180 100644 --- a/chrome/browser/extensions/activity_log/activity_log_unittest.cc +++ b/chrome/browser/extensions/activity_log/activity_log_unittest.cc @@ -49,7 +49,10 @@ class ActivityLogTest : public testing::Test { switches::kEnableExtensionActivityLogging); CommandLine::ForCurrentProcess()->AppendSwitch( switches::kEnableExtensionActivityLogTesting); - ActivityLog::RecomputeLoggingIsEnabled(); + extension_service_ = static_cast<TestExtensionSystem*>( + ExtensionSystem::Get(profile_.get()))->CreateExtensionService + (&command_line, base::FilePath(), false); + ActivityLog::RecomputeLoggingIsEnabled(false); } virtual ~ActivityLogTest() { @@ -58,9 +61,10 @@ class ActivityLogTest : public testing::Test { #endif base::RunLoop().RunUntilIdle(); profile_.reset(NULL); + base::RunLoop().RunUntilIdle(); // Restore the original command line and undo the affects of SetUp(). *CommandLine::ForCurrentProcess() = saved_cmdline_; - ActivityLog::RecomputeLoggingIsEnabled(); + ActivityLog::RecomputeLoggingIsEnabled(false); } static void RetrieveActions_LogAndFetchActions( @@ -106,13 +110,13 @@ class ActivityLogTest : public testing::Test { }; TEST_F(ActivityLogTest, Enabled) { - ASSERT_TRUE(ActivityLog::IsLogEnabled()); + ASSERT_TRUE(ActivityLog::IsLogEnabledOnAnyProfile()); } TEST_F(ActivityLogTest, Construct) { ActivityLog* activity_log = ActivityLog::GetInstance(profile_.get()); scoped_ptr<ListValue> args(new ListValue()); - ASSERT_TRUE(ActivityLog::IsLogEnabled()); + ASSERT_TRUE(activity_log->IsLogEnabled()); activity_log->LogAPIAction( kExtensionId, std::string("tabs.testMethod"), args.get(), std::string()); } @@ -120,7 +124,7 @@ TEST_F(ActivityLogTest, Construct) { TEST_F(ActivityLogTest, LogAndFetchActions) { ActivityLog* activity_log = ActivityLog::GetInstance(profile_.get()); scoped_ptr<ListValue> args(new ListValue()); - ASSERT_TRUE(ActivityLog::IsLogEnabled()); + ASSERT_TRUE(activity_log->IsLogEnabled()); // Write some API calls activity_log->LogAPIAction( @@ -141,7 +145,7 @@ TEST_F(ActivityLogTest, LogAndFetchActions) { TEST_F(ActivityLogTest, LogWithoutArguments) { ActivityLog* activity_log = ActivityLog::GetInstance(profile_.get()); activity_log->SetArgumentLoggingForTesting(false); - ASSERT_TRUE(ActivityLog::IsLogEnabled()); + ASSERT_TRUE(activity_log->IsLogEnabled()); scoped_ptr<ListValue> args(new ListValue()); args->Set(0, new base::StringValue("hello")); @@ -154,7 +158,7 @@ TEST_F(ActivityLogTest, LogWithoutArguments) { TEST_F(ActivityLogTest, LogWithArguments) { ActivityLog* activity_log = ActivityLog::GetInstance(profile_.get()); - ASSERT_TRUE(ActivityLog::IsLogEnabled()); + ASSERT_TRUE(activity_log->IsLogEnabled()); scoped_ptr<ListValue> args(new ListValue()); args->Set(0, new base::StringValue("hello")); diff --git a/chrome/browser/extensions/api/web_request/web_request_api.cc b/chrome/browser/extensions/api/web_request/web_request_api.cc index c631011..745c19c 100644 --- a/chrome/browser/extensions/api/web_request/web_request_api.cc +++ b/chrome/browser/extensions/api/web_request/web_request_api.cc @@ -1638,7 +1638,7 @@ void ExtensionWebRequestEventRouter::DecrementBlockCount( helpers::EventResponseDelta* delta = CalculateDelta(&blocked_request, response); - if (extensions::ActivityLog::IsLogEnabled()) { + if (extensions::ActivityLog::IsLogEnabledOnAnyProfile()) { LogExtensionActivity(static_cast<Profile*>(profile), extension_id, blocked_request.request->url(), diff --git a/chrome/browser/extensions/event_router.cc b/chrome/browser/extensions/event_router.cc index 43c34da..8cf1f4b 100644 --- a/chrome/browser/extensions/event_router.cc +++ b/chrome/browser/extensions/event_router.cc @@ -122,7 +122,7 @@ void EventRouter::DispatchExtensionMessage(IPC::Sender* ipc_sender, ListValue* event_args, UserGestureState user_gesture, const EventFilteringInfo& info) { - if (ActivityLog::IsLogEnabled()) { + if (ActivityLog::IsLogEnabledOnAnyProfile()) { LogExtensionEventMessage(profile_id, extension_id, event_name, scoped_ptr<ListValue>(event_args->DeepCopy())); } |