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 400 matching lines @@
   }
   if (!prepopulate_id.empty() && !*is_managed) {
     int value;
     base::StringToInt(prepopulate_id, &value);
     (*default_provider_data)->prepopulate_id = value;
   }
   return true;
 }
 
 // static
-void TemplateURLService::SaveDefaultSearchProviderToPrefs(
-    const TemplateURL* t_url, PrefService* prefs) {
-  if (!prefs)
-    return;
-
-  bool enabled = false;
-  std::string search_url;
-  std::string suggest_url;
-  std::string instant_url;
-  std::string image_url;
-  std::string new_tab_url;
-  std::string search_url_post_params;
-  std::string suggest_url_post_params;
-  std::string instant_url_post_params;
-  std::string image_url_post_params;
-  std::string icon_url;
-  std::string encodings;
-  std::string short_name;
-  std::string keyword;
-  std::string id_string;
-  std::string prepopulate_id;
-  base::ListValue alternate_urls;
-  std::string search_terms_replacement_key;
-  if (t_url) {
-    DCHECK_EQ(TemplateURL::NORMAL, t_url->GetType());
-    enabled = true;
-    search_url = t_url->url();
-    suggest_url = t_url->suggestions_url();
-    instant_url = t_url->instant_url();
-    image_url = t_url->image_url();
-    new_tab_url = t_url->new_tab_url();
-    search_url_post_params = t_url->search_url_post_params();
-    suggest_url_post_params = t_url->suggestions_url_post_params();
-    instant_url_post_params = t_url->instant_url_post_params();
-    image_url_post_params = t_url->image_url_post_params();
-    GURL icon_gurl = t_url->favicon_url();
-    if (!icon_gurl.is_empty())
-      icon_url = icon_gurl.spec();
-    encodings = JoinString(t_url->input_encodings(), ';');
-    short_name = base::UTF16ToUTF8(t_url->short_name());
-    keyword = base::UTF16ToUTF8(t_url->keyword());
-    id_string = base::Int64ToString(t_url->id());
-    prepopulate_id = base::Int64ToString(t_url->prepopulate_id());
-    for (size_t i = 0; i < t_url->alternate_urls().size(); ++i)
-      alternate_urls.AppendString(t_url->alternate_urls()[i]);
-    search_terms_replacement_key = t_url->search_terms_replacement_key();
-  }
-  prefs->SetBoolean(prefs::kDefaultSearchProviderEnabled, enabled);
-  prefs->SetString(prefs::kDefaultSearchProviderSearchURL, search_url);
-  prefs->SetString(prefs::kDefaultSearchProviderSuggestURL, suggest_url);
-  prefs->SetString(prefs::kDefaultSearchProviderInstantURL, instant_url);
-  prefs->SetString(prefs::kDefaultSearchProviderImageURL, image_url);
-  prefs->SetString(prefs::kDefaultSearchProviderNewTabURL, new_tab_url);
-  prefs->SetString(prefs::kDefaultSearchProviderSearchURLPostParams,
-                   search_url_post_params);
-  prefs->SetString(prefs::kDefaultSearchProviderSuggestURLPostParams,
-                   suggest_url_post_params);
-  prefs->SetString(prefs::kDefaultSearchProviderInstantURLPostParams,
-                   instant_url_post_params);
-  prefs->SetString(prefs::kDefaultSearchProviderImageURLPostParams,
-                   image_url_post_params);
-  prefs->SetString(prefs::kDefaultSearchProviderIconURL, icon_url);
-  prefs->SetString(prefs::kDefaultSearchProviderEncodings, encodings);
-  prefs->SetString(prefs::kDefaultSearchProviderName, short_name);
-  prefs->SetString(prefs::kDefaultSearchProviderKeyword, keyword);
-  prefs->SetString(prefs::kDefaultSearchProviderID, id_string);
-  prefs->SetString(prefs::kDefaultSearchProviderPrepopulateID, prepopulate_id);
-  prefs->Set(prefs::kDefaultSearchProviderAlternateURLs, alternate_urls);
-  prefs->SetString(prefs::kDefaultSearchProviderSearchTermsReplacementKey,
-      search_terms_replacement_key);
-}
-
-// static
 base::string16 TemplateURLService::GenerateKeyword(const GURL& url) {

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

This apparent diff is just fallout from moving
SaveDefaultSearchProviderToPrefs() lower down within the file.

   DCHECK(url.is_valid());
   // Strip "www." off the front of the keyword; otherwise the keyword won't work
   // properly.  See http://code.google.com/p/chromium/issues/detail?id=6984 .
   // Special case: if the host was exactly "www." (not sure this can happen but
   // perhaps with some weird intranet and custom DNS server?), ensure we at
   // least don't return the empty string.
   base::string16 keyword(net::StripWWWFromHost(url));
   return keyword.empty() ? base::ASCIIToUTF16("www") : keyword;
 }
 
