[Most Visited] Removing Most Visited Tiles experiment code, which is obsolete.

Experiment is no longer running and is in many ways obsolete. Removing the code.

BUG=None
TEST=manual

Review URL: https://codereview.chromium.org/437803003

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

chrome/browser/history/most_visited_tiles_experiment.cc

-// Copyright 2013 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/history/most_visited_tiles_experiment.h"
-
-#include "base/metrics/field_trial.h"
-#include "base/metrics/histogram.h"
-#include "base/strings/string_util.h"
-#include "chrome/common/instant_types.h"
-
-namespace history {
-
-namespace {
-
-// Constants for the most visited tile placement field trial.
-// ex:
-// "OneEightGroup_Flipped" --> Will cause tile 1 and 8 to be flipped.
-// "OneEightGroup_NoChange" --> Will not flip anything.
-//
-// See field trial config (MostVisitedTilePlacement.json) for details.
-const char kMostVisitedFieldTrialName[] = "MostVisitedTilePlacement";
-// Name of histogram tracking types of actions carried out by the field trial.
-const char kMostVisitedExperimentHistogramName[] =
-    "NewTabPage.MostVisitedTilePlacementExperiment";
-const char kOneEightGroupPrefix[] = "OneEight";
-const char kOneFourGroupPrefix[] = "OneFour";
-const char kFlippedSuffix[] = "Flipped";
-const char kDontShowOpenURLsGroupName[] = "DontShowOpenTabs";
-// Minimum number of Most Visited suggestions required in order for the Most
-// Visited Field Trial to remove a URL already open in the browser.
-const size_t kMinUrlSuggestions = 8;
-
-}  // namespace
-
-// static
-void MostVisitedTilesExperiment::MaybeShuffle(MostVisitedURLList* data) {
-  const std::string group_name =
-      base::FieldTrialList::FindFullName(kMostVisitedFieldTrialName);
-
-  // Depending on the study group of the client, we might flip the 1st and 4th
-  // tiles, or the 1st and 8th, or do nothing.
-  if (!EndsWith(group_name, kFlippedSuffix, true))
-    return;
-
-  size_t index_to_flip = 0;
-  if (StartsWithASCII(group_name, kOneEightGroupPrefix, true)) {
-    if (data->size() < 8) {
-      LogInHistogram(NTP_TILE_EXPERIMENT_ACTION_TOO_FEW_URLS_TILES_1_8);
-      return;
-    }
-    index_to_flip = 7;
-  } else if (StartsWithASCII(group_name, kOneFourGroupPrefix, true)) {
-    if (data->size() < 4) {
-      LogInHistogram(NTP_TILE_EXPERIMENT_ACTION_TOO_FEW_URLS_TILES_1_4);
-      return;
-    }
-    index_to_flip = 3;
-  }
-  std::swap((*data)[0], (*data)[index_to_flip]);
-}
-
-// static
-bool MostVisitedTilesExperiment::IsDontShowOpenURLsEnabled() {
-  return base::FieldTrialList::FindFullName(kMostVisitedFieldTrialName) ==
-      kDontShowOpenURLsGroupName;
-}
-
-// static
-void MostVisitedTilesExperiment::RemoveItemsMatchingOpenTabs(
-    const std::set<std::string>& open_urls,
-    std::vector<InstantMostVisitedItem>* items) {
-  for (size_t i = 0; i < items->size(); ) {
-    const std::string& url = (*items)[i].url.spec();
-    if (ShouldRemoveURL(open_urls, url, items->size()))
-      items->erase(items->begin() + i);
-    else
-      ++i;
-  }
-}
-
-// static
-void MostVisitedTilesExperiment::RemovePageValuesMatchingOpenTabs(
-    const std::set<std::string>& open_urls,
-    base::ListValue* pages_value) {
-  for (size_t i = 0; i < pages_value->GetSize(); ) {
-    base::DictionaryValue* page_value;
-    std::string url;
-    if (pages_value->GetDictionary(i, &page_value) &&
-        page_value->GetString("url", &url) &&
-        ShouldRemoveURL(open_urls, url, pages_value->GetSize())) {
-      pages_value->Remove(*page_value, &i);
-    } else {
-      ++i;
-    }
-  }
-}
-
-// static
-void MostVisitedTilesExperiment::LogInHistogram(
-    NtpTileExperimentActions action) {
-  UMA_HISTOGRAM_ENUMERATION(kMostVisitedExperimentHistogramName,
-                            action,
-                            NUM_NTP_TILE_EXPERIMENT_ACTIONS);
-}
-
-// static
-bool MostVisitedTilesExperiment::ShouldRemoveURL(
-    const std::set<std::string>& open_urls,
-    const std::string& url,
-    const size_t size) {
-  if (open_urls.count(url) == 0)
-    return false;
-
-  if (size <= kMinUrlSuggestions) {
-    LogInHistogram(NTP_TILE_EXPERIMENT_ACTION_DID_NOT_REMOVE_URL);
-    return false;
-  }
-
-  LogInHistogram(NTP_TILE_EXPERIMENT_ACTION_REMOVED_URL);
-  return true;
-}
-
-}  // namespace history

