Add DCHECK that HistoryURLProvider is on the main thread

chrome/browser/autocomplete/history_url_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/history_url_provider.h"
 
 #include <algorithm>
 
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/message_loop.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/autocomplete/autocomplete_field_trial.h"
 #include "chrome/browser/autocomplete/autocomplete_match.h"
 #include "chrome/browser/history/history.h"
 #include "chrome/browser/history/history_backend.h"
 #include "chrome/browser/history/history_database.h"
 #include "chrome/browser/history/history_types.h"
 #include "chrome/browser/net/url_fixer_upper.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
+#include "content/public/browser/browser_thread.h"
 #include "googleurl/src/gurl.h"
 #include "googleurl/src/url_parse.h"
 #include "googleurl/src/url_util.h"
 #include "net/base/net_util.h"
 #include "net/base/registry_controlled_domain.h"
 
 namespace {
 
 // Ensures that |matches| contains an entry for |info|, which may mean adding a
 // new such entry (using |input_location| and |match_in_scheme|).
@@ skipping 301 matching lines @@
     // Either: switch_value == switches::kOmniboxAggressiveHistoryURLAuto
     // or someone passed an invalid command line flag.  We'll default
     // the latter case to automatic but report an error.
     if (!switch_value.empty() &&
         (switch_value != switches::kOmniboxAggressiveHistoryURLAuto)) {
       LOG(ERROR) << "Invalid --omnibox-aggressive-with-history-url option "
                  << "received on command line: " << switch_value;
       LOG(ERROR) << "Making automatic.";
     }
     // Automatic means eligible for the field trial.
+    // For the field trial stuff to work correctly, we must be running

[Peter Kasting, 2012/02/09 21:51:19]

I wonder if the field trial is the only requirement imposition here.  It seems
like to use the in-memory data structures we also need to be on the main thread?
 Maybe this check should move to the top of the constructor?  Might be good to
ask Mike.

[mrossetti, 2012/02/09 21:58:58]

I was considering suggesting that the DCHECK be placed in the
AutocompleteProvider ctor.

On 2012/02/09 21:51:19, Peter Kasting wrote:

[Mark P, 2012/02/09 22:17:47]

I think we should pepper our code with these DCHECKs where we think it's
appropriate.  I think this place is a good stop for this one because we have a
good reason to put one here.  I'm not opposed to adding others (in other
changelists).

On 2012/02/09 21:58:58, mrossetti wrote:

+    // on the same thread as the thread that created the field trial,
+    // which happens via a call to AutocompleteFieldTrial::Active in
+    // chrome_browser_main.cc on the main thread.  Let's check this to
+    // be sure.  We check "if we've heard of the UI thread then we'd better
+    // be on it."  The first part is necessary so unit tests pass.  (Many
+    // unit tests don't set up the threading naming system; hence
+    // CurrentlyOn(UI thread will fail.)

[Peter Kasting, 2012/02/09 21:51:19]

Nit: Missing paren after "thread"

[Mark P, 2012/02/09 22:17:47]

Fixed.

+    DCHECK(!content::BrowserThread::IsWellKnownThread(
+               content::BrowserThread::UI) ||
+           content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
     if (AutocompleteFieldTrial::InAggressiveHUPFieldTrial()) {
       if (AutocompleteFieldTrial::InAggressiveHUPFieldTrialExperimentGroup()) {
         enable_aggressive_scoring_ = true;
         aggressiveness_option = AGGRESSIVENESS_FIELD_TRIAL_EXPERIMENT_GROUP;
       } else {
         enable_aggressive_scoring_ = false;
         aggressiveness_option = AGGRESSIVENESS_FIELD_TRIAL_DEFAULT_GROUP;
       }
     } else {
       enable_aggressive_scoring_ = false;
@@ skipping 637 matching lines @@
         &match.contents_class);
   }
   match.description = info.title();
   AutocompleteMatch::ClassifyMatchInString(params->input.text(),
                                            info.title(),
                                            ACMatchClassification::NONE,
                                            &match.description_class);
 
   return match;
 }