Merge search engines sync data type with Preferences. Sync the default search provider. Add some ...

chrome/browser/search_engines/template_url_service.cc

 // Copyright (c) 2011 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 "base/auto_reset.h"
 #include "base/command_line.h"
 #include "base/environment.h"
 #include "base/i18n/case_conversion.h"
@@ skipping 95 matching lines @@
     : profile_(profile),
       loaded_(false),
       load_failed_(false),
       load_handle_(0),
       default_search_provider_(NULL),
       is_default_search_managed_(false),
       next_id_(1),
       time_provider_(&base::Time::Now),
       models_associated_(false),
       processing_syncer_changes_(false),
-      sync_processor_(NULL) {
+      sync_processor_(NULL),
+      pending_synced_default_search_(false) {
   DCHECK(profile_);
   Init(NULL, 0);
 }
 
 TemplateURLService::TemplateURLService(const Initializer* initializers,
                                        const int count)
     : profile_(NULL),
       loaded_(false),
       load_failed_(false),
       load_handle_(0),
       service_(NULL),
       default_search_provider_(NULL),
       is_default_search_managed_(false),
       next_id_(1),
       time_provider_(&base::Time::Now),
       models_associated_(false),
       processing_syncer_changes_(false),
-      sync_processor_(NULL) {
+      sync_processor_(NULL),
+      pending_synced_default_search_(false) {
   Init(initializers, count);
 }
 
 TemplateURLService::~TemplateURLService() {
   if (load_handle_) {
     DCHECK(service_.get());
     service_->CancelRequest(load_handle_);
   }
 
   STLDeleteElements(&template_urls_);
@@ skipping 305 matching lines @@
 
   // We're not loaded, rely on the default search provider stored in prefs.
   return initial_default_search_provider_.get();
 }
 
 const TemplateURL* TemplateURLService::FindNewDefaultSearchProvider() {
   // See if the prepoluated default still exists.
   scoped_ptr<TemplateURL> prepopulated_default(
       TemplateURLPrepopulateData::GetPrepopulatedDefaultSearch(GetPrefs()));
   for (TemplateURLVector::iterator i = template_urls_.begin();
-      i != template_urls_.end(); ) {
+       i != template_urls_.end(); ++i) {
     if ((*i)->prepopulate_id() == prepopulated_default->prepopulate_id())
       return *i;
   }
   // If not, use the first of the templates.
   return template_urls_.empty() ? NULL : template_urls_[0];
 }
 
 void TemplateURLService::AddObserver(TemplateURLServiceObserver* observer) {
   model_observers_.AddObserver(observer);
 }
@@ skipping 99 matching lines @@
       if (managed_default) {
         managed_default->set_created_by_policy(true);
         managed_default->set_id(0);
         AddNoNotify(managed_default);
       }
       default_search_provider = managed_default;
     }
     // Note that this saves the default search provider to prefs.
     SetDefaultSearchProviderNoNotify(default_search_provider);
   } else {
-    // If we had a managed default, replace it with the first provider of
-    // the list.
-    if (database_specified_a_default &&
-        NULL == default_search_provider &&
-        !template_urls.empty())
+    // If we had a managed default, replace it with the synced default if
+    // applicable, or the first provider of the list.
+    const TemplateURL* synced_default = GetPendingSyncedDefaultSearchProvder();
+    if (synced_default) {
+      default_search_provider = synced_default;
+      pending_synced_default_search_ = false;
+    } else if (database_specified_a_default &&
+               NULL == default_search_provider &&
+               !template_urls.empty()) {
       default_search_provider = template_urls[0];
+    }
 
     // If the default search provider existed previously, then just
     // set the member variable. Otherwise, we'll set it using the method
     // to ensure that it is saved properly after its id is set.
     if (default_search_provider && default_search_provider->id() != 0) {
       default_search_provider_ = default_search_provider;
       default_search_provider = NULL;
     }
     SetTemplateURLs(template_urls);
 
@@ skipping 46 matching lines @@
     if (!loaded())
       visits_to_add_.push_back(*visit_details.ptr());
     else
       UpdateKeywordSearchTermsForURL(*visit_details.ptr());
   } else if (type == chrome::NOTIFICATION_GOOGLE_URL_UPDATED) {
     if (loaded_)
       GoogleBaseURLChanged();
   } else if (type == chrome::NOTIFICATION_PREF_CHANGED) {
     const std::string* pref_name = content::Details<std::string>(details).ptr();
     if (!pref_name || default_search_prefs_->IsObserved(*pref_name)) {
+      // Listen for changes to the default search from Sync. If it is
+      // specifically the synced default search provider GUID that changed, we
+      // have to set it (or wait for it).
+      PrefService* prefs = GetPrefs();
+      if (pref_name && *pref_name == prefs::kSyncedDefaultSearchProviderGUID &&
+          prefs) {
+        const TemplateURL* new_default_search = GetTemplateURLForGUID(
+            prefs->GetString(prefs::kSyncedDefaultSearchProviderGUID));

[jeanluc1, 2011/11/08 19:49:41]

Can GetString return NULL?  What's the behavior of GetTemplateURLForGUID then?

[jeanluc1, 2011/11/08 19:49:41]

Would GetPendingSyncedDefaultSearchProvder work here?

[SteveT, 2011/11/08 21:40:04]

PrefService::GetString can return an empty string (if the pref is not
registered, which should DCHECK first), which, if fed to GetTemplateURLForGUID,
returns NULL. This causes the code below to believe that there is no TURL with
that GUID, and therefore set pending_... to true and wait for the TURL to
arrive, which is what we want.

[SteveT, 2011/11/08 21:40:04]

It would not as it only retrieves the TURL if we are waiting on it. If we are
not waiting on it, but it's actually already synced in, we would miss it
(GetPendingSyncedDefaultSearchProvider would return NULL).

+        if (new_default_search && !is_default_search_managed_) {
+          if (new_default_search != GetDefaultSearchProvider())
+            SetDefaultSearchProvider(new_default_search);
+        } else {
+          // If it's not there, or if default search is currently managed, set a
+          // flag to indicate that we waiting on the search engine entry to come
+          // in through Sync.
+          pending_synced_default_search_ = true;

[jeanluc1, 2011/11/08 19:49:41]

What if we get a second Observe call, this time where we could call
SetDefaultSearchProvider.  Does it set pending_synced_default_search to false?

[SteveT, 2011/11/08 21:40:04]

No it doesn't. We should set pending_synced_default_search ourselves. Done.

+        }
+      }
+
       // A preference related to default search engine has changed.
       // Update the model if needed.
       UpdateDefaultSearch();
     }
   } else {
     NOTREACHED();
   }
 }
 
 SyncDataList TemplateURLService::GetAllSyncData(
@@ skipping 37 matching lines @@
     if (!turl.get()) {
       NOTREACHED() << "Failed to read search engine.";
       continue;
     }
 
     const TemplateURL* existing_turl = GetTemplateURLForGUID(turl->sync_guid());
     const TemplateURL* existing_keyword_turl =
         GetTemplateURLForKeyword(turl->keyword());
 
     if (iter->change_type() == SyncChange::ACTION_DELETE && existing_turl) {
+      bool delete_default = (existing_turl == GetDefaultSearchProvider());
+      if (delete_default && is_default_search_managed_) {
+        // We are not allowed to delete managed default search providers.
+        continue;

[jeanluc1, 2011/11/08 19:49:41]

Can we avoid the continue?  There are so many other things in the loop that I'm
afraid future mainteners won't expect there's a continue somewhere in the loop.

[SteveT, 2011/11/08 21:40:04]

Actually, we can be a bit more defensive and transform that continue into a
NOTREACHED, since we really should not be allowing the system to delete managed
defaults.

+      }
+      if (delete_default)
+        default_search_provider_ = NULL;
+
       Remove(existing_turl);
+
+      if (delete_default)
+        SetDefaultSearchProvider(FindNewDefaultSearchProvider());
     } else if (iter->change_type() == SyncChange::ACTION_ADD &&
                !existing_turl) {
+      std::string guid = turl->sync_guid();
       if (existing_keyword_turl)
         ResolveSyncKeywordConflict(turl.get(), &new_changes);
       // Force the local ID to 0 so we can add it.
       turl->set_id(0);
       Add(turl.release());
+
+      // Possibly set the newly added |turl| as the default search provider.
+      CheckForSyncedDefaultSearchProvider(guid);
     } else if (iter->change_type() == SyncChange::ACTION_UPDATE &&
                existing_turl) {
-      if (existing_keyword_turl)
+      // Possibly resolve a keyword conflict if they have the same keywords but
+      // are not the same entry.
+      if (existing_keyword_turl && existing_keyword_turl != existing_turl)
         ResolveSyncKeywordConflict(turl.get(), &new_changes);
       ResetTemplateURL(existing_turl, turl->short_name(), turl->keyword(),
           turl->url() ? turl->url()->url() : std::string());
     } else {
       // Something really unexpected happened. Either we received an
       // ACTION_INVALID, or Sync is in a crazy state:
       //  . Trying to DELETE or UPDATE a non-existent search engine.
       //  . Trying to ADD a search engine that already exists.
       NOTREACHED() << "Unexpected sync change state.";
       error = SyncError(FROM_HERE, "ProcessSyncChanges failed on ChangeType " +
@@ skipping 14 matching lines @@
 
 SyncError TemplateURLService::MergeDataAndStartSyncing(
     syncable::ModelType type,
     const SyncDataList& initial_sync_data,
     SyncChangeProcessor* sync_processor) {
   DCHECK(loaded());
   DCHECK_EQ(type, syncable::SEARCH_ENGINES);
   DCHECK(!sync_processor_);
   sync_processor_ = sync_processor;
 
+  // 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);

[jeanluc1, 2011/11/08 19:49:41]

Would GetPendingSyncedDefaultSearchProvder work here?

[SteveT, 2011/11/08 21:40:04]

Unfortunately not. At this point, we can't fully trust the value of
pending_synced_default_search_ yet (in fact, it'll be false every time on
startup/first sync), which makes GetPendingSyncedDefaultSearchProvider return
NULL every time. We just want the default Sync GUID here, and not the actual
synced default search provider (which might not be synced in yet).

+    const TemplateURL* current_default = GetDefaultSearchProvider();
+
+    if (!default_guid.empty() &&
+        (!current_default || current_default->sync_guid() == default_guid))

[Nicolas Zea, 2011/11/08 19:57:27]

Should that last condition be current_default->sync_guid() != default_guid? If
the actual default is already the same as the sync default aren't we fine?

[SteveT, 2011/11/08 21:40:04]

Yes, that was a slip. I'll add a small test case to ensure that if the expected
synced default is already set, nothing gets tripped up. Thanks!

+      pending_synced_default_search_ = true;
+  }
+
   // We do a lot of calls to Add/Remove/ResetTemplateURL here, so ensure we
   // don't step on our own toes.
   AutoReset<bool> processing_changes(&processing_syncer_changes_, true);
 
   SyncChangeList new_changes;
 
   // Build maps of our sync GUIDs to SyncData.
   SyncDataMap local_data_map = CreateGUIDToSyncDataMap(
       GetAllSyncData(syncable::SEARCH_ENGINES));
   SyncDataMap sync_data_map = CreateGUIDToSyncDataMap(initial_sync_data);
@@ skipping 39 matching lines @@
       if (dupe_turl) {
         // Merge duplicates and remove the processed local TURL from the map.
         TemplateURL* modifiable_dupe_turl =
             const_cast<TemplateURL*>(dupe_turl);
         std::string old_guid = dupe_turl->sync_guid();
         MergeSyncAndLocalURLDuplicates(sync_turl.release(),
                                        modifiable_dupe_turl,
                                        &new_changes);
         local_data_map.erase(old_guid);
       } else {
+        std::string guid = sync_turl->sync_guid();
         // Keyword conflict is possible in this case. Resolve it first before
         // adding the new TemplateURL. Note that we don't remove the local TURL
         // from local_data_map in this case as it may still need to be pushed to
         // the cloud.
         ResolveSyncKeywordConflict(sync_turl.get(), &new_changes);
         // Force the local ID to 0 so we can add it.
         sync_turl->set_id(0);
         Add(sync_turl.release());
+
+        // Possibly set the newly added |turl| as the default search provider.
+        CheckForSyncedDefaultSearchProvider(guid);
       }
     }
   }  // for
 
   // The remaining SyncData in local_data_map should be everything that needs to
   // be pushed as ADDs to sync.
   for (SyncDataMap::const_iterator iter = local_data_map.begin();
       iter != local_data_map.end(); ++iter) {
     new_changes.push_back(SyncChange(SyncChange::ACTION_ADD, iter->second));
   }
@@ skipping 112 matching lines @@
                                                      const GURL& url,
                                                      const string16& term) {
   HistoryService* history = profile_  ?
       profile_->GetHistoryService(Profile::EXPLICIT_ACCESS) : NULL;
   if (!history)
     return;
   history->SetKeywordSearchTermsForURL(url, t_url->id(), term);
 }
 
 void TemplateURLService::Init(const Initializer* initializers,
-                            int num_initializers) {
+                              int num_initializers) {
   // Register for notifications.
   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.
     registrar_.Add(this, chrome::NOTIFICATION_HISTORY_URL_VISITED,
                    content::Source<Profile>(profile_->GetOriginalProfile()));
     PrefService* prefs = GetPrefs();
     default_search_prefs_.reset(
@@ skipping 515 matching lines @@
     DCHECK(is_default_search_managed_ && !new_is_default_managed);
     is_default_search_managed_ = new_is_default_managed;
     // If we had a default, delete the previous default if created by policy
     // and set a likely default.
     if (NULL != default_search_provider_ &&
         default_search_provider_->created_by_policy()) {
       const TemplateURL* old_default = default_search_provider_;
       default_search_provider_ = NULL;
       RemoveNoNotify(old_default);
     }
-    SetDefaultSearchProviderNoNotify(FindNewDefaultSearchProvider());
+
+    // The likely default should be from Sync if we were waiting on Sync.
+    // Otherwise, it should be FindNewDefaultSearchProvider.
+    const TemplateURL* synced_default = GetPendingSyncedDefaultSearchProvder();
+    if (synced_default)
+      pending_synced_default_search_ = false;
+    SetDefaultSearchProviderNoNotify(synced_default ? synced_default :
+        FindNewDefaultSearchProvider());
   }
   NotifyObservers();
 }
 
 void TemplateURLService::SetDefaultSearchProviderNoNotify(
     const TemplateURL* url) {
   DCHECK(!url || find(template_urls_.begin(), template_urls_.end(), url) !=
          template_urls_.end());
   default_search_provider_ = url;
 
@@ skipping 11 matching lines @@
 #if defined(OS_WIN) && defined(GOOGLE_CHROME_BUILD)
       // Needs to be evaluated. See http://crbug.com/62328.
       base::ThreadRestrictions::ScopedAllowIO allow_io;
       RLZTracker::RecordProductEvent(rlz_lib::CHROME,
                                      rlz_lib::CHROME_OMNIBOX,
                                      rlz_lib::SET_TO_GOOGLE);
 #endif
     }
   }
 
