Updating histograms to allow for experiments & log origin-based histograms.

R=cbentzel@chromium.org, dominich@chromium.org
Review URL: http://codereview.chromium.org/7289020

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

chrome/browser/prerender/prerender_browsertest.cc

@@ -98,7 +98,8 @@ class TestPrerenderContents : public PrerenderContents {
       int number_of_loads,
       FinalStatus expected_final_status)
       : PrerenderContents(prerender_manager, prerender_tracker, profile,
-                          url, referrer, ORIGIN_LINK_REL_PRERENDER),
+                          url, referrer, ORIGIN_LINK_REL_PRERENDER,
+                          PrerenderManager::kNoExperiment),
         number_of_loads_(0),
         expected_number_of_loads_(number_of_loads),
         expected_final_status_(expected_final_status),
@@ -252,7 +253,8 @@ class WaitForLoadPrerenderContentsFactory : public PrerenderContents::Factory {
       Profile* profile,
       const GURL& url,
       const GURL& referrer,
-      Origin origin) OVERRIDE {
+      Origin origin,
+      uint8 experiment_id) OVERRIDE {
     CHECK(!expected_final_status_queue_.empty()) <<
           "Creating prerender contents for " << url.path() <<
           " with no expected final status";

chrome/browser/prerender/prerender_contents.cc

@@ -63,9 +63,9 @@ class PrerenderContentsFactoryImpl : public PrerenderContents::Factory {
   virtual PrerenderContents* CreatePrerenderContents(
       PrerenderManager* prerender_manager, PrerenderTracker* prerender_tracker,
       Profile* profile, const GURL& url, const GURL& referrer,
-      Origin origin) OVERRIDE {
+      Origin origin, uint8 experiment_id) OVERRIDE {
     return new PrerenderContents(prerender_manager, prerender_tracker, profile,
-                                 url, referrer, origin);
+                                 url, referrer, origin, experiment_id);
   }
 };
 
@@ -109,7 +109,8 @@ PrerenderContents::PrerenderContents(PrerenderManager* prerender_manager,
                                      Profile* profile,
                                      const GURL& url,
                                      const GURL& referrer,
-                                     Origin origin)
+                                     Origin origin,
+                                     uint8 experiment_id)
     : prerender_manager_(prerender_manager),
       prerender_tracker_(prerender_tracker),
       prerender_url_(url),
