[omnibox] Delete |alternate_nav_url_| member from AutocompleteResult

We can compute this on-demand rather than storing another piece of
state we have to keep track of.

I don't think this has a negative performance impact overall, as we
rarely use the alternate nav URL (only on navigation basically).

It does simplify the bookkeeping though, after removing this.

Bug: 1016845, 363656
Change-Id: Ia32efb0b9bd83f591cc78f3304ad7521475402c6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1913241
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#715374}

components/omnibox/browser/autocomplete_classifier.cc

@@ -89,6 +89,8 @@ void AutocompleteClassifier::Classify(
   }
 
   *match = *result.begin();
-  if (alternate_nav_url)
-    *alternate_nav_url = result.alternate_nav_url();
+  if (alternate_nav_url) {
+    *alternate_nav_url =
+        AutocompleteResult::ComputeAlternateNavUrl(input, *match);
+  }
 }

components/omnibox/browser/autocomplete_result.cc

@@ -191,7 +191,6 @@ void AutocompleteResult::AppendMatches(const AutocompleteInput& input,
     }
   }
   default_match_ = end();
-  alternate_nav_url_ = GURL();
 }
 
 void AutocompleteResult::SortAndCull(
@@ -285,7 +284,6 @@ void AutocompleteResult::SortAndCull(
        (input.current_page_classification() ==
         metrics::OmniboxEventProto::CHROMEOS_APP_LIST))) {
     default_match_ = end();
-    alternate_nav_url_ = GURL();
     return;
   }
 
@@ -295,9 +293,6 @@ void AutocompleteResult::SortAndCull(
   // |allowed_to_be_default_match|, it will always be the default match.
   default_match_ = matches_.begin();
 
-  // TODO(tommycli): Simplify our state by not pre-computing this.
-  alternate_nav_url_ = ComputeAlternateNavUrl(input, *default_match_);
-
   // Almost all matches are "navigable": they have a valid |destination_url|.
   // One example exception is the user tabbing into keyword search mode,
   // but not having typed a query yet. In that case, the default match should
@@ -620,7 +615,6 @@ void AutocompleteResult::Swap(AutocompleteResult* other) {
   matches_.swap(other->matches_);
   default_match_ = begin() + other_default_match_offset;
   other->default_match_ = other->begin() + default_match_offset;
-  alternate_nav_url_.Swap(&(other->alternate_nav_url_));
 }
 
 void AutocompleteResult::CopyFrom(const AutocompleteResult& rhs) {
@@ -633,8 +627,6 @@ void AutocompleteResult::CopyFrom(const AutocompleteResult& rhs) {
   default_match_ = (rhs.default_match_ == rhs.end())
                        ? end()
                        : (begin() + (rhs.default_match_ - rhs.begin()));
-
-  alternate_nav_url_ = rhs.alternate_nav_url_;
 }
 
 #if DCHECK_IS_ON()
@@ -738,12 +730,7 @@ void AutocompleteResult::InlineTailPrefixes() {
 }
 
 size_t AutocompleteResult::EstimateMemoryUsage() const {
-  size_t res = 0;
-
-  res += base::trace_event::EstimateMemoryUsage(matches_);
-  res += base::trace_event::EstimateMemoryUsage(alternate_nav_url_);
-
-  return res;
+  return base::trace_event::EstimateMemoryUsage(matches_);
 }
 
 std::vector<AutocompleteResult::MatchDedupComparator>

components/omnibox/browser/autocomplete_result.h

@@ -121,8 +121,6 @@ class AutocompleteResult {
       const ACMatches& matches,
       const CompareWithDemoteByType<AutocompleteMatch>& comparing_object);
 
-  const GURL& alternate_nav_url() const { return alternate_nav_url_; }
-
   // Clears the matches for this result set.
   void Reset();
 
@@ -136,9 +134,8 @@ class AutocompleteResult {
   void Validate() const;
 #endif  // DCHECK_IS_ON()
 
-  // Compute the "alternate navigation URL" for a given match. This is obtained
-  // by interpreting the user input directly as a URL. See comments on
-  // |alternate_nav_url_|.
+  // Returns a URL to offer the user as an alternative navigation when they
+  // open |match| after typing in |input|.
   static GURL ComputeAlternateNavUrl(const AutocompleteInput& input,
                                      const AutocompleteMatch& match);
 
@@ -245,15 +242,6 @@ class AutocompleteResult {
 
   const_iterator default_match_;
 
-  // The "alternate navigation URL", if any, for this result set.  This is a URL
-  // to try offering as a navigational option in case the user navigated to the
-  // URL of the default match but intended something else.  For example, if the
-  // user's local intranet contains site "foo", and the user types "foo", we
-  // default to searching for "foo" when the user may have meant to navigate
-  // there.  In cases like this, the default match will point to the "search for
-  // 'foo'" result, and this will contain "http://foo/".
-  GURL alternate_nav_url_;
-
   DISALLOW_COPY_AND_ASSIGN(AutocompleteResult);
 };
 

components/omnibox/browser/autocomplete_result_unittest.cc

@@ -266,14 +266,38 @@ TEST_F(AutocompleteResultTest, Swap) {
   r1.AppendMatches(input, matches);
   r1.SortAndCull(input, template_url_service_.get());
   EXPECT_EQ(r1.begin(), r1.default_match());
-  EXPECT_EQ("http://a/", r1.alternate_nav_url().spec());
+
   r1.Swap(&r2);
   EXPECT_TRUE(r1.empty());
   EXPECT_EQ(r1.end(), r1.default_match());
-  EXPECT_TRUE(r1.alternate_nav_url().is_empty());
   ASSERT_FALSE(r2.empty());
   EXPECT_EQ(r2.begin(), r2.default_match());
-  EXPECT_EQ("http://a/", r2.alternate_nav_url().spec());
+}
+
+TEST_F(AutocompleteResultTest, AlternateNavUrl) {
+  AutocompleteInput input(base::ASCIIToUTF16("a"),
+                          metrics::OmniboxEventProto::OTHER,
+                          TestSchemeClassifier());
+
+  // Against search matches, we should generate an alternate nav URL.
+  {
+    AutocompleteMatch match;
+    match.type = AutocompleteMatchType::SEARCH_SUGGEST;
+    match.destination_url = GURL("http://www.foo.com/s?q=foo");
+    GURL alternate_nav_url =
+        AutocompleteResult::ComputeAlternateNavUrl(input, match);
+    EXPECT_EQ("http://a/", alternate_nav_url.spec());
+  }
+
+  // Against matching URL matches, we should NOT generate an alternate nav URL.
+  {
+    AutocompleteMatch match;
+    match.type = AutocompleteMatchType::SEARCH_SUGGEST;
+    match.destination_url = GURL("http://a/");
+    GURL alternate_nav_url =
+        AutocompleteResult::ComputeAlternateNavUrl(input, match);
+    EXPECT_FALSE(alternate_nav_url.is_valid());
+  }
 }
 
 // Tests that if the new results have a lower max relevance score than last,

components/omnibox/browser/omnibox_edit_model.cc

@@ -1524,8 +1524,10 @@ void OmniboxEditModel::GetInfoForCurrentText(AutocompleteMatch* match,
       found_match_for_text = true;
     }
     if (found_match_for_text && alternate_nav_url &&
-        (!popup_model() || !popup_model()->has_selected_match()))
-      *alternate_nav_url = result().alternate_nav_url();
+        (!popup_model() || !popup_model()->has_selected_match())) {
+      *alternate_nav_url =
+          AutocompleteResult::ComputeAlternateNavUrl(input_, *match);
+    }
   }
 
   if (!found_match_for_text) {