diff options
| -rw-r--r-- | chrome/browser/browser_main.cc | 15 | ||||
| -rw-r--r-- | chrome/browser/google_url_tracker.cc | 61 | ||||
| -rw-r--r-- | chrome/browser/google_url_tracker.h | 65 | ||||
| -rw-r--r-- | chrome/browser/profile.cc | 6 | ||||
| -rw-r--r-- | chrome/browser/template_url_model.cc | 45 | ||||
| -rw-r--r-- | chrome/common/notification_types.h | 6 |
6 files changed, 168 insertions, 30 deletions
diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc index 67bc255..15703a2 100644 --- a/chrome/browser/browser_main.cc +++ b/chrome/browser/browser_main.cc @@ -453,6 +453,21 @@ int BrowserMain(CommandLine &parsed_command_line, int show_command, ShellIntegration::VerifyInstallation(); browser_process->InitBrokerServices(broker_services); + + // In unittest mode, this will do nothing. In normal mode, this will create + // the global GoogleURLTracker instance, which will promptly go to sleep for + // five seconds (to avoid slowing startup), and wake up afterwards to see if + // it should do anything else. If we don't cause this creation now, it won't + // happen until someone else asks for the tracker, at which point we may no + // longer want to sleep for five seconds. + // + // A simpler way of doing all this would be to have some function which could + // give the time elapsed since startup, and simply have the tracker check that + // when asked to initialize itself, but this doesn't seem to exist. + // + // This can't be created in the BrowserProcessImpl constructor because it + // needs to read prefs that get set after that runs. + browser_process->google_url_tracker(); // Have Chrome plugins write their data to the profile directory. PluginService::GetInstance()->SetChromePluginDataDir(profile->GetPath()); diff --git a/chrome/browser/google_url_tracker.cc b/chrome/browser/google_url_tracker.cc index 5c2830d..be092c6 100644 --- a/chrome/browser/google_url_tracker.cc +++ b/chrome/browser/google_url_tracker.cc @@ -7,7 +7,6 @@ #include "base/string_util.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/profile.h" -#include "chrome/common/notification_service.h" #include "chrome/common/pref_names.h" #include "chrome/common/pref_service.h" #include "net/base/load_flags.h" @@ -19,7 +18,15 @@ GoogleURLTracker::GoogleURLTracker() : google_url_(g_browser_process->local_state()->GetString( prefs::kLastKnownGoogleURL)), #pragma warning(suppress: 4355) // Okay to pass "this" here. - fetcher_factory_(this) { + fetcher_factory_(this), + in_startup_sleep_(true), + already_fetched_(false), + need_to_fetch_(false), + request_context_available_(!!Profile::GetDefaultRequestContext()) { + NotificationService::current()->AddObserver(this, + NOTIFY_DEFAULT_REQUEST_CONTEXT_AVAILABLE, + NotificationService::AllSources()); + // Because this function can be called during startup, when kicking off a URL // fetch can eat up 20 ms of time, we delay five seconds, which is hopefully // long enough to be after startup, but still get results back quickly. @@ -28,16 +35,31 @@ GoogleURLTracker::GoogleURLTracker() // no function to do this. static const int kStartFetchDelayMS = 5000; MessageLoop::current()->PostDelayedTask(FROM_HERE, - fetcher_factory_.NewRunnableMethod(&GoogleURLTracker::StartFetch), + fetcher_factory_.NewRunnableMethod(&GoogleURLTracker::FinishSleep), kStartFetchDelayMS); } +GoogleURLTracker::~GoogleURLTracker() { + NotificationService::current()->RemoveObserver(this, + NOTIFY_DEFAULT_REQUEST_CONTEXT_AVAILABLE, + NotificationService::AllSources()); +} + +// static GURL GoogleURLTracker::GoogleURL() { const GoogleURLTracker* const tracker = g_browser_process->google_url_tracker(); return tracker ? tracker->google_url_ : GURL(kDefaultGoogleHomepage); } +// static +void GoogleURLTracker::RequestServerCheck() { + GoogleURLTracker* const tracker = g_browser_process->google_url_tracker(); + if (tracker) + tracker->SetNeedToFetch(); +} + +// static void GoogleURLTracker::RegisterPrefs(PrefService* prefs) { prefs->RegisterStringPref(prefs::kLastKnownGoogleURL, ASCIIToWide(kDefaultGoogleHomepage)); @@ -71,7 +93,31 @@ bool GoogleURLTracker::CheckAndConvertToGoogleBaseURL(const GURL& url, return true; } -void GoogleURLTracker::StartFetch() { +void GoogleURLTracker::SetNeedToFetch() { + need_to_fetch_ = true; + StartFetchIfDesirable(); +} + +void GoogleURLTracker::FinishSleep() { + in_startup_sleep_ = false; + StartFetchIfDesirable(); +} + +void GoogleURLTracker::StartFetchIfDesirable() { + // Bail if a fetch isn't appropriate right now. This function will be called + // again each time one of the preconditions changes, so we'll fetch + // immediately once all of them are met. + // + // See comments in header on the class, on RequestServerCheck(), and on the + // various members here for more detail on exactly what the conditions are. + if (in_startup_sleep_ || already_fetched_ || !need_to_fetch_ || + !request_context_available_) + return; + + need_to_fetch_ = false; + already_fetched_ = true; // If fetching fails, we don't bother to reset this + // flag; we just live with an outdated URL for this + // run of the browser. fetcher_.reset(new URLFetcher(GURL(kDefaultGoogleHomepage), URLFetcher::HEAD, this)); fetcher_->set_load_flags(net::LOAD_DISABLE_CACHE); @@ -111,3 +157,10 @@ void GoogleURLTracker::OnURLFetchComplete(const URLFetcher* source, } } +void GoogleURLTracker::Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details) { + DCHECK_EQ(NOTIFY_DEFAULT_REQUEST_CONTEXT_AVAILABLE, type); + request_context_available_ = true; + StartFetchIfDesirable(); +} diff --git a/chrome/browser/google_url_tracker.h b/chrome/browser/google_url_tracker.h index 04e6c81..e270397 100644 --- a/chrome/browser/google_url_tracker.h +++ b/chrome/browser/google_url_tracker.h @@ -3,13 +3,24 @@ // found in the LICENSE file. #include "chrome/browser/url_fetcher.h" +#include "chrome/common/notification_service.h" class PrefService; -// This object is responsible for updating the Google URL exactly once (when -// first constructed) and tracking the currently known value, which is also -// saved to a pref. -class GoogleURLTracker : public URLFetcher::Delegate { +// This object is responsible for updating the Google URL at most once per run, +// and tracking the currently known value, which is also saved to a pref. +// +// Most consumers should only call GoogleURL(), which is guaranteed to +// synchronously return a value at all times (even during startup or in unittest +// mode). Consumers who need to be notified when things change should listen to +// the notification service for NOTIFY_GOOGLE_URL_UPDATED, and call GoogleURL() +// again after receiving it, in order to get the updated value. +// +// To protect users' privacy and reduce server load, no updates will be +// performed (ever) unless at least one consumer registers interest by calling +// RequestServerCheck(). +class GoogleURLTracker : public URLFetcher::Delegate, + public NotificationObserver { public: // Only the main browser process loop should call this, when setting up // g_browser_process->google_url_tracker_. No code other than the @@ -19,15 +30,23 @@ class GoogleURLTracker : public URLFetcher::Delegate { // anyway). GoogleURLTracker(); - // This returns the current Google URL. If this is the first call to it in - // this session, it also kicks off a timer to fetch the correct Google URL; - // when that fetch completes, it will fire NOTIFY_GOOGLE_URL_UPDATED. + ~GoogleURLTracker(); + + // Returns the current Google URL. This will return a valid URL even in + // unittest mode. // // This is the only function most code should ever call. - // - // This will return a valid URL even in unittest mode. static GURL GoogleURL(); + // Requests that the tracker perform a server check to update the Google URL + // as necessary. This will happen at most once per run, not sooner than five + // seconds after startup (checks requested before that time will occur then; + // checks requested afterwards will occur immediately, if no other checks have + // been made during this run). + // + // In unittest mode, this function does nothing. + static void RequestServerCheck(); + static void RegisterPrefs(PrefService* prefs); private: @@ -38,8 +57,16 @@ class GoogleURLTracker : public URLFetcher::Delegate { // check succeeded (and thus whether |base_url| was actually updated). static bool CheckAndConvertToGoogleBaseURL(const GURL& url, GURL* base_url); - // Starts the fetch of the up-to-date Google URL. - void StartFetch(); + // Registers consumer interest in getting an updated URL from the server. + void SetNeedToFetch(); + + // Called when the five second startup sleep has finished. Runs any pending + // fetch. + void FinishSleep(); + + // Starts the fetch of the up-to-date Google URL if we actually want to fetch + // it and can currently do so. + void StartFetchIfDesirable(); // URLFetcher::Delegate virtual void OnURLFetchComplete(const URLFetcher *source, @@ -49,11 +76,27 @@ class GoogleURLTracker : public URLFetcher::Delegate { const ResponseCookies& cookies, const std::string& data); + // NotificationObserver + virtual void Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details); + static const char kDefaultGoogleHomepage[]; GURL google_url_; ScopedRunnableMethodFactory<GoogleURLTracker> fetcher_factory_; scoped_ptr<URLFetcher> fetcher_; + bool in_startup_sleep_; // True if we're in the five-second "no fetching" + // period that begins at browser start. + bool already_fetched_; // True if we've already fetched a URL once this run; + // we won't fetch again until after a restart. + bool need_to_fetch_; // True if a consumer actually wants us to fetch an + // updated URL. If this is never set, we won't + // bother to fetch anything. + bool request_context_available_; + // True when the profile has been loaded and the + // default request context created, so we can + // actually do the fetch with the right data. DISALLOW_EVIL_CONSTRUCTORS(GoogleURLTracker); }; diff --git a/chrome/browser/profile.cc b/chrome/browser/profile.cc index 3dd106f..28027c7 100644 --- a/chrome/browser/profile.cc +++ b/chrome/browser/profile.cc @@ -142,6 +142,9 @@ class ProfileImpl::RequestContext : public URLRequestContext, // profile - at least until we support multiple profiles. if (!default_request_context_) default_request_context_ = this; + NotificationService::current()->Notify( + NOTIFY_DEFAULT_REQUEST_CONTEXT_AVAILABLE, + NotificationService::AllSources(), NotificationService::NoDetails()); // Register for notifications about prefs. prefs_->AddPrefObserver(prefs::kAcceptLanguages, this); @@ -707,7 +710,8 @@ URLRequestContext* ProfileImpl::GetRequestContext() { file_util::AppendToPath(&cookie_path, chrome::kCookieFilename); std::wstring cache_path = GetPath(); file_util::AppendToPath(&cache_path, chrome::kCacheDirname); - request_context_ = new ProfileImpl::RequestContext(cookie_path, cache_path, GetPrefs()); + request_context_ = + new ProfileImpl::RequestContext(cookie_path, cache_path, GetPrefs()); request_context_->AddRef(); DCHECK(request_context_->cookie_store()); diff --git a/chrome/browser/template_url_model.cc b/chrome/browser/template_url_model.cc index 35c9262..8a2be29 100644 --- a/chrome/browser/template_url_model.cc +++ b/chrome/browser/template_url_model.cc @@ -10,6 +10,7 @@ #include "base/string_util.h" #include "chrome/app/locales/locale_settings.h" #include "chrome/browser/browser_process.h" +#include "chrome/browser/google_url_tracker.h" #include "chrome/browser/history/history.h" #include "chrome/browser/profile.h" #include "chrome/browser/rlz/rlz.h" @@ -64,15 +65,7 @@ TemplateURLModel::TemplateURLModel(Profile* profile) default_search_provider_(NULL), next_id_(1) { DCHECK(profile_); - NotificationService* ns = NotificationService::current(); - // TODO(sky): bug 1166191. The keywords should be moved into the history - // db, which will mean we no longer need this notification and the history - // backend can handle automatically adding the search terms as the user - // navigates. - ns->AddObserver(this, NOTIFY_HISTORY_URL_VISITED, - Source<Profile>(profile_->GetOriginalProfile())); - ns->AddObserver(this, NOTIFY_GOOGLE_URL_UPDATED, - NotificationService::AllSources()); + Init(NULL, 0); } TemplateURLModel::TemplateURLModel(const Initializer* initializers, @@ -94,17 +87,31 @@ TemplateURLModel::~TemplateURLModel() { STLDeleteElements(&template_urls_); + NotificationService* ns = NotificationService::current(); if (profile_) { - NotificationService* ns = NotificationService::current(); ns->RemoveObserver(this, NOTIFY_HISTORY_URL_VISITED, Source<Profile>(profile_->GetOriginalProfile())); - ns->RemoveObserver(this, NOTIFY_GOOGLE_URL_UPDATED, - NotificationService::AllSources()); } + ns->RemoveObserver(this, NOTIFY_GOOGLE_URL_UPDATED, + NotificationService::AllSources()); } void TemplateURLModel::Init(const Initializer* initializers, int num_initializers) { + // Register for notifications. + NotificationService* ns = NotificationService::current(); + if (profile_) { + // TODO(sky): bug 1166191. The keywords should be moved into the history + // db, which will mean we no longer need this notification and the history + // backend can handle automatically adding the search terms as the user + // navigates. + ns->AddObserver(this, NOTIFY_HISTORY_URL_VISITED, + Source<Profile>(profile_->GetOriginalProfile())); + } + ns->AddObserver(this, NOTIFY_GOOGLE_URL_UPDATED, + NotificationService::AllSources()); + + // Add specific initializers, if any. for (int i(0); i < num_initializers; ++i) { DCHECK(initializers[i].keyword); DCHECK(initializers[i].url); @@ -124,6 +131,13 @@ void TemplateURLModel::Init(const Initializer* initializers, template_url->SetURL(osd_url, 0, 0); Add(template_url); } + + // Request a server check for the correct Google URL if Google is the default + // search engine. + const TemplateURL* default_provider = GetDefaultSearchProvider(); + const TemplateURLRef* default_provider_ref = default_provider->url(); + if (default_provider_ref && default_provider_ref->HasGoogleBaseURLs()) + GoogleURLTracker::RequestServerCheck(); } // static @@ -355,7 +369,8 @@ void TemplateURLModel::RemoveAutoGeneratedBetween(Time created_after, Time created_before) { for (size_t i = 0; i < template_urls_.size();) { if (template_urls_[i]->date_created() >= created_after && - (created_before.is_null() || template_urls_[i]->date_created() < created_before) && + (created_before.is_null() || + template_urls_[i]->date_created() < created_before) && CanReplace(template_urls_[i])) { Remove(template_urls_[i]); } else { @@ -492,10 +507,12 @@ void TemplateURLModel::SetDefaultSearchProvider(const TemplateURL* url) { service_.get()->UpdateKeyword(*url); const TemplateURLRef* url_ref = url->url(); - if (url_ref && url_ref->HasGoogleBaseURLs()) + if (url_ref && url_ref->HasGoogleBaseURLs()) { + GoogleURLTracker::RequestServerCheck(); RLZTracker::RecordProductEvent(RLZTracker::CHROME, RLZTracker::CHROME_OMNIBOX, RLZTracker::SET_TO_GOOGLE); + } } SaveDefaultSearchProviderToPrefs(url); diff --git a/chrome/common/notification_types.h b/chrome/common/notification_types.h index d9e3b44..a57d978 100644 --- a/chrome/common/notification_types.h +++ b/chrome/common/notification_types.h @@ -432,6 +432,12 @@ enum NotificationType { // This is sent to a pref observer when a pref is changed. NOTIFY_PREF_CHANGED, + // Sent when a default request context has been created, so calling + // Profile::GetDefaultRequestContext() will not return NULL. This is sent on + // the thread where Profile::GetRequestContext() is first called, which should + // be the UI thread. + NOTIFY_DEFAULT_REQUEST_CONTEXT_AVAILABLE, + // Autocomplete -------------------------------------------------------------- // This is sent when an item of the Omnibox popup is selected. The source is |