Always report hijacked/corrupt search provider UMA even when Protector disabled.

BUG=None
TEST=Manual with chrome://histograms


Review URL: http://codereview.chromium.org/9141008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@118720 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 80 insertions, 14 deletions

diff --git a/chrome/browser/protector/base_setting_change.h b/chrome/browser/protector/base_setting_change.h
index 8601ca3..0d1cdff 100644
--- a/chrome/browser/protector/base_setting_change.h
+++ b/chrome/browser/protector/base_setting_change.h
@@ -74,9 +74,9 @@ class BaseSettingChange {
 // TODO(ivankr): CompositeSettingChange that incapsulates multiple
 // BaseSettingChange instances.
 
-// Allocates and initializes SettingChange implementation for default search
-// provider setting. Both |actual| and |backup| may be NULL if corresponding
-// values are unknown or invalid.
+// Allocates and initializes BaseSettingChange implementation for default search
+// provider setting. Reports corresponding histograms. Both |actual| and
+// |backup| may be NULL if corresponding values are unknown or invalid.
 // |backup| will be owned by the returned |BaseSettingChange| instance. |actual|
 // is not owned and is safe to destroy after Protector::ShowChange has been
 // called for the returned instance.
diff --git a/chrome/browser/protector/default_search_provider_change.cc b/chrome/browser/protector/default_search_provider_change.cc
index 6bfed3b..d7b29a8 100644
--- a/chrome/browser/protector/default_search_provider_change.cc
+++ b/chrome/browser/protector/default_search_provider_change.cc
@@ -163,17 +163,7 @@ DefaultSearchProviderChange::DefaultSearchProviderChange(
       new_search_provider_(new_search_provider),
       default_search_provider_(NULL),
       backup_search_provider_(backup_search_provider) {
-}
-
-DefaultSearchProviderChange::~DefaultSearchProviderChange() {
-  GetTemplateURLService()->RemoveObserver(this);
-}
-
-bool DefaultSearchProviderChange::Init(Profile* profile) {
-  if (!BaseSettingChange::Init(profile))
-    return false;
-
-  if (backup_search_provider_.get()) {
+  if (backup_search_provider) {
     UMA_HISTOGRAM_ENUMERATION(
         kProtectorHistogramSearchProviderHijacked,
         new_histogram_id_,
@@ -183,6 +173,19 @@ bool DefaultSearchProviderChange::Init(Profile* profile) {
         kProtectorHistogramSearchProviderCorrupt,
         new_histogram_id_,
         kProtectorMaxSearchProviderID);
+  }
+}
+
+DefaultSearchProviderChange::~DefaultSearchProviderChange() {
+  if (profile())
+    GetTemplateURLService()->RemoveObserver(this);
+}
+
+bool DefaultSearchProviderChange::Init(Profile* profile) {
+  if (!BaseSettingChange::Init(profile))
+    return false;
+
+  if (!backup_search_provider_.get()) {
     // Fallback to a prepopulated default search provider, ignoring any
     // overrides in Prefs.
     backup_search_provider_.reset(
diff --git a/chrome/browser/protector/default_search_provider_change_browsertest.cc b/chrome/browser/protector/default_search_provider_change_browsertest.cc
index 206da5b..bfc88b5 100644
--- a/chrome/browser/protector/default_search_provider_change_browsertest.cc
+++ b/chrome/browser/protector/default_search_provider_change_browsertest.cc
@@ -3,8 +3,10 @@
 // found in the LICENSE file.
 
 #include "base/memory/scoped_ptr.h"
+#include "base/metrics/histogram.h"
 #include "base/message_loop.h"
 #include "base/utf_string_conversions.h"
+#include "chrome/browser/protector/histograms.h"
 #include "chrome/browser/protector/mock_protector_service.h"
 #include "chrome/browser/protector/protector_service_factory.h"
 #include "chrome/browser/search_engines/template_url.h"
@@ -122,6 +124,17 @@ class DefaultSearchProviderChangeTest : public InProcessBrowserTest {
     EXPECT_CALL(*mock_protector_service_, OpenTab(settings_url));
   }
 
+  void ExpectHistogramCount(const std::string& name,
+                            size_t bucket,
+                            base::Histogram::Count count) {
+    base::Histogram* histogram;
+    EXPECT_TRUE(base::StatisticsRecorder::FindHistogram(name, &histogram));
+    base::Histogram::SampleSet sample;
+    histogram->SnapshotSample(&sample);
+    EXPECT_EQ(count, sample.counts(bucket)) <<
+        "Invalid " << name << " value for bucket " << bucket;
+  }
+
  protected:
   MockProtectorService* mock_protector_service_;
   TemplateURLService* turl_service_;
@@ -154,6 +167,12 @@ IN_PROC_BROWSER_TEST_F(DefaultSearchProviderChangeTest, BackupValid) {
   EXPECT_EQ(FindTemplateURL(http_example_info),
             turl_service_->GetDefaultSearchProvider());
 
+  // Verify histograms.
+  ExpectHistogramCount(kProtectorHistogramSearchProviderHijacked,
+                       SEARCH_ENGINE_OTHER, 1);
+  ExpectHistogramCount(kProtectorHistogramSearchProviderRestored,
+                       SEARCH_ENGINE_OTHER, 1);
+
   // Verify text messages.
   EXPECT_EQ(GetBubbleMessage(), change->GetBubbleMessage());
   EXPECT_EQ(GetChangeSearchButtonText(example_com),
@@ -165,11 +184,15 @@ IN_PROC_BROWSER_TEST_F(DefaultSearchProviderChangeTest, BackupValid) {
   change->Discard();
   EXPECT_EQ(FindTemplateURL(http_example_info),
             turl_service_->GetDefaultSearchProvider());
+  ExpectHistogramCount(kProtectorHistogramSearchProviderDiscarded,
+                       SEARCH_ENGINE_OTHER, 1);
 
   // Verify that Apply switches back to |current_url|.
   change->Apply();
   EXPECT_EQ(FindTemplateURL(http_example_com),
             turl_service_->GetDefaultSearchProvider());
+  ExpectHistogramCount(kProtectorHistogramSearchProviderApplied,
+                       SEARCH_ENGINE_OTHER, 1);
 }
 
 IN_PROC_BROWSER_TEST_F(DefaultSearchProviderChangeTest, BackupValidLongNames) {
@@ -238,6 +261,14 @@ IN_PROC_BROWSER_TEST_F(DefaultSearchProviderChangeTest, BackupInvalid) {
   EXPECT_EQ(FindTemplateURL(prepopulated_url_->url()->url()),
             turl_service_->GetDefaultSearchProvider());
 
+  // Verify histograms.
+  ExpectHistogramCount(kProtectorHistogramSearchProviderCorrupt,
+                       SEARCH_ENGINE_OTHER, 1);
+  ExpectHistogramCount(kProtectorHistogramSearchProviderRestored,
+                       SEARCH_ENGINE_GOOGLE, 1);
+  ExpectHistogramCount(kProtectorHistogramSearchProviderFallback,
+                       SEARCH_ENGINE_GOOGLE, 1);
+
   // Verify text messages.
   EXPECT_EQ(GetBubbleMessage(prepopulated_url_->short_name()),
             change->GetBubbleMessage());
@@ -282,6 +313,16 @@ IN_PROC_BROWSER_TEST_F(DefaultSearchProviderChangeTest,
   EXPECT_EQ(FindTemplateURL(prepopulated_url_->url()->url()),
             turl_service_->GetDefaultSearchProvider());
 
+  // Verify histograms.
+  ExpectHistogramCount(kProtectorHistogramSearchProviderCorrupt,
+                       SEARCH_ENGINE_OTHER, 1);
+  ExpectHistogramCount(kProtectorHistogramSearchProviderRestored,
+                       SEARCH_ENGINE_GOOGLE, 1);
+  ExpectHistogramCount(kProtectorHistogramSearchProviderFallback,
+                       SEARCH_ENGINE_GOOGLE, 1);
+  ExpectHistogramCount(kProtectorHistogramSearchProviderMissing,
+                       SEARCH_ENGINE_GOOGLE, 1);
+
   // Verify text messages.
   EXPECT_EQ(GetBubbleMessage(prepopulated_url_->short_name()),
             change->GetBubbleMessage());
@@ -319,6 +360,12 @@ IN_PROC_BROWSER_TEST_F(DefaultSearchProviderChangeTest,
   EXPECT_EQ(FindTemplateURL(http_example_info),
             turl_service_->GetDefaultSearchProvider());
 
+  // Verify histograms.
+  ExpectHistogramCount(kProtectorHistogramSearchProviderHijacked,
+                       SEARCH_ENGINE_OTHER, 1);
+  ExpectHistogramCount(kProtectorHistogramSearchProviderRestored,
+                       SEARCH_ENGINE_OTHER, 1);
+
   // Verify text messages.
   EXPECT_EQ(GetBubbleMessage(), change->GetBubbleMessage());
   EXPECT_EQ(GetOpenSettingsButtonText(), change->GetApplyButtonText());
@@ -352,6 +399,14 @@ IN_PROC_BROWSER_TEST_F(DefaultSearchProviderChangeTest,
   EXPECT_EQ(FindTemplateURL(prepopulated_url_->url()->url()),
             turl_service_->GetDefaultSearchProvider());
 
+  // Verify histograms.
+  ExpectHistogramCount(kProtectorHistogramSearchProviderCorrupt,
+                       SEARCH_ENGINE_OTHER, 1);
+  ExpectHistogramCount(kProtectorHistogramSearchProviderRestored,
+                       SEARCH_ENGINE_GOOGLE, 1);
+  ExpectHistogramCount(kProtectorHistogramSearchProviderFallback,
+                       SEARCH_ENGINE_GOOGLE, 1);
+
   // Verify text messages.
   EXPECT_EQ(GetBubbleMessage(prepopulated_url_->short_name()),
             change->GetBubbleMessage());
@@ -383,6 +438,14 @@ IN_PROC_BROWSER_TEST_F(DefaultSearchProviderChangeTest,
   // Verify that the default search has not changed.
   EXPECT_EQ(current_url, turl_service_->GetDefaultSearchProvider());
 
+  // Verify histograms.
+  ExpectHistogramCount(kProtectorHistogramSearchProviderCorrupt,
+                       SEARCH_ENGINE_GOOGLE, 1);
+  ExpectHistogramCount(kProtectorHistogramSearchProviderRestored,
+                       SEARCH_ENGINE_GOOGLE, 1);
+  ExpectHistogramCount(kProtectorHistogramSearchProviderFallback,
+                       SEARCH_ENGINE_GOOGLE, 1);
+
   // Verify text messages.
   EXPECT_EQ(GetBubbleMessage(prepopulated_url_->short_name()),
             change->GetBubbleMessage());