Refuse invalid SearchProvider and OSDD suggest URLs; etc.

chrome/browser/autocomplete/search_provider.cc

 // Copyright (c) 2012 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/autocomplete/search_provider.h"
 
 #include <algorithm>
 #include <cmath>
 
 #include "base/callback.h"
@@ skipping 294 matching lines @@
     // Sort in descending relevance order.
     return a.relevance() > b.relevance();
   }
 };
 
 void SearchProvider::Run() {
   // Start a new request with the current input.
   DCHECK(!done_);
   suggest_results_pending_ = 0;
   time_suggest_request_sent_ = base::TimeTicks::Now();
-  const TemplateURL* default_url = providers_.GetDefaultProviderURL();
-  if (default_url && !default_url->suggestions_url().empty()) {
-    suggest_results_pending_++;
-    LogOmniboxSuggestRequest(REQUEST_SENT);
-    default_fetcher_.reset(CreateSuggestFetcher(kDefaultProviderURLFetcherID,
-        default_url->suggestions_url_ref(), input_.text()));
-  }
-  const TemplateURL* keyword_url = providers_.GetKeywordProviderURL();
-  if (keyword_url && !keyword_url->suggestions_url().empty()) {
-    suggest_results_pending_++;
-    LogOmniboxSuggestRequest(REQUEST_SENT);
-    keyword_fetcher_.reset(CreateSuggestFetcher(kKeywordProviderURLFetcherID,
-        keyword_url->suggestions_url_ref(), keyword_input_text_));
-  }
+
+  default_fetcher_.reset(CreateSuggestFetcher(kDefaultProviderURLFetcherID,
+      providers_.GetDefaultProviderURL(), input_.text()));
+  keyword_fetcher_.reset(CreateSuggestFetcher(kKeywordProviderURLFetcherID,
+      providers_.GetKeywordProviderURL(), keyword_input_text_));
 
   // Both the above can fail if the providers have been modified or deleted
   // since the query began.
   if (suggest_results_pending_ == 0) {
     UpdateDone();
     // We only need to update the listener if we're actually done.
     if (done_)
       listener_->OnProviderUpdate(false);
   }
 }
@@ skipping 295 matching lines @@
 void SearchProvider::ApplyCalculatedNavigationRelevance(NavigationResults* list,
                                                         bool is_keyword) {
   for (size_t i = 0; i < list->size(); ++i) {
     (*list)[i].set_relevance(CalculateRelevanceForNavigation(is_keyword) +
                              (list->size() - i - 1));
   }
 }
 
 net::URLFetcher* SearchProvider::CreateSuggestFetcher(
     int id,
-    const TemplateURLRef& suggestions_url,
+    const TemplateURL* template_url,
     const string16& text) {
-  DCHECK(suggestions_url.SupportsReplacement());
-  net::URLFetcher* fetcher = net::URLFetcher::Create(id,
-      GURL(suggestions_url.ReplaceSearchTerms(
-          TemplateURLRef::SearchTermsArgs(text))),
-      net::URLFetcher::GET, this);
+  if (!template_url || template_url->suggestions_url().empty() ||
+      !template_url->suggestions_url_ref().IsValid())

[Peter Kasting, 2012/09/05 23:31:27]

You don't need to check IsValid(); if it fails, ReplaceSearchTerms() below will
return the empty string and you'll fail the next conditional.

[msw, 2012/09/06 20:02:29]

Done.

+    return NULL;
+
+  // Bail if the suggestion URL is invalid with the given replacements.
+  GURL suggest_url(template_url->suggestions_url_ref().ReplaceSearchTerms(
+                       TemplateURLRef::SearchTermsArgs(text)));

[Peter Kasting, 2012/09/05 23:31:27]

Nit: Indent 4 from beginning of line

[msw, 2012/09/06 20:02:29]

Done.

+  if (!suggest_url.is_valid())
+    return NULL;
+
+  suggest_results_pending_++;
+  LogOmniboxSuggestRequest(REQUEST_SENT);
+
+  net::URLFetcher* fetcher =
+      net::URLFetcher::Create(id, suggest_url, net::URLFetcher::GET, this);
   fetcher->SetRequestContext(profile_->GetRequestContext());
   fetcher->SetLoadFlags(net::LOAD_DO_NOT_SAVE_COOKIES);
   fetcher->Start();
   return fetcher;
 }
 
 bool SearchProvider::ParseSuggestResults(Value* root_val, bool is_keyword) {
   // TODO(pkasting): Fix |have_suggest_results_|; see http://crbug.com/130631
   have_suggest_results_ = false;
 
@@ skipping 570 matching lines @@
       ACMatchClassification::NONE, &match.description_class);
   return match;
 }
 
 void SearchProvider::UpdateDone() {
   // We're done when the timer isn't running, there are no suggest queries
   // pending, and we're not waiting on instant.
   done_ = (!timer_.IsRunning() && (suggest_results_pending_ == 0) &&
            (instant_finalized_ || !InstantController::IsEnabled(profile_)));
 }