Sets correct ActivityLog enabled status to the first renderer process

chrome/browser/extensions/activity_log/activity_log.cc

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 7eac24f596c1e10fd2aff8f89cfc5872aae5b042..d59130018af4679e476f69c7f4e78fd57c0f78e5 100644
--- a/chrome/browser/extensions/activity_log/activity_log.cc
+++ b/chrome/browser/extensions/activity_log/activity_log.cc
@@ -18,12 +18,14 @@
 #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/prefs/pref_service_syncable.h"
 #include "chrome/browser/prerender/prerender_manager.h"
 #include "chrome/browser/prerender/prerender_manager_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 "chrome/common/pref_names.h"
 #include "components/browser_context_keyed_service/browser_context_dependency_manager.h"
 #include "content/public/browser/web_contents.h"
 #include "third_party/re2/re2/re2.h"
@@ -163,45 +165,43 @@ ActivityLog::ActivityLog(Profile* profile)
       policy_type_(ActivityLogPolicy::POLICY_INVALID),
       profile_(profile),
       enabled_(false),
-      initialized_(false),
       policy_chosen_(false),
       testing_mode_(false),
-      has_threads_(true),
-      tracker_(NULL) {
+      tracker_(NULL),
+      activity_log_extension_enabled_(false) {
   // This controls whether logging statements are printed, which policy is set,
   // etc.
   testing_mode_ = CommandLine::ForCurrentProcess()->HasSwitch(
     switches::kEnableExtensionActivityLogTesting);
 
+  activity_log_extension_enabled_ =

[felt, 2013/07/18 20:42:03]

Can you also add a comment here documenting what this is checking (is the
extension installed & enabled)?

[pmarch, 2013/07/18 21:37:14]

Done.

+    profile_->GetPrefs()->GetBoolean(prefs::kActivityLogExtensionEnabled);
+
   // 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;
-  } else {
-    enabled_ = IsLogEnabledOnAnyProfile();
-    ExtensionSystem::Get(profile_)->ready().Post(
-      FROM_HERE, base::Bind(&ActivityLog::Init, base::Unretained(this)));
+    return;

[felt, 2013/07/18 20:42:03]

What if the extension is installed later, causing enabled_ to be set to true? I
think we still need the has_threads_ check to make sure that we don't end up
with enabled_=true when has_threads_=false.

[pmarch, 2013/07/18 21:37:14]

nice catch, thanks

   }
 
+  enabled_ = IsLogEnabledOnAnyProfile();

[felt, 2013/07/18 20:42:03]

And another comment here specifying that this is checking for the flag?

[pmarch, 2013/07/18 21:37:14]

Done.

   observers_ = new ObserverListThreadSafe<Observer>;
-}
 
-void ActivityLog::Init() {
-  DCHECK(has_threads_);
-  DCHECK(!initialized_);
-  const Extension* whitelisted_extension = ExtensionSystem::Get(profile_)->
-      extension_service()->GetExtensionById(kActivityLogExtensionId, false);
-  if (whitelisted_extension) {
+  if (activity_log_extension_enabled_) {
     enabled_ = true;
     LogIsEnabled::GetInstance()->SetProfileEnabled(true);
   }
+  ExtensionSystem::Get(profile_)->ready().Post(
+      FROM_HERE,
+      base::Bind(&ActivityLog::InitInstallTracker, base::Unretained(this)));
+  ChooseDefaultPolicy();
+}
+
+void ActivityLog::InitInstallTracker() {
   tracker_ = InstallTrackerFactory::GetForProfile(profile_);
   tracker_->AddObserver(this);
-  ChooseDefaultPolicy();
-  initialized_ = true;
 }
 
 void ActivityLog::ChooseDefaultPolicy() {
@@ -222,13 +222,16 @@ ActivityLog::~ActivityLog() {
 }
 
 bool ActivityLog::IsLogEnabled() {
-  if (!has_threads_ || !initialized_) return false;
   return enabled_;
 }
 
 void ActivityLog::OnExtensionLoaded(const Extension* extension) {
   if (extension->id() != kActivityLogExtensionId) return;
   enabled_ = true;
+  if (!activity_log_extension_enabled_) {
+    activity_log_extension_enabled_ = true;
+    profile_->GetPrefs()->SetBoolean(prefs::kActivityLogExtensionEnabled, true);
+  }
   LogIsEnabled::GetInstance()->SetProfileEnabled(true);
   ChooseDefaultPolicy();
 }
@@ -238,6 +241,11 @@ void ActivityLog::OnExtensionUnloaded(const Extension* extension) {
   if (!CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kEnableExtensionActivityLogging))
     enabled_ = false;
+  if (activity_log_extension_enabled_) {
+    activity_log_extension_enabled_ = false;
+    profile_->GetPrefs()->SetBoolean(prefs::kActivityLogExtensionEnabled,
+                                     false);
+  }
 }
 
 // static
@@ -511,4 +519,13 @@ void ActivityLog::OnScriptsExecuted(
   }
 }
 
+// static
+void ActivityLog::RegisterProfilePrefs(
+    user_prefs::PrefRegistrySyncable* registry) {
+  registry->RegisterBooleanPref(
+      prefs::kActivityLogExtensionEnabled,
+      false,
+      user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
+}
+
 }  // namespace extensions