Handle TemplateURLService load failure better

chrome/browser/search_engines/template_url_service.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/search_engines/template_url_service.h"
 
 #include <algorithm>
 #include <utility>
 
 #include "base/auto_reset.h"
@@ skipping 612 matching lines @@
       matches->push_back(i->second);
   }
 }
 
 TemplateURL* TemplateURLService::GetTemplateURLForKeyword(
     const base::string16& keyword) {
   KeywordToTemplateMap::const_iterator elem(
       keyword_to_template_map_.find(keyword));
   if (elem != keyword_to_template_map_.end())
     return elem->second;
   return ((!loaded_ || load_failed_) &&

[erikwright (departed), 2014/05/03 01:01:54]

Now that we copied initial_default_search_provider_ over, isn't it inappropriate
to reference it if load_failed_?

(Also futile, since initial... is NULL).

Here and the next few methods.

       initial_default_search_provider_.get() &&
       (initial_default_search_provider_->keyword() == keyword)) ?
       initial_default_search_provider_.get() : NULL;
 }
 
 TemplateURL* TemplateURLService::GetTemplateURLForGUID(
     const std::string& sync_guid) {
   GUIDToTemplateMap::const_iterator elem(guid_to_template_map_.find(sync_guid));
   if (elem != guid_to_template_map_.end())
     return elem->second;
@@ skipping 207 matching lines @@
   SetDefaultSearchProvider(url);
   if (default_search_manager_) {
     if (url)
       default_search_manager_->SetUserSelectedDefaultSearchEngine(url->data());
     else
       default_search_manager_->ClearUserSelectedDefaultSearchEngine();
   }
 }
 
 TemplateURL* TemplateURLService::GetDefaultSearchProvider() {
   if (loaded_ && !load_failed_)

[erikwright (departed), 2014/05/03 01:01:54]

Ditto here.

In fact, load_failed_ basically seems useless now since there is also no
accessor for it...

     return default_search_provider_;
   // We're not loaded, rely on the default search provider stored in prefs.
   return initial_default_search_provider_.get();
 }
 
 bool TemplateURLService::IsSearchResultsPageFromDefaultSearchProvider(
     const GURL& url) {
   TemplateURL* default_provider = GetDefaultSearchProvider();
   return default_provider && default_provider->IsSearchURL(url);
 }
@@ skipping 97 matching lines @@
 }
 
 void TemplateURLService::Load() {
   if (loaded_ || load_handle_)
     return;
 
   if (!service_.get()) {
     service_ = WebDataService::FromBrowserContext(profile_);
   }
 
-  if (service_.get()) {
+  if (service_)
     load_handle_ = service_->GetKeywords(this);
-  } else {
-    ChangeToLoadedState();
-    on_loaded_callbacks_.Notify();
-  }
+  else
+    OnFailedLoad();
 }
 
 scoped_ptr<TemplateURLService::Subscription>
     TemplateURLService::RegisterOnLoadedCallback(
         const base::Closure& callback) {
   return loaded_ ?
       scoped_ptr<TemplateURLService::Subscription>() :
       on_loaded_callbacks_.Add(callback);
 }
 
 void TemplateURLService::OnWebDataServiceRequestDone(
     WebDataService::Handle h,
     const WDTypedResult* result) {
   // Reset the load_handle so that we don't try and cancel the load in
   // the destructor.
   load_handle_ = 0;
 
   if (!result) {
     // Results are null if the database went away or (most likely) wasn't
     // loaded.
     load_failed_ = true;

[erikwright (departed), 2014/05/03 01:01:54]

Shouldn't this also be set in the '!service_' case in TUS::Load()?

[Peter Kasting, 2014/05/05 18:46:24]

Yes.  Moved to OnFailedLoad().

-    ChangeToLoadedState();
-    on_loaded_callbacks_.Notify();
+    OnFailedLoad();
     return;
   }
 
   // initial_default_search_provider_ is only needed before we've finished
   // loading. Now that we've loaded we can nuke it.
   initial_default_search_provider_.reset();
 
   TemplateURLVector template_urls;
   TemplateURL* default_search_provider = NULL;
   int new_resource_keyword_version = 0;
@@ skipping 760 matching lines @@
 }
 
 void TemplateURLService::ChangeToLoadedState() {
   DCHECK(!loaded_);
 
   UIThreadSearchTermsData search_terms_data(profile_);
   provider_map_->Init(template_urls_, search_terms_data);
   loaded_ = true;
 }
 
+void TemplateURLService::OnFailedLoad() {
+  TemplateURL* default_search_provider =
+      initial_default_search_provider_.release();
+  AddNoNotify(default_search_provider, true);
+  SetDefaultSearchProviderNoNotify(default_search_provider);
+  ChangeToLoadedState();
+  on_loaded_callbacks_.Notify();
+}
+
 void TemplateURLService::ClearDefaultProviderFromPrefs() {
   // We overwrite user preferences. If the default search engine is managed,
   // there is no effect.
   SaveDefaultSearchProviderToPrefs(NULL, GetPrefs());
   // Default value for kDefaultSearchProviderEnabled is true.
   PrefService* prefs = GetPrefs();
   if (prefs)
     prefs->SetBoolean(prefs::kDefaultSearchProviderEnabled, true);
 }
 
@@ skipping 956 matching lines @@
           new_dse = *i;
           break;
         }
       }
     }
   }
   if (!new_dse)
     new_dse = FindNewDefaultSearchProvider();
   SetDefaultSearchProviderNoNotify(new_dse);
 }