Omnibox: Cleanup Comments by Page Classification Enum

Followed up previous code review; see linked bug.

Once you approve of these comments, I'll revise them in the other
two places (google-internal protocol file and autocomplete_input.h).

BUG=264938

Review URL: https://chromiumcodereview.appspot.com/22912009

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

7 files changed, 88 insertions, 51 deletions

diff --git a/chrome/browser/autocomplete/autocomplete_input.h b/chrome/browser/autocomplete/autocomplete_input.h
index 3aba2d1..6c6b2e8 100644
--- a/chrome/browser/autocomplete/autocomplete_input.h
+++ b/chrome/browser/autocomplete/autocomplete_input.h
@@ -50,25 +50,43 @@ class AutocompleteInput {
   // and update omnibox_event.proto::PageClassification and
   // omnibox_edit_model.cc::ClassifyPage() too.
   enum PageClassification {
-    INVALID_SPEC = 0,   // invalid URI; shouldn't happen
-    NEW_TAB_PAGE = 1,   // chrome://newtab/
-    // Note that chrome://newtab/ doesn't have to be the built-in
-    // version; it could be replaced by an extension.
-    BLANK = 2,          // about:blank
-    HOMEPAGE = 3,       // user switched settings to "open this page" mode.
-    // Note that if the homepage is set to the new tab page or about blank,
-    // then we'll classify the web page into those categories, not HOMEPAGE.
-    OTHER = 4,          // everything not included somewhere else on this list
+    // An invalid URL; shouldn't happen.
+    INVALID_SPEC = 0,
+
+    // chrome://newtab/.  This can be either the built-in version or a
+    // replacement new tab page from an extension.  Note that when Instant
+    // Extended is enabled, the new tab page will be reported as either
+    // INSTANT_NEW_TAB_PAGE_WITH_OMNIBOX_AS_STARTING_FOCUS or
+    // INSTANT_NEW_TAB_PAGE_WITH_FAKEBOX_AS_STARTING_FOCUS below,
+    // unless an extension is replacing the new tab page, in which case
+    // it will still be reported as NEW_TAB_PAGE.
+    NEW_TAB_PAGE = 1,
+
+    // about:blank.
+    BLANK = 2,
+
+    // The user's home page.  Note that if the home page is set to any
+    // of the new tab page versions or to about:blank, then we'll
+    // classify the page into those categories, not HOME_PAGE.
+    HOME_PAGE = 3,
+
+    // The catch-all entry of everything not included somewhere else
+    // on this list.
+    OTHER = 4,
+
     // The user is on a search result page that's doing search term
     // replacement, meaning the search terms should've appeared in the omnibox
     // before the user started editing it, not the URL of the page.
     SEARCH_RESULT_PAGE_DOING_SEARCH_TERM_REPLACEMENT = 6,
+
     // The new tab page in which this omnibox interaction first started
     // with the user having focus in the omnibox.
     INSTANT_NEW_TAB_PAGE_WITH_OMNIBOX_AS_STARTING_FOCUS = 7,
+
     // The new tab page in which this omnibox interaction first started
     // with the user having focus in the fakebox.
     INSTANT_NEW_TAB_PAGE_WITH_FAKEBOX_AS_STARTING_FOCUS = 8,
+
     // The user is on a search result page that's not doing search term
     // replacement, meaning the URL of the page should've appeared in the
     // omnibox before the user started editing it, not the search terms.
diff --git a/chrome/browser/autocomplete/autocomplete_result_unittest.cc b/chrome/browser/autocomplete/autocomplete_result_unittest.cc
index dc8ed62..e51c9da 100644
--- a/chrome/browser/autocomplete/autocomplete_result_unittest.cc
+++ b/chrome/browser/autocomplete/autocomplete_result_unittest.cc
@@ -351,7 +351,7 @@ TEST_F(AutocompleteResultTest, SortAndCullWithDemotionsByType) {
   {
     std::map<std::string, std::string> params;
     params[std::string(OmniboxFieldTrial::kDemoteByTypeRule) + ":3:*"] =
-        "1:50,7:100,2:0";  // 3 == HOMEPAGE
+        "1:50,7:100,2:0";  // 3 == HOME_PAGE
     ASSERT_TRUE(chrome_variations::AssociateVariationParams(
         OmniboxFieldTrial::kBundledExperimentFieldTrialName, "A", params));
   }
@@ -361,7 +361,7 @@ TEST_F(AutocompleteResultTest, SortAndCullWithDemotionsByType) {
   AutocompleteResult result;
   result.AppendMatches(matches);
   AutocompleteInput input(string16(), string16::npos, string16(), GURL(),
-                          AutocompleteInput::HOMEPAGE, false, false, false,
+                          AutocompleteInput::HOME_PAGE, false, false, false,
                           AutocompleteInput::ALL_MATCHES);
   result.SortAndCull(input, test_util_.profile());
 
@@ -386,7 +386,7 @@ TEST_F(AutocompleteResultTest, SortAndCullReorderForDefaultMatch) {
   };
 
   std::map<std::string, std::string> params;
-  // Enable reorder for omnibox inputs on the user's homepage.
+  // Enable reorder for omnibox inputs on the user's home page.
   params[std::string(OmniboxFieldTrial::kReorderForLegalDefaultMatchRule) +
          ":3:*"] = OmniboxFieldTrial::kReorderForLegalDefaultMatchRuleEnabled;
   ASSERT_TRUE(chrome_variations::AssociateVariationParams(
@@ -403,7 +403,7 @@ TEST_F(AutocompleteResultTest, SortAndCullReorderForDefaultMatch) {
     AutocompleteResult result;
     result.AppendMatches(matches);
     AutocompleteInput input(string16(), string16::npos, string16(), GURL(),
-                            AutocompleteInput::HOMEPAGE, false, false, false,
+                            AutocompleteInput::HOME_PAGE, false, false, false,
                             AutocompleteInput::ALL_MATCHES);
     result.SortAndCull(input, test_util_.profile());
     AssertResultMatches(result, data, 4);
@@ -418,7 +418,7 @@ TEST_F(AutocompleteResultTest, SortAndCullReorderForDefaultMatch) {
     AutocompleteResult result;
     result.AppendMatches(matches);
     AutocompleteInput input(string16(), string16::npos, string16(), GURL(),
-                            AutocompleteInput::HOMEPAGE, false, false, false,
+                            AutocompleteInput::HOME_PAGE, false, false, false,
                             AutocompleteInput::ALL_MATCHES);
     result.SortAndCull(input, test_util_.profile());
     ASSERT_EQ(4U, result.size());
diff --git a/chrome/browser/metrics/metrics_log.cc b/chrome/browser/metrics/metrics_log.cc
index d5403271..7a0f210 100644
--- a/chrome/browser/metrics/metrics_log.cc
+++ b/chrome/browser/metrics/metrics_log.cc
@@ -150,8 +150,8 @@ OmniboxEventProto::PageClassification AsOmniboxEventPageClassification(
       return OmniboxEventProto::NEW_TAB_PAGE;
     case AutocompleteInput::BLANK:
       return OmniboxEventProto::BLANK;
-    case AutocompleteInput::HOMEPAGE:
-      return OmniboxEventProto::HOMEPAGE;
+    case AutocompleteInput::HOME_PAGE:
+      return OmniboxEventProto::HOME_PAGE;
     case AutocompleteInput::OTHER:
       return OmniboxEventProto::OTHER;
     case AutocompleteInput::SEARCH_RESULT_PAGE_DOING_SEARCH_TERM_REPLACEMENT:
diff --git a/chrome/browser/omnibox/omnibox_field_trial_unittest.cc b/chrome/browser/omnibox/omnibox_field_trial_unittest.cc
index 9439bd2..adb43a8 100644
--- a/chrome/browser/omnibox/omnibox_field_trial_unittest.cc
+++ b/chrome/browser/omnibox/omnibox_field_trial_unittest.cc
@@ -179,7 +179,7 @@ TEST_F(OmniboxFieldTrialTest, GetDemotionsByTypeWithFallback) {
   VerifyDemotion(demotions_by_type, AutocompleteMatchType::HISTORY_URL, 0.5);
   VerifyDemotion(demotions_by_type, AutocompleteMatchType::HISTORY_TITLE, 0.0);
   OmniboxFieldTrial::GetDemotionsByType(
-      AutocompleteInput::HOMEPAGE, &demotions_by_type);
+      AutocompleteInput::HOME_PAGE, &demotions_by_type);
   ASSERT_EQ(1u, demotions_by_type.size());
   VerifyDemotion(demotions_by_type, AutocompleteMatchType::NAVSUGGEST, 1.0);
   OmniboxFieldTrial::GetDemotionsByType(
@@ -196,7 +196,7 @@ TEST_F(OmniboxFieldTrialTest, GetValueForRuleInContext) {
     std::map<std::string, std::string> params;
     // Rule 1 has some exact matches and fallbacks at every level.
     params["rule1:1:0"] = "rule1-1-0-value";  // NEW_TAB_PAGE
-    params["rule1:3:0"] = "rule1-3-0-value";  // HOMEPAGE
+    params["rule1:3:0"] = "rule1-3-0-value";  // HOME_PAGE
     params["rule1:4:1"] = "rule1-4-1-value";  // OTHER
     params["rule1:4:*"] = "rule1-4-*-value";  // OTHER
     params["rule1:*:1"] = "rule1-*-1-value";  // global
@@ -225,38 +225,38 @@ TEST_F(OmniboxFieldTrialTest, GetValueForRuleInContext) {
   ExpectRuleValue("rule1-1-0-value",
                   "rule1", AutocompleteInput::NEW_TAB_PAGE);  // exact match
   ExpectRuleValue("rule1-*-*-value",
-                  "rule1", AutocompleteInput::BLANK);     // fallback to global
+                  "rule1", AutocompleteInput::BLANK);      // fallback to global
   ExpectRuleValue("rule1-3-0-value",
-                  "rule1", AutocompleteInput::HOMEPAGE);  // exact match
+                  "rule1", AutocompleteInput::HOME_PAGE);  // exact match
   ExpectRuleValue("rule1-4-*-value",
-                  "rule1", AutocompleteInput::OTHER);     // partial fallback
+                  "rule1", AutocompleteInput::OTHER);      // partial fallback
   ExpectRuleValue("rule1-*-*-value",
                   "rule1",
-                  AutocompleteInput::                     // fallback to global
+                  AutocompleteInput::                      // fallback to global
                       SEARCH_RESULT_PAGE_DOING_SEARCH_TERM_REPLACEMENT);
   // Tests for rule 2.
   ExpectRuleValue("rule2-*-0-value",
-                  "rule2", AutocompleteInput::HOMEPAGE);  // partial fallback
+                  "rule2", AutocompleteInput::HOME_PAGE);  // partial fallback
   ExpectRuleValue("rule2-*-0-value",
-                  "rule2", AutocompleteInput::OTHER);     // partial fallback
+                  "rule2", AutocompleteInput::OTHER);      // partial fallback
 
   // Tests for rule 3.
   ExpectRuleValue("rule3-*-*-value",
-                  "rule3", AutocompleteInput::HOMEPAGE);  // fallback to global
+                  "rule3", AutocompleteInput::HOME_PAGE);  // fallback to global
   ExpectRuleValue("rule3-*-*-value",
-                  "rule3", AutocompleteInput::OTHER);     // fallback to global
+                  "rule3", AutocompleteInput::OTHER);      // fallback to global
 
   // Tests for rule 4.
   ExpectRuleValue("",
-                  "rule4", AutocompleteInput::BLANK);     // no global fallback
+                  "rule4", AutocompleteInput::BLANK);      // no global fallback
   ExpectRuleValue("",
-                  "rule4", AutocompleteInput::HOMEPAGE);  // no global fallback
+                  "rule4", AutocompleteInput::HOME_PAGE);  // no global fallback
   ExpectRuleValue("rule4-4-0-value",
-                  "rule4", AutocompleteInput::OTHER);     // exact match
+                  "rule4", AutocompleteInput::OTHER);      // exact match
 
   // Tests for rule 5 (a missing rule).
   ExpectRuleValue("",
-                  "rule5", AutocompleteInput::OTHER);     // no rule at all
+                  "rule5", AutocompleteInput::OTHER);      // no rule at all
 
   // Now change the Instant Extended state and run analogous tests.
   // Instant Extended only works on non-mobile platforms.
@@ -268,34 +268,34 @@ TEST_F(OmniboxFieldTrialTest, GetValueForRuleInContext) {
   // Tests with Instant Extended enabled.
   // Tests for rule 1.
   ExpectRuleValue("rule1-4-1-value",
-                  "rule1", AutocompleteInput::OTHER);     // exact match
+                  "rule1", AutocompleteInput::OTHER);      // exact match
   ExpectRuleValue("rule1-*-1-value",
-                  "rule1", AutocompleteInput::BLANK);     // partial fallback
+                  "rule1", AutocompleteInput::BLANK);      // partial fallback
   ExpectRuleValue("rule1-*-1-value",
                   "rule1",
-                  AutocompleteInput::NEW_TAB_PAGE);       // partial fallback
+                  AutocompleteInput::NEW_TAB_PAGE);        // partial fallback
 
   // Tests for rule 2.
   ExpectRuleValue("rule2-1-*-value",
                   "rule2",
-                  AutocompleteInput::NEW_TAB_PAGE);       // partial fallback
+                  AutocompleteInput::NEW_TAB_PAGE);        // partial fallback
   ExpectRuleValue("rule2-*-*-value",
-                  "rule2", AutocompleteInput::OTHER);     // global fallback
+                  "rule2", AutocompleteInput::OTHER);      // global fallback
 
   // Tests for rule 3.
   ExpectRuleValue("rule3-*-*-value",
-                  "rule3", AutocompleteInput::HOMEPAGE);  // global fallback
+                  "rule3", AutocompleteInput::HOME_PAGE);  // global fallback
   ExpectRuleValue("rule3-*-*-value",
-                  "rule3", AutocompleteInput::OTHER);     // global fallback
+                  "rule3", AutocompleteInput::OTHER);      // global fallback
 
   // Tests for rule 4.
   ExpectRuleValue("",
-                  "rule4", AutocompleteInput::BLANK);     // no global fallback
+                  "rule4", AutocompleteInput::BLANK);      // no global fallback
   ExpectRuleValue("",
-                  "rule4", AutocompleteInput::HOMEPAGE);  // no global fallback
+                  "rule4", AutocompleteInput::HOME_PAGE);  // no global fallback
 
   // Tests for rule 5 (a missing rule).
   ExpectRuleValue("",
-                  "rule5", AutocompleteInput::OTHER);     // no rule at all
+                  "rule5", AutocompleteInput::OTHER);      // no rule at all
 #endif  // !defined(OS_IOS) && !defined(OS_ANDROID)
 }
