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

chrome/browser/prerender/prerender_manager.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_MANAGER_H_
 #define CHROME_BROWSER_PRERENDER_PRERENDER_MANAGER_H_
 #pragma once
 
 #include <list>
 #include <map>
@@ skipping 49 matching lines @@
   };
 
   // One or more of these flags must be passed to ClearData() to specify just
   // what data to clear.  See function declaration for more information.
   enum ClearFlags {
     CLEAR_PRERENDER_CONTENTS = 0x1 << 0,
     CLEAR_PRERENDER_HISTORY = 0x1 << 1,
     CLEAR_MAX = 0x1 << 2
   };
 
+  // ID indicating that no experiment is active.
+  static const char kNoExperiment = -1;

[dominich, 2011/06/30 18:11:15]

A uint8 set to static_cast<uint8>(-1) if you agree to use uint8 for the type. I
wonder if an enum is even better in this case.

[tburkard, 2011/06/30 19:45:48]

Done.

+
   // Owned by a Profile object for the lifetime of the profile.
   PrerenderManager(Profile* profile, PrerenderTracker* prerender_tracker);
 
   virtual ~PrerenderManager();
 
   // Entry points for adding prerenders.
 
   // Adds a prerender for |url| if valid. |process_id| and |route_id| identify
   // the RenderViewHost that the prerender request came from and are used to
   // set the initial window size of the RenderViewHost used for prerendering.
@@ skipping 98 matching lines @@
   // cancelled and then deleted, and any TabContents queued for destruction are
   // destroyed as well.
   //
   // If the CLEAR_PRERENDER_HISTORY bit is set, the prerender history is
   // cleared, including any entries newly created by destroying them in
   // response to the CLEAR_PRERENDER_CONTENTS flag.
   //
   // 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,
+                         char experiment_id,
+                         FinalStatus final_status) const;
+
  protected:
   // Test that needs needs access to internal functions.
   FRIEND_TEST_ALL_PREFIXES(PrerenderManagerTest, ExpireTest);
   FRIEND_TEST_ALL_PREFIXES(PrerenderManagerTest, ExtractURLInQueryStringTest);
 
   struct PendingContentsData;
 
   void SetPrerenderContentsFactory(
       PrerenderContents::Factory* prerender_contents_factory);
 
   bool rate_limit_enabled_;
 
   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);
 
+  // Extracts an experiment stored in the query parameter

[cbentzel, 2011/06/30 18:15:39]

Can we just combine this with MaybeGetQueryStringBasedAliasURL?

I'd also consider moving it to a separate file and just make it a top-level
function, rather than a static on PrernederManager. 

[tburkard, 2011/06/30 19:45:48]

Since it's a local check that only prerendermanager needs to perform, I think it
makes sense to make it part of that class.  Would you prefer it being a full
member as opposed to static?

On 2011/06/30 18:15:39, cbentzel wrote:

[cbentzel, 2011/06/30 19:51:28]

The reason to move it out is the prerender_manager.[h|cc] are pretty big. Easier
to read and test if top-level function that this includes. I also noticed that
the new method is not tested. 

+  // 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 0 to 9.
+  static char GetQueryStringBasedExperiment(const GURL& url);
+
   base::TimeDelta max_prerender_age() const;
   unsigned int max_elements() const;
 
  private:
   // Test that needs needs access to internal functions.
   friend class PrerenderBrowserTest;
 
   friend class base::RefCountedThreadSafe<PrerenderManager>;
 
   struct PrerenderContentsData;
@@ skipping 33 matching lines @@
   // objects.  If |this| is deleted before the task is run, the task will
   // automatically be cancelled.
   void PostCleanupTask();
 
   bool IsPrerenderElementFresh(const base::Time start) const;
   void DeleteOldEntries();
   virtual base::Time GetCurrentTime() const;
   virtual base::TimeTicks GetCurrentTimeTicks() const;
   virtual PrerenderContents* CreatePrerenderContents(const GURL& url,
                                                      const GURL& referrer,
-                                                     Origin origin);
+                                                     Origin origin,
+                                                     char experiment_id);
 
   // Checks if the PrerenderContents has been added to the pending delete list.
   bool IsPendingDelete(PrerenderContents* entry) const;
 
   // Deletes any PrerenderContents that have been added to the pending delete
   // list.
   void DeletePendingDeleteEntries();
 
   // Finds the specified PrerenderContents and returns it, if it exists.
   // Returns NULL otherwise.  Unlike GetEntry, the PrerenderManager maintains
@@ skipping 43 matching lines @@
 
   // Destroys all pending prerenders using FinalStatus.  Also deletes them as
   // well as any swapped out TabContents queued for destruction.
   // Used both on destruction, and when clearing the browing history.
   void DestroyAllContents(FinalStatus final_status);
 
   // Records the time from when a page starts prerendering to when the user
   // 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.

[dominich, 2011/06/30 18:11:15]

It doesn't look like these really need to be in the PrerenderManager. Can we
have a PrerenderHistogram or PrerenderExperimentTracker class that manages this
stuff?

[tburkard, 2011/06/30 19:45:48]

Can I do the cleanup in a separate CL?  This CL is already complex, I'd rather
get in the functionality first, and then do a more straightforward cleanup.

On 2011/06/30 18:11:15, dominich wrote:

+  std::string ComposeHistogramName(std::string prefix_type,
+                                   std::string name) const;
+
+  // Returns the histogram name for a given origin and experiment.
+  std::string GetHistogramName(Origin origin, char experiment_id,
+                               std::string name) const;
+  // Returns the histogram name for the current window.
+  std::string GetDefaultHistogramName(std::string name) const;
+  // Returns the current experiment.
+  char GetCurrentExperimentId() const;
+  // Returns the current origin.
+  Origin GetCurrentOrigin() const;
+  // Returns whether or not there is currently an origin/experiment wash.
+  bool IsOriginExperimentWash() const;
+
   // Specifies whether prerendering is currently enabled for this
   // manager. The value can change dynamically during the lifetime
   // of the PrerenderManager.
   bool enabled_;
 
   Profile* profile_;
 
   PrerenderTracker* prerender_tracker_;
 
   base::TimeDelta max_prerender_age_;
@@ skipping 21 matching lines @@
 
   // Map of child/route id pairs to pending prerender data.
   typedef std::map<std::pair<int, int>, std::vector<PendingContentsData> >
       PendingPrerenderList;
   PendingPrerenderList pending_prerender_list_;
 
   scoped_ptr<PrerenderContents::Factory> prerender_contents_factory_;
 
   static PrerenderManagerMode mode_;
 
+  // An integer indicating a Prerendering Experiment being currently conducted.
+  // (The last experiment ID seen).
+  char 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.
   base::TimeTicks last_prerender_seen_time_;
 
   // A count of how many prerenders we do per session. Initialized to 0 then
   // incremented and emitted to a histogram on each successful prerender.
   static int prerenders_per_session_count_;
 
   // RepeatingTimer to perform periodic cleanups of pending prerendered
@@ skipping 11 matching lines @@
   ScopedVector<OnCloseTabContentsDeleter> on_close_tab_contents_deleters_;
 
   scoped_ptr<PrerenderHistory> prerender_history_;
 
   DISALLOW_COPY_AND_ASSIGN(PrerenderManager);
 };
 
 }  // namespace prerender
 
 #endif  // CHROME_BROWSER_PRERENDER_PRERENDER_MANAGER_H_