@@ skipping 56 matching lines @@
   // case the term replaces the URL it's unlikely another keyword would have the
   // same url.
   // TODO(jnd): Add additional parameters to get post data when the search URL
   // has post parameters.
   return GURL(search_ref.ReplaceSearchTermsUsingTermsData(
       TemplateURLRef::SearchTermsArgs(
           base::ASCIIToUTF16("blah.blah.blah.blah.blah")),
       search_terms_data, NULL));
 }
 
+void TemplateURLService::SaveDefaultSearchProviderToPrefs(
+    const TemplateURL* t_url,
+    PrefService* prefs) const {
+  if (!prefs || load_failed_)
+    return;
+
+  bool enabled = false;
+  std::string search_url;
+  std::string suggest_url;
+  std::string instant_url;
+  std::string image_url;
+  std::string new_tab_url;
+  std::string search_url_post_params;
+  std::string suggest_url_post_params;
+  std::string instant_url_post_params;
+  std::string image_url_post_params;
+  std::string icon_url;
+  std::string encodings;
+  std::string short_name;
+  std::string keyword;
+  std::string id_string;
+  std::string prepopulate_id;
+  base::ListValue alternate_urls;
+  std::string search_terms_replacement_key;
+  if (t_url) {
+    DCHECK_EQ(TemplateURL::NORMAL, t_url->GetType());
+    enabled = true;
+    search_url = t_url->url();
+    suggest_url = t_url->suggestions_url();
+    instant_url = t_url->instant_url();
+    image_url = t_url->image_url();
+    new_tab_url = t_url->new_tab_url();
+    search_url_post_params = t_url->search_url_post_params();
+    suggest_url_post_params = t_url->suggestions_url_post_params();
+    instant_url_post_params = t_url->instant_url_post_params();
+    image_url_post_params = t_url->image_url_post_params();
+    GURL icon_gurl = t_url->favicon_url();
+    if (!icon_gurl.is_empty())
+      icon_url = icon_gurl.spec();
+    encodings = JoinString(t_url->input_encodings(), ';');
+    short_name = base::UTF16ToUTF8(t_url->short_name());
+    keyword = base::UTF16ToUTF8(t_url->keyword());
+    id_string = base::Int64ToString(t_url->id());
+    prepopulate_id = base::Int64ToString(t_url->prepopulate_id());
+    for (size_t i = 0; i < t_url->alternate_urls().size(); ++i)
+      alternate_urls.AppendString(t_url->alternate_urls()[i]);
+    search_terms_replacement_key = t_url->search_terms_replacement_key();
+  }
+  prefs->SetBoolean(prefs::kDefaultSearchProviderEnabled, enabled);
+  prefs->SetString(prefs::kDefaultSearchProviderSearchURL, search_url);
+  prefs->SetString(prefs::kDefaultSearchProviderSuggestURL, suggest_url);
+  prefs->SetString(prefs::kDefaultSearchProviderInstantURL, instant_url);
+  prefs->SetString(prefs::kDefaultSearchProviderImageURL, image_url);
+  prefs->SetString(prefs::kDefaultSearchProviderNewTabURL, new_tab_url);
+  prefs->SetString(prefs::kDefaultSearchProviderSearchURLPostParams,
+                   search_url_post_params);
+  prefs->SetString(prefs::kDefaultSearchProviderSuggestURLPostParams,
+                   suggest_url_post_params);
+  prefs->SetString(prefs::kDefaultSearchProviderInstantURLPostParams,
+                   instant_url_post_params);
+  prefs->SetString(prefs::kDefaultSearchProviderImageURLPostParams,
+                   image_url_post_params);
+  prefs->SetString(prefs::kDefaultSearchProviderIconURL, icon_url);
+  prefs->SetString(prefs::kDefaultSearchProviderEncodings, encodings);
+  prefs->SetString(prefs::kDefaultSearchProviderName, short_name);
+  prefs->SetString(prefs::kDefaultSearchProviderKeyword, keyword);
+  prefs->SetString(prefs::kDefaultSearchProviderID, id_string);
+  prefs->SetString(prefs::kDefaultSearchProviderPrepopulateID, prepopulate_id);
+  prefs->Set(prefs::kDefaultSearchProviderAlternateURLs, alternate_urls);
+  prefs->SetString(prefs::kDefaultSearchProviderSearchTermsReplacementKey,
+      search_terms_replacement_key);
+}
+
 bool TemplateURLService::CanReplaceKeyword(
     const base::string16& keyword,
     const GURL& url,
     TemplateURL** template_url_to_replace) {
   DCHECK(!keyword.empty());  // This should only be called for non-empty
                              // keywords. If we need to support empty kewords
                              // the code needs to change slightly.
   TemplateURL* existing_url = GetTemplateURLForKeyword(keyword);
   if (template_url_to_replace)
     *template_url_to_replace = existing_url;
@@ skipping 42 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_) &&
-      initial_default_search_provider_.get() &&
+  return (!loaded_ && initial_default_search_provider_.get() &&

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

Leaving the "load_failed_" checks in these next three functions is actively
wrong, since it means that upon failed load, we'll continue to try to reference
|initial_default_search_provider_|, which we don't want to pay attention to
anymore.

That said, in a patch set I haven't yet uploaded, I also tweaked OnFailedLoad()
to ensure |initial_default_search_provider_| is reset in all cases.

       (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;
-  return ((!loaded_ || load_failed_) &&
-      initial_default_search_provider_.get() &&
+  return (!loaded_ && initial_default_search_provider_.get() &&
       (initial_default_search_provider_->sync_guid() == sync_guid)) ?
       initial_default_search_provider_.get() : NULL;
 }
 
 TemplateURL* TemplateURLService::GetTemplateURLForHost(
     const std::string& host) {
   if (loaded_) {
     TemplateURL* t_url = provider_map_->GetTemplateURLForHost(host);
     if (t_url)
       return t_url;
   }
-  return ((!loaded_ || load_failed_) &&
-      initial_default_search_provider_.get() &&
+  return (!loaded_ && initial_default_search_provider_.get() &&
       (GenerateSearchURL(initial_default_search_provider_.get()).host() ==
           host)) ? initial_default_search_provider_.get() : NULL;
 }
 
 void TemplateURLService::Add(TemplateURL* template_url) {
   WebDataService::KeywordBatchModeScoper keyword_scoper(service_.get());
   if (AddNoNotify(template_url, true))
     NotifyObservers();
 }
 
@@ skipping 134 matching lines @@
 void TemplateURLService::IncrementUsageCount(TemplateURL* url) {
   DCHECK(url);
   // Extension-controlled search engines are not persisted.
   if (url->GetType() == TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION)
     return;
   if (std::find(template_urls_.begin(), template_urls_.end(), url) ==
       template_urls_.end())
     return;
   ++url->data_.usage_count;
 
-  if (service_.get())
-    service_.get()->UpdateKeyword(url->data());
+  if (service_)
+    service_->UpdateKeyword(url->data());

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

I removed some unnecessary get()s here and below while I was touching the file. 
No functional change in these sorts of cases, I just noticed them and wanted to
clean them up.

 }
 
 void TemplateURLService::ResetTemplateURL(TemplateURL* url,
                                           const base::string16& title,
                                           const base::string16& keyword,
                                           const std::string& search_url) {
   if (!loaded_)
     return;
   DCHECK(!keyword.empty());
   DCHECK(!search_url.empty());
@@ skipping 26 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_)
-    return default_search_provider_;
-  // We're not loaded, rely on the default search provider stored in prefs.
-  return initial_default_search_provider_.get();
+  return loaded_ ?

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

Another case where examining |load_failed_| is actually wrong.

+      default_search_provider_ : initial_default_search_provider_.get();
 }
 
 bool TemplateURLService::IsSearchResultsPageFromDefaultSearchProvider(
     const GURL& url) {
   TemplateURL* default_provider = GetDefaultSearchProvider();
   return default_provider && default_provider->IsSearchURL(url);
 }
 
 bool TemplateURLService::IsExtensionControlledDefaultSearch() {
   const TemplateURL* default_provider = GetDefaultSearchProvider();
@@ skipping 90 matching lines @@
 }
 
 void TemplateURLService::RemoveObserver(TemplateURLServiceObserver* observer) {
   model_observers_.RemoveObserver(observer);
 }
 
 void TemplateURLService::Load() {
   if (loaded_ || load_handle_)
     return;
 
-  if (!service_.get()) {
+  if (!service_)
     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;
-    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 255 matching lines @@
     syncer::ModelType type,
     const syncer::SyncDataList& initial_sync_data,
     scoped_ptr<syncer::SyncChangeProcessor> sync_processor,
     scoped_ptr<syncer::SyncErrorFactory> sync_error_factory) {
   DCHECK(loaded_);
   DCHECK_EQ(type, syncer::SEARCH_ENGINES);
   DCHECK(!sync_processor_.get());
   DCHECK(sync_processor.get());
   DCHECK(sync_error_factory.get());
   syncer::SyncMergeResult merge_result(type);
+
+  // Disable sync if we failed to load.

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

Because the in-memory state won't match the on-disk database (and thus the
server-side state), I don't think we want to try to sync anything -- we could
wind up sending the server undesirable changes.

+  if (load_failed_) {
+    merge_result.set_error(syncer::SyncError(
+        FROM_HERE, syncer::SyncError::DATATYPE_ERROR,
+        "Local database load failed.", syncer::SEARCH_ENGINES));
+    return merge_result;
+  }
+
   sync_processor_ = sync_processor.Pass();
   sync_error_factory_ = sync_error_factory.Pass();
 
   // We just started syncing, so set our wait-for-default flag if we are
   // expecting a default from Sync.
   if (GetPrefs()) {
     std::string default_guid = GetPrefs()->GetString(
         prefs::kSyncedDefaultSearchProviderGUID);
     const TemplateURL* current_default = GetDefaultSearchProvider();
 
@@ skipping 485 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() {
+  load_failed_ = true;
+  service_ = NULL;
+  ChangeToLoadedState();
+  if (!GetDefaultSearchProvider()) {

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

Some tests set a new default search provider before reaching this point.  This
conditional causes us to respect those changes.

+    TemplateURL* default_search_provider =
+        initial_default_search_provider_.release();
+    if (default_search_provider) {
+      AddNoNotify(default_search_provider,
+                  default_search_provider->id() == kInvalidTemplateURLID);

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

We want AddNoNotify() to ensure the provided TemplateURL gets an ID, since
various pieces of the TemplateURL system rely on this, so we can't pass "false"
here.  However, |initial_default_search_provider_| may already have an ID if
it's being loaded from prefs on disk instead of initialized directly from the
prepopulate data during first run, so we can't pass "true" either.  Instead we
tell AddNoNotify() to give the URL an ID iff it's necessary.

+      SetDefaultSearchProviderNoNotify(default_search_provider);
+    }
+  }
+  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 80 matching lines @@
         RemoveNoNotify(existing_keyword_turl);
       } else {
         existing_turl->data_.SetKeyword(old_keyword);
         keyword_to_template_map_[old_keyword] = existing_turl;
       }
     }
   }
   if (!existing_turl->sync_guid().empty())
     guid_to_template_map_[existing_turl->sync_guid()] = existing_turl;
 
-  if (service_.get())
+  if (service_)
     service_->UpdateKeyword(existing_turl->data());
 
   // Inform sync of the update.
   ProcessTemplateURLChange(FROM_HERE,
                            existing_turl,
                            syncer::SyncChange::ACTION_UPDATE);
 
   if (default_search_provider_ == existing_turl) {
     bool success = SetDefaultSearchProviderNoNotify(existing_turl);
     DCHECK(success);
@@ skipping 268 matching lines @@
       UMA_HISTOGRAM_ENUMERATION("Search.DefaultSearchChangeOrigin",
                                 dsp_change_origin_, DSP_CHANGE_MAX);
       default_search_provider_ = url;
     }
   }
 
   if (url) {
     // Don't mark the url as edited, otherwise we won't be able to rev the
     // template urls we ship with.
     url->data_.show_in_default_list = true;
-    if (service_.get() && (url->GetType() == TemplateURL::NORMAL))
+    if (service_ && (url->GetType() == TemplateURL::NORMAL))
       service_->UpdateKeyword(url->data());
 
     if (url->url_ref().HasGoogleBaseURLs()) {
       GoogleURLTracker::RequestServerCheck(profile_, false);
 #if defined(ENABLE_RLZ)
       RLZTracker::RecordProductEvent(rlz_lib::CHROME,
                                      RLZTracker::CHROME_OMNIBOX,
                                      rlz_lib::SET_TO_GOOGLE);
 #endif
     }
@@ skipping 14 matching lines @@
     // needs to set a new default as well. If we update the default here, we're
     // likely to race with the update from the other client, resulting in
     // a possibly random default search provider.
     if (sync_processor_.get() && url && !url->sync_guid().empty() &&
         GetPrefs() && !processing_syncer_changes_) {
       GetPrefs()->SetString(prefs::kSyncedDefaultSearchProviderGUID,
                             url->sync_guid());
     }
   }
 
-  if (service_.get())
+  if (service_)
     service_->SetDefaultSearchProviderID(url ? url->id() : 0);
 
   // Inform sync the change to the show_in_default_list flag.
   if (url)
     ProcessTemplateURLChange(FROM_HERE,
                              url,
                              syncer::SyncChange::ACTION_UPDATE);
   return true;
 }
 