chrome/browser/history/most_visited_tiles_experiment.h

-// Copyright 2013 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_HISTORY_MOST_VISITED_TILES_EXPERIMENT_H_
-#define CHROME_BROWSER_HISTORY_MOST_VISITED_TILES_EXPERIMENT_H_
-
-#include "base/values.h"
-#include "chrome/browser/history/history_types.h"
-#include "chrome/common/instant_types.h"
-
-namespace history {
-
-// This enum is also defined in histograms.xml. These values represent the
-// types of actions carried out by the Most Visited Tile Placement experiment.
-enum NtpTileExperimentActions {
-  NTP_TILE_EXPERIMENT_ACTION_REMOVED_URL = 0,
-  NTP_TILE_EXPERIMENT_ACTION_DID_NOT_REMOVE_URL = 1,
-  NTP_TILE_EXPERIMENT_ACTION_TOO_FEW_URLS_TILES_1_8 = 2,
-  NTP_TILE_EXPERIMENT_ACTION_TOO_FEW_URLS_TILES_1_4 = 3,
-  // The number of Most Visited Tile Placement experiment actions logged.
-  NUM_NTP_TILE_EXPERIMENT_ACTIONS
-};
-
-// Class for implementing the Most Visited Tile Placement experiment.
-class MostVisitedTilesExperiment {
- public:
-  // Helper method to shuffle MostVisited tiles for A/B testing purposes.
-  static void MaybeShuffle(MostVisitedURLList* data);
-
-  // Returns true if this user is part of the Most Visited Tile Placement
-  // experiment group where URLs currently open in another browser tab are not
-  // displayed on an NTP tile. Note: the experiment targets only the top-level
-  // of sites i.e. if www.foo.com/bar is open in browser, and www.foo.com is a
-  // recommended URL, www.foo.com will still appear on the next NTP open. The
-  // experiment will not remove a URL if doing so would cause the number of Most
-  // Visited recommendations to drop below eight.
-  static bool IsDontShowOpenURLsEnabled();
-
-  // Removes URLs already open in browser, for 1993 clients, if part of
-  // experiment described for IsDontShowOpenURLsEnabled().
-  static void RemoveItemsMatchingOpenTabs(
-      const std::set<std::string>& open_urls,
-      std::vector<InstantMostVisitedItem>* items);
-
-  // Removes URLs already open in browser, for non-1993 clients, if part of
-  // experiment described for IsDontShowOpenURLsEnabled().
-  static void RemovePageValuesMatchingOpenTabs(
-      const std::set<std::string>& open_urls,
-      base::ListValue* pages_value);
-
- private:
-  // Helper method to log the actions carried out by the Most Visited Tile
-  // Placement experiment.
-  static void LogInHistogram(NtpTileExperimentActions action);
-
-  // Helper method to determine whether |url| is in |open_urls|.
-  static bool ShouldRemoveURL(const std::set<std::string>& open_urls,
-                       const std::string& url,
-                       const size_t size);
-
-  DISALLOW_IMPLICIT_CONSTRUCTORS(MostVisitedTilesExperiment);
-};
-
-}  // namespace history
-
-#endif  // CHROME_BROWSER_HISTORY_MOST_VISITED_TILES_EXPERIMENT_H_