diff --git a/chrome/browser/search_engines/template_url_unittest.cc b/chrome/browser/search_engines/template_url_unittest.cc
index 886583e..5882e5d 100644
--- a/chrome/browser/search_engines/template_url_unittest.cc
+++ b/chrome/browser/search_engines/template_url_unittest.cc
@@ -1195,7 +1195,7 @@ TEST_F(TemplateURLTest, ReplacePageClassification) {
   EXPECT_EQ("http://www.google.com/?pgcl=1&q=foo", result);
 
   search_terms_args.page_classification =
-      AutocompleteInput::HOMEPAGE;
+      AutocompleteInput::HOME_PAGE;
   result = url.url_ref().ReplaceSearchTerms(search_terms_args);
   EXPECT_EQ("http://www.google.com/?pgcl=3&q=foo", result);
 }
diff --git a/chrome/browser/ui/omnibox/omnibox_edit_model.cc b/chrome/browser/ui/omnibox/omnibox_edit_model.cc
index 70dac1f..6b0241a 100644
--- a/chrome/browser/ui/omnibox/omnibox_edit_model.cc
+++ b/chrome/browser/ui/omnibox/omnibox_edit_model.cc
@@ -1320,7 +1320,7 @@ AutocompleteInput::PageClassification OmniboxEditModel::ClassifyPage() const {
   if (url == content::kAboutBlankURL)
     return AutocompleteInput::BLANK;
   if (url == profile()->GetPrefs()->GetString(prefs::kHomePage))
-    return AutocompleteInput::HOMEPAGE;
+    return AutocompleteInput::HOME_PAGE;
   if (controller_->GetToolbarModel()->WouldReplaceSearchURLWithSearchTerms(
       true))
     return AutocompleteInput::SEARCH_RESULT_PAGE_DOING_SEARCH_TERM_REPLACEMENT;
diff --git a/chrome/common/metrics/proto/omnibox_event.proto b/chrome/common/metrics/proto/omnibox_event.proto
index ebe0ab6..8f36337 100644
--- a/chrome/common/metrics/proto/omnibox_event.proto
+++ b/chrome/common/metrics/proto/omnibox_event.proto
@@ -64,27 +64,46 @@ message OmniboxEventProto {
 
   // The type of page currently displayed when the user used the omnibox.
   enum PageClassification {
-    INVALID_SPEC = 0;   // invalid URI; shouldn't happen
-    NEW_TAB_PAGE = 1;   // chrome://newtab/
-    // Note that chrome://newtab/ doesn't have to be the built-in
-    // version; it could be replaced by an extension.
-    BLANK = 2;          // about:blank
-    HOMEPAGE = 3;       // user switched settings to "open this page" mode.
-    // Note that if the homepage is set to the new tab page or about blank,
-    // then we'll classify the web page into those categories, not HOMEPAGE.
-    OTHER = 4;          // everything not included somewhere else on this list
+    // An invalid URL; shouldn't happen.
+    INVALID_SPEC = 0;
+
+    // chrome://newtab/.  This can be either the built-in version or a
+    // replacement new tab page from an extension.  Note that when Instant
+    // Extended is enabled, the new tab page will be reported as either
+    // INSTANT_NEW_TAB_PAGE_WITH_OMNIBOX_AS_STARTING_FOCUS or
+    // INSTANT_NEW_TAB_PAGE_WITH_FAKEBOX_AS_STARTING_FOCUS below,
+    // unless an extension is replacing the new tab page, in which case
+    // it will still be reported as NEW_TAB_PAGE.
+    NEW_TAB_PAGE = 1;
+
+    // about:blank.
+    BLANK = 2;
+
+    // The user's home page.  Note that if the home page is set to any
+    // of the new tab page versions or to about:blank, then we'll
+    // classify the page into those categories, not HOME_PAGE.
+    HOME_PAGE = 3;
+
+    // The catch-all entry of everything not included somewhere else
+    // on this list.
+    OTHER = 4;
+
     // The instant new tab page enum value was deprecated on August 2, 2013.
     OBSOLETE_INSTANT_NEW_TAB_PAGE = 5;
+
     // The user is on a search result page that's doing search term
     // replacement, meaning the search terms should've appeared in the omnibox
     // before the user started editing it, not the URL of the page.
     SEARCH_RESULT_PAGE_DOING_SEARCH_TERM_REPLACEMENT = 6;
+
     // The new tab page in which this omnibox interaction first started
     // with the user having focus in the omnibox.
     INSTANT_NEW_TAB_PAGE_WITH_OMNIBOX_AS_STARTING_FOCUS = 7;
+
     // The new tab page in which this omnibox interaction first started
     // with the user having focus in the fakebox.
     INSTANT_NEW_TAB_PAGE_WITH_FAKEBOX_AS_STARTING_FOCUS = 8;
+
     // The user is on a search result page that's not doing search term
     // replacement, meaning the URL of the page should've appeared in the
     // omnibox before the user started editing it, not the search terms.