@@ skipping 31 matching lines @@
                        existing_keyword_turl->short_name(), new_keyword,
                        existing_keyword_turl->url());
     }
   }
   template_urls_.push_back(template_url);
   AddToMaps(template_url);
 
   if (newly_adding &&
       (template_url->GetType() !=
           TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION)) {
-    if (service_.get())
+    if (service_)
       service_->AddKeyword(template_url->data());
 
     // Inform sync of the addition. Note that this will assign a GUID to
     // template_url and add it to the guid_to_template_map_.
     ProcessTemplateURLChange(FROM_HERE,
                              template_url,
                              syncer::SyncChange::ACTION_ADD);
   }
 
   return true;
 }
 
 void TemplateURLService::RemoveNoNotify(TemplateURL* template_url) {
   DCHECK(template_url != default_search_provider_);
 
   TemplateURLVector::iterator i =
       std::find(template_urls_.begin(), template_urls_.end(), template_url);
   if (i == template_urls_.end())
     return;
 
   RemoveFromMaps(template_url);
 
   // Remove it from the vector containing all TemplateURLs.
   template_urls_.erase(i);
 
   if (template_url->GetType() != TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION) {
-    if (service_.get())
+    if (service_)
       service_->RemoveKeyword(template_url->id());
 
     // Inform sync of the deletion.
     ProcessTemplateURLChange(FROM_HERE,
                              template_url,
                              syncer::SyncChange::ACTION_DELETE);
 
     UMA_HISTOGRAM_ENUMERATION(kDeleteSyncedEngineHistogramName,
                               DELETE_ENGINE_USER_ACTION, DELETE_ENGINE_MAX);
   }
@@ skipping 49 matching lines @@
       }
 
       // The database loaded a managed |default_search_provider|, but it has
       // been updated in the prefs. Remove it from the database, and update the
       // |default_search_provider| pointer here.
       if (*default_search_provider &&
           (*default_search_provider)->id() == template_url->id())
         *default_search_provider = NULL;
 
       i = template_urls->erase(i);