@@ -123,7 +124,8 @@ PrerenderContents::PrerenderContents(PrerenderManager* prerender_manager,
       child_id_(-1),
       route_id_(-1),
       starting_page_id_(-1),
-      origin_(origin) {
+      origin_(origin),
+      experiment_id_(experiment_id) {
   DCHECK(prerender_manager != NULL);
 }
 
@@ -294,7 +296,8 @@ PrerenderContents::~PrerenderContents() {
   // If we haven't even started prerendering, we were just in the control
   // group, which means we do not want to record the status.
   if (prerendering_has_started())
-    RecordFinalStatus(origin_, final_status_);
+    prerender_manager_->RecordFinalStatus(origin_, experiment_id_,
+                                          final_status_);
 
   if (child_id_ != -1 && route_id_ != -1)
     prerender_tracker_->OnPrerenderingFinished(child_id_, route_id_);

chrome/browser/prerender/prerender_contents.h

@@ -61,7 +61,8 @@ class PrerenderContents : public NotificationObserver,
         Profile* profile,
         const GURL& url,
         const GURL& referrer,
-        Origin origin) = 0;
+        Origin origin,
+        uint8 experiment_id) = 0;
 
    private:
     DISALLOW_COPY_AND_ASSIGN(Factory);
@@ -171,7 +172,8 @@ class PrerenderContents : public NotificationObserver,
                     Profile* profile,
                     const GURL& url,
                     const GURL& referrer,
-                    Origin origin);
+                    Origin origin,
+                    uint8 experiment_id);
 
   NotificationRegistrar& notification_registrar() {
     return notification_registrar_;
@@ -277,6 +279,9 @@ class PrerenderContents : public NotificationObserver,
   // Origin for this prerender.
   Origin origin_;
 
+  // Experiment during which this prerender is performed.
+  uint8 experiment_id_;
+
   // Offset by which to offset prerendered pages
   static const int32 kPrerenderPageIdOffset = 10;
 

chrome/browser/prerender/prerender_final_status.cc

@@ -4,7 +4,6 @@
 
 #include "chrome/browser/prerender/prerender_final_status.h"
 
-#include "base/metrics/histogram.h"
 #include "chrome/browser/prerender/prerender_manager.h"
 
 namespace prerender {
@@ -51,34 +50,6 @@ COMPILE_ASSERT(arraysize(kFinalStatusNames) == FINAL_STATUS_MAX + 1,
 
 }
 
-void RecordFinalStatus(Origin origin, FinalStatus final_status) {
-  DCHECK(final_status != FINAL_STATUS_MAX);
-  // FINAL_STATUS_CONTROL_GROUP indicates that the PrerenderContents
-  // was created only to measure "would-have-been-prerendered" for
-  // control group measurements. Don't pollute data with it.
-  if (PrerenderManager::IsControlGroup() ||
-      final_status == FINAL_STATUS_CONTROL_GROUP)
-    return;
-  UMA_HISTOGRAM_ENUMERATION("Prerender.FinalStatus",
-                            final_status,
-                            FINAL_STATUS_MAX);
-  switch (origin) {
-    case ORIGIN_LINK_REL_PRERENDER:
-      UMA_HISTOGRAM_ENUMERATION("Prerender.FinalStatus_LinkRelPrerender",
-                                final_status,
-                                FINAL_STATUS_MAX);
-      break;
-    case ORIGIN_OMNIBOX:
-      UMA_HISTOGRAM_ENUMERATION("Prerender.FinalStatus_Omnibox",
-                                final_status,
-                                FINAL_STATUS_MAX);
-      break;
-    default:
-      NOTREACHED();
-      break;
-  };
-}
-
 const char* NameFromFinalStatus(FinalStatus final_status) {
   DCHECK(static_cast<int>(final_status) >= 0 &&
          static_cast<int>(final_status) <= FINAL_STATUS_MAX);

chrome/browser/prerender/prerender_final_status.h

@@ -47,8 +47,6 @@ enum FinalStatus {
   FINAL_STATUS_MAX,
 };
 
-void RecordFinalStatus(Origin origin, FinalStatus final_status);
-
 // Return a human-readable name for |final_status|. |final_status|
 // is expected to be a valid value.
 const char* NameFromFinalStatus(FinalStatus final_status);

chrome/browser/prerender/prerender_manager.h

@@ -68,6 +68,9 @@ class PrerenderManager : public base::SupportsWeakPtr<PrerenderManager>,
     CLEAR_MAX = 0x1 << 2
   };
 
+  // ID indicating that no experiment is active.
+  static const uint8 kNoExperiment = 0;
+
   // Owned by a Profile object for the lifetime of the profile.
   PrerenderManager(Profile* profile, PrerenderTracker* prerender_tracker);
 
@@ -181,6 +184,11 @@ class PrerenderManager : public base::SupportsWeakPtr<PrerenderManager>,
   // Intended to be used when clearing the cache or history.
   void ClearData(int clear_flags);
 
+  // Record a final status of a prerendered page in a histogram.
+  void RecordFinalStatus(Origin origin,
+                         uint8 experiment_id,
+                         FinalStatus final_status) const;
+
   const Config& config() const { return config_; }
   Config& mutable_config() { return config_; }
 
@@ -196,12 +204,6 @@ class PrerenderManager : public base::SupportsWeakPtr<PrerenderManager>,
 
   PendingContentsData* FindPendingEntry(const GURL& url);
 
-  // Extracts a urlencoded URL stored in a url= query parameter from a URL
-  // supplied, if available, and stores it in alias_url.  Returns whether or not
-  // the operation succeeded (i.e. a valid URL was found).
-  static bool MaybeGetQueryStringBasedAliasURL(const GURL& url,
-                                               GURL* alias_url);
-
  private:
   // Test that needs needs access to internal functions.
   friend class PrerenderBrowserTest;
@@ -252,7 +254,8 @@ class PrerenderManager : public base::SupportsWeakPtr<PrerenderManager>,
   virtual base::TimeTicks GetCurrentTimeTicks() const;
   virtual PrerenderContents* CreatePrerenderContents(const GURL& url,
                                                      const GURL& referrer,
-                                                     Origin origin);
+                                                     Origin origin,
+                                                     uint8 experiment_id);
 
   // Checks if the PrerenderContents has been added to the pending delete list.
   bool IsPendingDelete(PrerenderContents* entry) const;
@@ -316,6 +319,22 @@ class PrerenderManager : public base::SupportsWeakPtr<PrerenderManager>,
   // navigates to it. This must be called on the UI thread.
   void RecordTimeUntilUsed(base::TimeDelta time_until_used);
 
+  // Composes a histogram name based on a histogram type.
+  std::string ComposeHistogramName(const std::string& prefix_type,
+                                   const std::string& name) const;
+
+  // Returns the histogram name for a given origin and experiment.
+  std::string GetHistogramName(Origin origin, uint8 experiment_id,
+                               const std::string& name) const;
+  // Returns the histogram name for the current window.
+  std::string GetDefaultHistogramName(const std::string& name) const;
+  // Returns the current experiment.
+  uint8 GetCurrentExperimentId() const;
+  // Returns the current origin.
+  Origin GetCurrentOrigin() const;
+  // Returns whether or not there is currently an origin/experiment wash.
+  bool IsOriginExperimentWash() const;
+
   // The configuration.
   Config config_;
 
@@ -355,6 +374,18 @@ class PrerenderManager : public base::SupportsWeakPtr<PrerenderManager>,
 
   static PrerenderManagerMode mode_;
 
+  // An integer indicating a Prerendering Experiment being currently conducted.
+  // (The last experiment ID seen).
+  uint8 last_experiment_id_;
+
+  // Origin of the last prerender seen.
+  Origin last_origin_;
+
+  // A boolean indicating that we have recently encountered a combination of
+  // different experiments and origins, making an attribution of PPLT's to
+  // experiments / origins impossible.
+  bool origin_experiment_wash_;
+
   // The time when we last saw a prerender request coming from a renderer.
   // This is used to record perceived PLT's for a certain amount of time
   // from the point that we last saw a <link rel=prerender> tag.

chrome/browser/prerender/prerender_manager_unittest.cc

@@ -25,7 +25,8 @@ class DummyPrerenderContents : public PrerenderContents {
                          const GURL& url,
                          FinalStatus expected_final_status)
       : PrerenderContents(prerender_manager, prerender_tracker, NULL, url,
-                          GURL(), ORIGIN_LINK_REL_PRERENDER),
+                          GURL(), ORIGIN_LINK_REL_PRERENDER,
+                          PrerenderManager::kNoExperiment),
         has_started_(false),
         expected_final_status_(expected_final_status) {
   }
