Omnibox: Create DemoteByType Experiment

chrome/browser/omnibox/omnibox_field_trial_unittest.cc

 // Copyright (c) 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/omnibox/omnibox_field_trial.h"
 
 #include "base/basictypes.h"
 #include "base/metrics/field_trial.h"
 #include "base/strings/string16.h"
 #include "chrome/common/metrics/entropy_provider.h"
 #include "chrome/common/metrics/variations/variations_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 class OmniboxFieldTrialTest : public testing::Test {
  public:
-  OmniboxFieldTrialTest() {}
-
-  static void SetUpTestCase() {
+  OmniboxFieldTrialTest() {
     ResetFieldTrialList();
   }
 
   static void TearDownTestCase() {
     delete field_trial_list_;

[Alexei Svitkine (slow), 2013/08/05 18:33:11]

If you make field_trial_list_ a non-static scoped_ptr, you can delete this
method and also simplify ResetFieldTrialList below by just calling .reset() on
it with the new instance. (Also remove static from ResetFieldTrialList in that
case.)

[Mark P, 2013/08/05 22:11:41]

If I make the changes you suggest, I get a failure:
[30420:30420:0805/144359:958398008167:FATAL:field_trial.cc(213)] Check failed:
!global_. 

[Alexei Svitkine (slow), 2013/08/06 18:20:41]

Hmm, that should not be happening if you're actually creating the FieldTrialList
instance.

i.e. if you still call ResetFieldTrialList() from the constructor and in that
function, you do:

field_trial_list_.reset(
    new base::FieldTrialList(new metrics::SHA1EntropyProvider("foo")));

If you're doing that and that's still happening, does it give you stack trace?

[Mark P, 2013/08/06 19:52:18]

Stack trace below.  I put the code I used to generate this stack trace on the
patch set labeled "snapshot for Alexei".

./out/Debug/unit_tests --gtest_filter=OmniboxF*.*
Note: Google Test filter = OmniboxF*.*
[==========] Running 4 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 4 tests from OmniboxFieldTrialTest
[ RUN      ] OmniboxFieldTrialTest.GetDisabledProviderTypes
[5938:5938:0806/125041:1038000432976:WARNING:omnibox_field_trial.cc(138)]
Malformed DisabledProviders string: DisabledProviders_
[5938:5938:0806/125041:1038000433107:FATAL:field_trial.cc(213)] Check failed:
!global_. 
 [0x7f2ec09b58ac] base::debug::StackTrace::StackTrace()
 [0x7f2ec09f8553] logging::LogMessage::~LogMessage()
 [0x7f2ec0996fe3] base::FieldTrialList::FieldTrialList()
 [0x00000116ed65] OmniboxFieldTrialTest::ResetFieldTrialList()
 [0x00000116ba28]
OmniboxFieldTrialTest_GetDisabledProviderTypes_Test::TestBody()
 [0x00000337e2d6] testing::internal::HandleSehExceptionsInMethodIfSupported<>()
 [0x00000337bb3f] testing::internal::HandleExceptionsInMethodIfSupported<>()
 [0x000003371ef9] testing::Test::Run()
 [0x0000033724d6] testing::TestInfo::Run()
 [0x0000033729e4] testing::TestCase::Run()
 [0x000003376fe4] testing::internal::UnitTestImpl::RunAllTests()
 [0x00000337f0a7] testing::internal::HandleSehExceptionsInMethodIfSupported<>()
 [0x00000337c2d3] testing::internal::HandleExceptionsInMethodIfSupported<>()
 [0x00000337612f] testing::UnitTest::Run()
 [0x000003f24138] base::TestSuite::Run()
 [0x000003dfb4b6] content::UnitTestTestSuite::Run()
 [0x000003c454ed] main
 [0x7f2eb1fa476d] __libc_start_main
 [0x0000007e81a9] <unknown>

[5938:5938:0806/125041:1038000433107:FATAL:field_trial.cc(213)] Check failed:
!global_. 
 [0x7f2ec09b58ac] base::debug::StackTrace::StackTrace()
 [0x7f2ec09f8553] logging::LogMessage::~LogMessage()
 [0x7f2ec0996fe3] base::FieldTrialList::FieldTrialList()
 [0x00000116ed65] OmniboxFieldTrialTest::ResetFieldTrialList()
 [0x00000116ba28]
OmniboxFieldTrialTest_GetDisabledProviderTypes_Test::TestBody()
 [0x00000337e2d6] testing::internal::HandleSehExceptionsInMethodIfSupported<>()
 [0x00000337bb3f] testing::internal::HandleExceptionsInMethodIfSupported<>()
 [0x000003371ef9] testing::Test::Run()
 [0x0000033724d6] testing::TestInfo::Run()
 [0x0000033729e4] testing::TestCase::Run()
 [0x000003376fe4] testing::internal::UnitTestImpl::RunAllTests()
 [0x00000337f0a7] testing::internal::HandleSehExceptionsInMethodIfSupported<>()
 [0x00000337c2d3] testing::internal::HandleExceptionsInMethodIfSupported<>()
 [0x00000337612f] testing::UnitTest::Run()
 [0x000003f24138] base::TestSuite::Run()
 [0x000003dfb4b6] content::UnitTestTestSuite::Run()
 [0x000003c454ed] main
 [0x7f2eb1fa476d] __libc_start_main
 [0x0000007e81a9] <unknown>

     field_trial_list_ = NULL;
   }
 
   static void ResetFieldTrialList() {
     // It's important to delete the old pointer first which sets
     // FieldTrialList::global_ to NULL.
     if (field_trial_list_)
       delete field_trial_list_;
     field_trial_list_ = new base::FieldTrialList(
         new metrics::SHA1EntropyProvider("foo"));
+    chrome_variations::testing::ClearAllVariationParams();
     OmniboxFieldTrial::ActivateDynamicTrials();
   }
 
   // Creates and activates a field trial.
   static base::FieldTrial* CreateTestTrial(const std::string& name,
                                            const std::string& group_name) {
     base::FieldTrial* trial = base::FieldTrialList::CreateFieldTrial(
         name, group_name);
     trial->group();
     return trial;
   }
 
  private:
-  // Needed for Activate{Static/Dynamic}Trials().
   static base::FieldTrialList* field_trial_list_;
 
   DISALLOW_COPY_AND_ASSIGN(OmniboxFieldTrialTest);
 };
 
 // static
 base::FieldTrialList* OmniboxFieldTrialTest::field_trial_list_ = NULL;
 
 // Test if GetDisabledProviderTypes() properly parses various field trial
 // group names.