chrome/browser/search/instant_service.cc

@@ -5,7 +5,6 @@
 #include "chrome/browser/search/instant_service.h"
 
 #include "chrome/browser/chrome_notification_types.h"
-#include "chrome/browser/history/most_visited_tiles_experiment.h"
 #include "chrome/browser/history/top_sites.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search/instant_io_context.h"
@@ -265,8 +264,6 @@ void InstantService::OnRendererProcessTerminated(int process_id) {
 void InstantService::OnMostVisitedItemsReceived(
     const history::MostVisitedURLList& data) {
   history::MostVisitedURLList reordered_data(data);
-  history::MostVisitedTilesExperiment::MaybeShuffle(&reordered_data);
-
   std::vector<InstantMostVisitedItem> new_most_visited_items;
   for (size_t i = 0; i < reordered_data.size(); i++) {
     const history::MostVisitedURL& url = reordered_data[i];

chrome/browser/ui/search/search_tab_helper.cc

@@ -11,7 +11,6 @@
 #include "base/strings/string16.h"
 #include "base/strings/string_util.h"
 #include "chrome/browser/chrome_notification_types.h"
-#include "chrome/browser/history/most_visited_tiles_experiment.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search/instant_service.h"
 #include "chrome/browser/search/instant_service_factory.h"
@@ -419,27 +418,13 @@ void SearchTabHelper::ThemeInfoChanged(const ThemeBackgroundInfo& theme_info) {
 
 void SearchTabHelper::MostVisitedItemsChanged(
     const std::vector<InstantMostVisitedItem>& items) {
-  std::vector<InstantMostVisitedItem> items_copy(items);
-  MaybeRemoveMostVisitedItems(&items_copy);
-  ipc_router_.SendMostVisitedItems(items_copy);
+  ipc_router_.SendMostVisitedItems(items);
 }
 
 void SearchTabHelper::OmniboxStartMarginChanged(int omnibox_start_margin) {
   ipc_router_.SetOmniboxStartMargin(omnibox_start_margin);
 }
 
-void SearchTabHelper::MaybeRemoveMostVisitedItems(
-    std::vector<InstantMostVisitedItem>* items) {
-  if (!delegate_)
-    return;
-
-  if (!history::MostVisitedTilesExperiment::IsDontShowOpenURLsEnabled())
-    return;
-
-  history::MostVisitedTilesExperiment::RemoveItemsMatchingOpenTabs(
-      delegate_->GetOpenUrls(), items);
-}
-
 void SearchTabHelper::FocusOmnibox(OmniboxFocusState state) {
 // TODO(kmadhusu): Move platform specific code from here and get rid of #ifdef.
 #if !defined(OS_ANDROID)

chrome/browser/ui/search/search_tab_helper.h

@@ -183,11 +183,6 @@ class SearchTabHelper : public content::WebContentsObserver,
       const std::vector<InstantMostVisitedItem>& items) OVERRIDE;
   virtual void OmniboxStartMarginChanged(int omnibox_start_margin) OVERRIDE;
 
-  // Removes recommended URLs if a matching URL is already open in the Browser,
-  // if the Most Visited Tile Placement experiment is enabled, and the client is
-  // in the experiment group.
-  void MaybeRemoveMostVisitedItems(std::vector<InstantMostVisitedItem>* items);
-
   // Sets the mode of the model based on the current URL of web_contents().
   // Only updates the origin part of the mode if |update_origin| is true,
   // otherwise keeps the current origin. If |is_preloaded_ntp| is true, the mode

chrome/browser/ui/webui/ntp/most_visited_handler.cc

@@ -21,8 +21,6 @@
 #include "base/threading/thread.h"
 #include "base/values.h"
 #include "chrome/browser/chrome_notification_types.h"
-#include "chrome/browser/history/most_visited_tiles_experiment.h"
-#include "chrome/browser/history/page_usage_data.h"
 #include "chrome/browser/history/top_sites.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/thumbnails/thumbnail_list_source.h"
@@ -144,12 +142,9 @@ void MostVisitedHandler::SendPagesValue() {
         profile->GetPrefs()->GetDictionary(prefs::kNtpMostVisitedURLsBlacklist);
     bool has_blacklisted_urls = !url_blacklist->empty();
     history::TopSites* ts = profile->GetTopSites();
-    if (ts) {
+    if (ts)
       has_blacklisted_urls = ts->HasBlacklistedItems();
 
-      MaybeRemovePageValues();
-    }
-
     base::FundamentalValue has_blacklisted_urls_value(has_blacklisted_urls);
     web_ui()->CallJavascriptFunction("ntp.setMostVisitedPages",
                                      *pages_value_,
@@ -223,8 +218,6 @@ void MostVisitedHandler::SetPagesValueFromTopSites(
   pages_value_.reset(new base::ListValue);
 
   history::MostVisitedURLList top_sites(data);
-  history::MostVisitedTilesExperiment::MaybeShuffle(&top_sites);
-
   for (size_t i = 0; i < top_sites.size(); i++) {
     const history::MostVisitedURL& url = top_sites[i];
     base::DictionaryValue* page_value = new base::DictionaryValue();
@@ -269,25 +262,6 @@ std::string MostVisitedHandler::GetDictionaryKeyForUrl(const std::string& url) {
   return base::MD5String(url);
 }
 
-void MostVisitedHandler::MaybeRemovePageValues() {
-  if (!history::MostVisitedTilesExperiment::IsDontShowOpenURLsEnabled())
-    return;
-
-  TabStripModel* tab_strip_model = chrome::FindBrowserWithWebContents(
-      web_ui()->GetWebContents())->tab_strip_model();
-  history::TopSites* top_sites = Profile::FromWebUI(web_ui())->GetTopSites();
-  if (!tab_strip_model || !top_sites) {
-    NOTREACHED();
-    return;
-  }
-
-  std::set<std::string> open_urls;
-  chrome::GetOpenUrls(*tab_strip_model, *top_sites, &open_urls);
-  history::MostVisitedTilesExperiment::RemovePageValuesMatchingOpenTabs(
-      open_urls,
-      pages_value_.get());
-}
-
 // static
 void MostVisitedHandler::RegisterProfilePrefs(
     user_prefs::PrefRegistrySyncable* registry) {

chrome/browser/ui/webui/ntp/most_visited_handler.h

@@ -89,11 +89,6 @@ class MostVisitedHandler : public content::WebUIMessageHandler,
   // Returns the key used in url_blacklist_ for the passed |url|.
   std::string GetDictionaryKeyForUrl(const std::string& url);
 
-  // Removes recommended URLs if a matching URL is already open in the Browser,
-  // if the Most Visited Tile Placement experiment is enabled, and the client is
-  // in the experiment group.
-  void MaybeRemovePageValues();
-
   // Sends pages_value_ to the javascript side and resets page_value_.
   void SendPagesValue();
 

chrome/chrome_browser.gypi

@@ -559,8 +559,6 @@
       'browser/history/in_memory_url_index.h',
       'browser/history/in_memory_url_index_types.cc',
       'browser/history/in_memory_url_index_types.h',
-      'browser/history/most_visited_tiles_experiment.cc',
-      'browser/history/most_visited_tiles_experiment.h',
       'browser/history/page_usage_data.cc',
       'browser/history/page_usage_data.h',
       'browser/history/scored_history_match.cc',

chrome/chrome_tests_unit.gypi

@@ -1010,7 +1010,6 @@
         'browser/history/history_unittest_base.h',
         'browser/history/in_memory_url_index_types_unittest.cc',
         'browser/history/in_memory_url_index_unittest.cc',
-        'browser/history/most_visited_tiles_experiment_unittest.cc',
         'browser/history/scored_history_match_unittest.cc',
         'browser/history/select_favicon_frames_unittest.cc',
         'browser/history/shortcuts_database_unittest.cc',