-  if (!is_default_search_managed_)
+  if (!is_default_search_managed_) {
     SaveDefaultSearchProviderToPrefs(url);
 
+    // If we are syncing, we want to set the synced pref that will notify other
+    // instances to change their default to this new search provider.
+    if (sync_processor_ && !url->sync_guid().empty() && GetPrefs()) {
+      GetPrefs()->SetString(prefs::kSyncedDefaultSearchProviderGUID,
+                            url->sync_guid());
+    }
+  }
+
   if (service_.get())
     service_->SetDefaultSearchProvider(url);
 
   // Inform sync the change to the show_in_default_list flag.
   if (url)
     ProcessTemplateURLChange(url, SyncChange::ACTION_UPDATE);
 }
 
 void TemplateURLService::AddNoNotify(TemplateURL* template_url) {
   DCHECK(template_url);
@@ skipping 138 matching lines @@
 }
 
 bool TemplateURLService::ResolveSyncKeywordConflict(
     TemplateURL* sync_turl,
     SyncChangeList* change_list) {
   DCHECK(sync_turl);
   DCHECK(change_list);
 
   const TemplateURL* existing_turl =
       GetTemplateURLForKeyword(sync_turl->keyword());
-  if (!existing_turl)
+  // If there is no conflict, or it's just conflicting with itself, return.
+  if (!existing_turl || existing_turl->sync_guid() == sync_turl->sync_guid())
     return false;
 
   if (existing_turl->last_modified() > sync_turl->last_modified() ||
       existing_turl->created_by_policy()) {
     string16 new_keyword = UniquifyKeyword(*sync_turl);
     DCHECK(!GetTemplateURLForKeyword(new_keyword));
     sync_turl->set_keyword(new_keyword);
     // If we update the cloud TURL, we need to push an update back to sync
     // informing it that something has changed.
     SyncData sync_data = CreateSyncDataFromTemplateURL(*sync_turl);
@@ skipping 42 matching lines @@
   } else {
     // Change the local TURL's GUID to the server's GUID and push an update to
     // Sync. This ensures that the rest of local_url's fields are sync'd up to
     // the server, and the next time local_url is synced, it is recognized by
     // having the same GUID.
     ResetTemplateURLGUID(local_turl, sync_turl->sync_guid());
     SyncData sync_data = CreateSyncDataFromTemplateURL(*local_turl);
     change_list->push_back(SyncChange(SyncChange::ACTION_UPDATE, sync_data));
   }
 }