@@ -157,7 +158,8 @@ class TestPrerenderManager : public PrerenderManager {
   virtual PrerenderContents* CreatePrerenderContents(
       const GURL& url,
       const GURL& referrer,
-      Origin origin) OVERRIDE {
+      Origin origin,
+      uint8 experiment_id) OVERRIDE {
     DCHECK(next_prerender_contents_.get());
     return next_prerender_contents_.release();
   }
@@ -436,24 +438,6 @@ TEST_F(PrerenderManagerTest, PendingPrerenderTest) {
   ASSERT_EQ(prerender_contents, prerender_manager()->GetEntry(url));
 }
 
-// Ensure that extracting a urlencoded URL in the url= query string component
-// works.
-TEST_F(PrerenderManagerTest, ExtractURLInQueryStringTest) {
-  GURL result;
-  EXPECT_TRUE(PrerenderManager::MaybeGetQueryStringBasedAliasURL(
-      GURL("http://www.google.com/url?sa=t&source=web&cd=1&ved=0CBcQFjAA&url=http%3A%2F%2Fwww.abercrombie.com%2Fwebapp%2Fwcs%2Fstores%2Fservlet%2FStoreLocator%3FcatalogId%3D%26storeId%3D10051%26langId%3D-1&rct=j&q=allinurl%3A%26&ei=KLyUTYGSEdTWiAKUmLCdCQ&usg=AFQjCNF8nJ2MpBFfr1ijO39_f22bcKyccw&sig2=2ymyGpO0unJwU1d4kdCUjQ"),
-      &result));
-  ASSERT_EQ(GURL("http://www.abercrombie.com/webapp/wcs/stores/servlet/StoreLocator?catalogId=&storeId=10051&langId=-1").spec(), result.spec());
-  EXPECT_FALSE(PrerenderManager::MaybeGetQueryStringBasedAliasURL(
-      GURL("http://www.google.com/url?sadf=test&blah=blahblahblah"), &result));
-  EXPECT_FALSE(PrerenderManager::MaybeGetQueryStringBasedAliasURL(
-      GURL("http://www.google.com/?url=INVALIDurlsAREsoMUCHfun.com"), &result));
-  EXPECT_TRUE(PrerenderManager::MaybeGetQueryStringBasedAliasURL(
-      GURL("http://www.google.com/?url=http://validURLSareGREAT.com"),
-      &result));
-  ASSERT_EQ(GURL("http://validURLSareGREAT.com").spec(), result.spec());
-}
-
 // Tests that a PrerenderManager created for a browser session in the control
 // group will not be able to override FINAL_STATUS_CONTROL_GROUP.
 TEST_F(PrerenderManagerTest, ControlGroup) {

chrome/browser/prerender/prerender_util.cc

+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/prerender/prerender_util.h"
+
+#include "base/logging.h"
+#include "googleurl/src/url_canon.h"
+#include "googleurl/src/url_parse.h"
+#include "googleurl/src/url_util.h"
+
+namespace prerender {
+
+bool MaybeGetQueryStringBasedAliasURL(
+    const GURL& url, GURL* alias_url) {
+  DCHECK(alias_url);
+  url_parse::Parsed parsed;
+  url_parse::ParseStandardURL(url.spec().c_str(), url.spec().length(),
+                              &parsed);
+  url_parse::Component query = parsed.query;
+  url_parse::Component key, value;
+  while (url_parse::ExtractQueryKeyValue(url.spec().c_str(), &query, &key,
+                                         &value)) {
+    if (key.len != 3 || strncmp(url.spec().c_str() + key.begin, "url", key.len))
+      continue;
+    // We found a url= query string component.
+    if (value.len < 1)
+      continue;
+    url_canon::RawCanonOutputW<1024> decoded_url;
+    url_util::DecodeURLEscapeSequences(url.spec().c_str() + value.begin,
+                                       value.len, &decoded_url);
+    GURL new_url(string16(decoded_url.data(), decoded_url.length()));
+    if (!new_url.is_empty() && new_url.is_valid()) {
+      *alias_url = new_url;
+      return true;
+    }
+    return false;
+  }
+  return false;
+}
+
+uint8 GetQueryStringBasedExperiment(const GURL& url) {
+  url_parse::Parsed parsed;
+  url_parse::ParseStandardURL(url.spec().c_str(), url.spec().length(),
+                              &parsed);
+  url_parse::Component query = parsed.query;
+  url_parse::Component key, value;
+  while (url_parse::ExtractQueryKeyValue(url.spec().c_str(), &query, &key,
+                                         &value)) {
+    if (key.len != 3 || strncmp(url.spec().c_str() + key.begin, "lpe", key.len))
+      continue;
+
+    // We found a lpe= query string component.
+    if (value.len != 1)
+      continue;
+    uint8 exp = *(url.spec().c_str() + value.begin) - '0';
+    if (exp < 1 || exp > 9)
+      continue;
+    return exp;
+  }
+  return kNoExperiment;
+}
+
+}  // namespace prerender

chrome/browser/prerender/prerender_util.h

+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_PRERENDER_PRERENDER_UTIL_H_
+#define CHROME_BROWSER_PRERENDER_PRERENDER_UTIL_H_
+
+#include "base/basictypes.h"
+#include "googleurl/src/gurl.h"
+
+namespace prerender {
+
+// ID indicating that no experiment is active.
+const uint8 kNoExperiment = 0;
+
+// Extracts a urlencoded URL stored in a url= query parameter from a URL
+// supplied, if available, and stores it in alias_url.  Returns whether or not
+// the operation succeeded (i.e. a valid URL was found).
+bool MaybeGetQueryStringBasedAliasURL(const GURL& url, GURL* alias_url);
+
+// Extracts an experiment stored in the query parameter
+// lpe= from the URL supplied, and returns it.
+// Returns kNoExperiment if no experiment ID is found, or if the ID
+// is not an integer in the range 1 to 9.
+uint8 GetQueryStringBasedExperiment(const GURL& url);
+
+}  // namespace prerender
+
+#endif  // CHROME_BROWSER_PRERENDER_PRERENDER_UTIL_H_

chrome/browser/prerender/prerender_util_unittest.cc

+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/prerender/prerender_util.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace prerender {
+
+class PrerenderUtilTest : public testing::Test {
+ public:
+  PrerenderUtilTest() {
+  }
+};
+
+// Ensure that extracting a urlencoded URL in the url= query string component
+// works.
+TEST_F(PrerenderUtilTest, ExtractURLInQueryStringTest) {
+  GURL result;
+  EXPECT_TRUE(MaybeGetQueryStringBasedAliasURL(
+      GURL("http://www.google.com/url?sa=t&source=web&cd=1&ved=0CBcQFjAA&url=http%3A%2F%2Fwww.abercrombie.com%2Fwebapp%2Fwcs%2Fstores%2Fservlet%2FStoreLocator%3FcatalogId%3D%26storeId%3D10051%26langId%3D-1&rct=j&q=allinurl%3A%26&ei=KLyUTYGSEdTWiAKUmLCdCQ&usg=AFQjCNF8nJ2MpBFfr1ijO39_f22bcKyccw&sig2=2ymyGpO0unJwU1d4kdCUjQ"),
+      &result));
+  ASSERT_EQ(GURL("http://www.abercrombie.com/webapp/wcs/stores/servlet/StoreLocator?catalogId=&storeId=10051&langId=-1").spec(), result.spec());
+  EXPECT_FALSE(MaybeGetQueryStringBasedAliasURL(
+      GURL("http://www.google.com/url?sadf=test&blah=blahblahblah"), &result));
+  EXPECT_FALSE(MaybeGetQueryStringBasedAliasURL(
+      GURL("http://www.google.com/?url=INVALIDurlsAREsoMUCHfun.com"), &result));
+  EXPECT_TRUE(MaybeGetQueryStringBasedAliasURL(
+      GURL("http://www.google.com/?url=http://validURLSareGREAT.com"),
+      &result));
+  ASSERT_EQ(GURL("http://validURLSareGREAT.com").spec(), result.spec());
+}
+
+// Ensure that extracting an experiment in the lpe= query string component
+// works.
+TEST_F(PrerenderUtilTest, ExtractExperimentInQueryStringTest) {
+  GURL result;
+  EXPECT_EQ(GetQueryStringBasedExperiment(
+      GURL("http://www.google.com/url?sa=t&source=web&cd=1&ved=0CBcQFjAA&url=http%3A%2F%2Fwww.abercrombie.com%2Fwebapp%2Fwcs%2Fstores%2Fservlet%2FStoreLocator%3FcatalogId%3D%26storeId%3D10051%26langId%3D-1&rct=j&q=allinurl%3A%26&ei=KLyUTYGSEdTWiAKUmLCdCQ&usg=AFQjCNF8nJ2MpBFfr1ijO39_f22bcKyccw&sig2=2ymyGpO0unJwU1d4kdCUjQ&lpe=4&asdf=test")), 4);
+  EXPECT_EQ(GetQueryStringBasedExperiment(
+      GURL("http://www.google.com/test.php?a=b")), kNoExperiment);
+  EXPECT_EQ(GetQueryStringBasedExperiment(
+      GURL("http://www.google.com/test.php?lpe=5")), 5);
+  EXPECT_EQ(GetQueryStringBasedExperiment(
+      GURL("http://www.google.com/test.php?lpe=50")), kNoExperiment);
+  EXPECT_EQ(GetQueryStringBasedExperiment(
+      GURL("http://www.google.com/test.php?lpe=0")), kNoExperiment);
+  EXPECT_EQ(GetQueryStringBasedExperiment(
+      GURL("http://www.google.com/test.php?lpe=10")), kNoExperiment);
+}
+
+}  // namespace prerender

chrome/chrome_browser.gypi

@@ -1715,6 +1715,8 @@
         'browser/prerender/prerender_render_view_host_observer.h',
         'browser/prerender/prerender_tracker.cc',
         'browser/prerender/prerender_tracker.h',
+        'browser/prerender/prerender_util.cc',
+        'browser/prerender/prerender_util.h',
         'browser/printing/background_printing_manager.cc',
         'browser/printing/background_printing_manager.h',
         'browser/printing/cloud_print/cloud_print_proxy_service.cc',

chrome/chrome_tests.gypi

@@ -1544,6 +1544,7 @@
         'browser/prerender/prerender_history_unittest.cc',
         'browser/prerender/prerender_manager_unittest.cc',
         'browser/prerender/prerender_tracker_unittest.cc',
+        'browser/prerender/prerender_util_unittest.cc',
         'browser/printing/cloud_print/cloud_print_setup_source_unittest.cc',
         'browser/printing/print_dialog_cloud_unittest.cc',
         'browser/printing/print_job_unittest.cc',