@@ skipping 66 matching lines @@
     ResetFieldTrialList();
     CreateTestTrial("AutocompleteDynamicTrial_2", "EnableZeroSuggest_Queries");
     EXPECT_TRUE(OmniboxFieldTrial::InZeroSuggestFieldTrial());
 
     ResetFieldTrialList();
     CreateTestTrial("AutocompleteDynamicTrial_3", "EnableZeroSuggest_URLs");
     EXPECT_TRUE(OmniboxFieldTrial::InZeroSuggestFieldTrial());
   }
 }
 
+TEST_F(OmniboxFieldTrialTest, GetDemotionsByTypeWithFallback) {
+  // Must be the same as kBundledExperimentFieldTrialName
+  // defined in omnibox_field_trial.cc.
+  const std::string kTrialName = "OmniboxBundledExperimentV1";
+  // Must be the same as kDemoteByTypeRule defined in
+  // omnibox_field_trial.cc.
+  const std::string kRuleName = "DemoteByType";
+  {
+    std::map<std::string, std::string> params;
+    params[kRuleName + ":1"] = "1:50,2:0";
+    params[kRuleName + ":3"] = "5:100";
+    params[kRuleName + ":*"] = "1:25";
+    ASSERT_TRUE(chrome_variations::AssociateVariationParams(
+        kTrialName, "A", params));
+  }
+  base::FieldTrialList::CreateFieldTrial(kTrialName, "A");
+  OmniboxFieldTrial::DemotionMultiplierByType demotions_by_type;
+  OmniboxFieldTrial::GetDemotionsByType(
+      AutocompleteInput::NEW_TAB_PAGE, &demotions_by_type);
+  ASSERT_EQ(2u, demotions_by_type.size());
+  EXPECT_FLOAT_EQ(0.5f, demotions_by_type[AutocompleteMatchType::HISTORY_URL]);
+  EXPECT_FLOAT_EQ(0.0f,
+      demotions_by_type[AutocompleteMatchType::HISTORY_TITLE]);

[Peter Kasting, 2013/08/06 19:33:50]

I think this expectation will still pass even if the code is broken, because
operator[] will add a value if it doesn't already exist in the map (and 0 would
be the default value).  You might want to use find() instead of [] for these
tests (perhaps via a small helper function to reduce verbosity?).

Nit: All lines of args must be aligned.

[Mark P, 2013/08/06 22:00:15]

Good point.  Added helper function.
now obsolete.

+  OmniboxFieldTrial::GetDemotionsByType(
+      AutocompleteInput::HOMEPAGE, &demotions_by_type);
+  ASSERT_EQ(1u, demotions_by_type.size());
+  EXPECT_FLOAT_EQ(1.0f, demotions_by_type[AutocompleteMatchType::NAVSUGGEST]);
+  OmniboxFieldTrial::GetDemotionsByType(
+      AutocompleteInput::BLANK, &demotions_by_type);
+  ASSERT_EQ(1u, demotions_by_type.size());
+  EXPECT_FLOAT_EQ(0.25f, demotions_by_type[AutocompleteMatchType::HISTORY_URL]);
+}
+
 TEST_F(OmniboxFieldTrialTest, GetValueForRuleInContext) {
   // Must be the same as kBundledExperimentFieldTrialName
   // defined in omnibox_field_trial.cc.
   const std::string kTrialName = "OmniboxBundledExperimentV1";
   {
     std::map<std::string, std::string> params;
     // Rule 1 has some exact matches and a global fallback.
     params["rule1:1"] = "rule1-1-value";  // NEW_TAB_PAGE
     params["rule1:3"] = "rule1-3-value";  // HOMEPAGE
     params["rule1:*"] = "rule1-*-value";  // global
@@ skipping 50 matching lines @@
       "rule3-4-value",
       OmniboxFieldTrial::GetValueForRuleInContext(
           "rule3", AutocompleteInput::OTHER));  // exact match
 
   // Tests for rule 4 (a missing rule).
   EXPECT_EQ(
       "",
       OmniboxFieldTrial::GetValueForRuleInContext(
           "rule4", AutocompleteInput::OTHER));  // no rule at all
 }