+
+void TemplateURLService::CheckForSyncedDefaultSearchProvider(
+    const std::string& guid) {
+  // If we're not syncing or if default search is managed by policy, ignore.
+  if (!sync_processor_ || is_default_search_managed_)
+    return;
+
+  PrefService* prefs = GetPrefs();
+  if (prefs && pending_synced_default_search_ &&
+      prefs->GetString(prefs::kSyncedDefaultSearchProviderGUID) == guid) {
+    // Make sure this actually exists. We should not be calling this unless we
+    // really just added this TemplateURL.
+    const TemplateURL* turl_from_sync = GetTemplateURLForGUID(guid);
+    DCHECK(turl_from_sync);
+    SetDefaultSearchProvider(turl_from_sync);
+    pending_synced_default_search_ = false;
+  }
+}
+
+const TemplateURL* TemplateURLService::GetPendingSyncedDefaultSearchProvder() {

[jeanluc1, 2011/11/08 19:49:41]

Provder -> Provider

[SteveT, 2011/11/08 21:40:04]

Done.

+  PrefService* prefs = GetPrefs();
+  if (!prefs || !pending_synced_default_search_)
+    return NULL;
+
+  // Could be NULL if no such thing exists.
+  return GetTemplateURLForGUID(
+      prefs->GetString(prefs::kSyncedDefaultSearchProviderGUID));
+}