-      if (service_.get())
+      if (service_)
         service_->RemoveKeyword(template_url->id());
       delete template_url;
     } else {
       ++i;
     }
   }
 }
 
 void TemplateURLService::ResetTemplateURLGUID(TemplateURL* url,
                                               const std::string& guid) {
@@ skipping 203 matching lines @@
     TemplateURLVector* template_urls) {
   DCHECK(template_urls);
   for (TemplateURLVector::iterator i = template_urls->begin();
        i != template_urls->end(); ++i) {
     TemplateURL* template_url = *i;
     DCHECK(template_url);
     if (template_url->sync_guid().empty() &&
         (template_url->GetType() !=
             TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION)) {
       template_url->data_.sync_guid = base::GenerateGUID();
-      if (service_.get())
+      if (service_)
         service_->UpdateKeyword(template_url->data());
     }
   }
 }
 
 void TemplateURLService::AddTemplateURLsAndSetupDefaultEngine(
     TemplateURLVector* template_urls,
     TemplateURL* default_search_provider) {
   DCHECK(template_urls);
   is_default_search_managed_ = false;
@@ skipping 158 matching lines @@
           new_dse = *i;
           break;
         }
       }
     }
   }
   if (!new_dse)
     new_dse = FindNewDefaultSearchProvider();
   SetDefaultSearchProviderNoNotify(new_dse);
 }