Introduce ActiveScriptController; track active extension scripts

chrome/browser/extensions/activity_log/uma_policy.cc

Index: chrome/browser/extensions/activity_log/uma_policy.cc
diff --git a/chrome/browser/extensions/activity_log/uma_policy.cc b/chrome/browser/extensions/activity_log/uma_policy.cc
index e94f781eac05e5f87fd467b3f35efe9a93b252cb..549b3ed52ca660efc7eeddf6241d72e0f3557d8b 100644
--- a/chrome/browser/extensions/activity_log/uma_policy.cc
+++ b/chrome/browser/extensions/activity_log/uma_policy.cc
@@ -7,8 +7,11 @@
 #include "base/metrics/histogram.h"
 #include "base/strings/stringprintf.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/extensions/active_script_controller.h"
 #include "chrome/browser/extensions/activity_log/activity_action_constants.h"
 #include "chrome/browser/extensions/activity_log/ad_network_database.h"
+#include "chrome/browser/extensions/location_bar_controller.h"
+#include "chrome/browser/extensions/tab_helper.h"
 #include "chrome/browser/sessions/session_id.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
@@ -41,6 +44,9 @@ const int kAdInjected         = 1 << UmaPolicy::AD_INJECTED;
 const int kAdRemoved          = 1 << UmaPolicy::AD_REMOVED;
 const int kAdReplaced         = 1 << UmaPolicy::AD_REPLACED;
 
+// A mask of all the ad injection flags.
+const int kAnyAdInjection = kAdInjected | kAdRemoved | kAdReplaced;

[not at google - send to devlin, 2014/05/07 22:49:02]

how about kAnyAdActivity?

[Devlin, 2014/05/08 18:15:46]

Sure.

+
 }  // namespace
 
 // Class constants, also used in testing. --------------------------------------
@@ -164,7 +170,8 @@ int UmaPolicy::MatchActionToStatus(scoped_refptr<Action> action) {
   return ret_bit;
 }
 
-void UmaPolicy::HistogramOnClose(const std::string& url) {
+void UmaPolicy::HistogramOnClose(const std::string& url,
+                                 content::WebContents* web_contents) {
   // Let's try to avoid histogramming useless URLs.
   if (url.empty() || url == content::kAboutBlankURL ||
       url == chrome::kChromeUINewTabURL)
@@ -173,16 +180,31 @@ void UmaPolicy::HistogramOnClose(const std::string& url) {
   int statuses[MAX_STATUS - 1];
   std::memset(statuses, 0, sizeof(statuses));
 
+  TabHelper* tab_helper = TabHelper::FromWebContents(web_contents);
+  ActiveScriptController* active_script_controller =

[not at google - send to devlin, 2014/05/07 22:49:02]

maybe ActiveScriptController should be a WebContentsUserData rather than hanging
off TabHelper.

related point, at a high level having the LocationBarController own the
ActiveScriptController doesn't seem right. It's a separate entity, or at least
that's how I'd like to think about it.

Similar comment made in the other file.

[Devlin, 2014/05/08 18:15:46]

Talked about offline, and decided to keep current structure.

+      tab_helper && tab_helper->location_bar_controller() ?
+          tab_helper->location_bar_controller()->active_script_controller() :
+          NULL;
   SiteMap::iterator site_lookup = url_status_.find(url);
-  ExtensionMap exts = site_lookup->second;
-  ExtensionMap::iterator ext_iter;
-  for (ext_iter = exts.begin(); ext_iter != exts.end(); ++ext_iter) {
+  const ExtensionMap& exts = site_lookup->second;
+  for (ExtensionMap::const_iterator ext_iter = exts.begin();
+       ext_iter != exts.end();
+       ++ext_iter) {
     if (ext_iter->first == kNumberOfTabs)
       continue;
     for (int i = NONE + 1; i < MAX_STATUS; ++i) {
       if (ext_iter->second & (1 << i))
         statuses[i-1]++;
     }
+
+    if ((ext_iter->second & kAnyAdInjection) &&
+        active_script_controller) {
+      bool injection_could_have_been_stopped =
+          active_script_controller->HasActionForExtensionId(ext_iter->first);
+      UMA_HISTOGRAM_BOOLEAN(
+          "ExtensionActivity.AdInjectionCouldHaveBeenStopped",
+          injection_could_have_been_stopped);
+    }
   }
 
   std::string prefix = "ExtensionActivity.";
@@ -292,7 +314,7 @@ void UmaPolicy::TabChangedAt(content::WebContents* contents,
 
   // Is this an existing tab whose URL has changed.
   if (tab_it != tab_list_.end()) {
-    CleanupClosedPage(tab_it->second);
+    CleanupClosedPage(tab_it->second, contents);
     tab_list_.erase(tab_id);
   }
 
@@ -319,20 +341,21 @@ void UmaPolicy::TabClosingAt(TabStripModel* tab_strip_model,
   if (tab_it != tab_list_.end())
     tab_list_.erase(tab_id);
 
-  CleanupClosedPage(url);
+  CleanupClosedPage(url, contents);
 }
 
 void UmaPolicy::SetupOpenedPage(const std::string& url) {
   url_status_[url][kNumberOfTabs]++;
 }
 
-void UmaPolicy::CleanupClosedPage(const std::string& url) {
+void UmaPolicy::CleanupClosedPage(const std::string& url,
+                                  content::WebContents* web_contents) {
   SiteMap::iterator old_site_lookup = url_status_.find(url);
   if (old_site_lookup == url_status_.end())
     return;
   old_site_lookup->second[kNumberOfTabs]--;
   if (old_site_lookup->second[kNumberOfTabs] == 0) {
-    HistogramOnClose(url);
+    HistogramOnClose(url, web_contents);
     url_status_.erase(url);
   }
 }