Fixes bug in importer where we could set the default search provider...

chrome/browser/importer/importer.cc

 // Copyright (c) 2006-2008 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/importer/importer.h"
 
 #include <map>
 
 #include "base/file_util.h"
 #include "base/gfx/image_operations.h"
@@ skipping 121 matching lines @@
 }
 
 void ProfileWriter::AddFavicons(
     const std::vector<history::ImportedFavIconUsage>& favicons) {
   profile_->GetHistoryService(Profile::EXPLICIT_ACCESS)->
       SetImportedFavicons(favicons);
 }
 
 typedef std::map<std::string, const TemplateURL*> HostPathMap;
 
+// Returns the key for the map built by BuildHostPathMap. If url_string is not
+// a valid URL, an empty string is returned, otherwise host+path is returned.
+static std::string HostPathKeyForURL(const std::wstring& url_string) {
+  GURL url(url_string);
+  return url.is_valid() ? url.host() + url.path() : std::string();
+}
+
 // Builds the key to use in HostPathMap for the specified TemplateURL. Returns
 // an empty string if a host+path can't be generated for the TemplateURL.
-// If an empty string is returned, it should not be added to HostPathMap.
-static std::string BuildHostPathKey(const TemplateURL* t_url) {
-  if (t_url->url() && t_url->url()->SupportsReplacement()) {
-    GURL search_url(t_url->url()->ReplaceSearchTerms(
-        *t_url, L"random string", TemplateURLRef::NO_SUGGESTIONS_AVAILABLE,
-        std::wstring()));
-    if (search_url.is_valid())
-      return search_url.host() + search_url.path();
+// If an empty string is returned, the TemplateURL should not be added to
+// HostPathMap.
+//
+// If |try_url_if_invalid| is true, and |t_url| isn't valid, a string is built
+// from the raw TemplateURL string. Use a value of true for |try_url_if_invalid|
+// when checking imported URLs as the imported URL may not be valid yet may
+// match the host+path of one of the default URLs. This is used to catch the
+// case of IE using an invalid OSDD URL for Live Search, yet the host+path
+// matches our prepopulate data. IE's URL for Live Search is something like
+// 'http://...{Language}...'. As {Language} is not a valid OSDD parameter value
+// the TemplateURL is invalid.

[Peter Kasting, 2008/09/25 23:44:19]

Thanks for this comment, it helps.

It might be even clearer if you moved it to the caller location and changed it
to something like: "Set the default engine based on even invalid URLs, if they
match a valid URL we already know.  For example, IE's Live Search URL contains
'{Language}', which is not a valid OSDD parameter and thus makes the TemplateURL
invalid, but its host+path portion still matches our Live Search prepopulate
data entry."

Up to you.

+static std::string BuildHostPathKey(const TemplateURL* t_url,
+                                    bool try_url_if_invalid) {
+  if (t_url->url()) {
+    if (try_url_if_invalid && !t_url->url()->IsValid())
+      return HostPathKeyForURL(t_url->url()->url());
+
+    if (t_url->url()->SupportsReplacement()) {
+      return HostPathKeyForURL(
+          t_url->url()->ReplaceSearchTerms(
+              *t_url, L"random string",
+              TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, std::wstring()));
+    }
   }
   return std::string();
 }
 
 // Builds a set that contains an entry of the host+path for each TemplateURL in
 // the TemplateURLModel that has a valid search url.
 static void BuildHostPathMap(const TemplateURLModel& model,
                              HostPathMap* host_path_map) {
   std::vector<const TemplateURL*> template_urls = model.GetTemplateURLs();
   for (size_t i = 0; i < template_urls.size(); ++i) {
-    const std::string host_path = BuildHostPathKey(template_urls[i]);
+    const std::string host_path = BuildHostPathKey(template_urls[i], false);
     if (!host_path.empty()) {
       const TemplateURL* existing_turl = (*host_path_map)[host_path];
       if (!existing_turl ||
           (template_urls[i]->show_in_default_list() &&
            !existing_turl->show_in_default_list())) {
         // If there are multiple TemplateURLs with the same host+path, favor
         // those shown in the default list.  If there are multiple potential
         // defaults, favor the first one, which should be the more commonly used
         // one.
         (*host_path_map)[host_path] = template_urls[i];
@@ skipping 28 matching lines @@
       delete t_url;
       continue;
     }
 
     // For search engines if there is already a keyword with the same
     // host+path, we don't import it. This is done to avoid both duplicate
     // search providers (such as two Googles, or two Yahoos) as well as making
     // sure the search engines we provide aren't replaced by those from the
     // imported browser.
     if (unique_on_host_and_path &&
-        host_path_map.find(BuildHostPathKey(t_url)) != host_path_map.end()) {
+        host_path_map.find(
+            BuildHostPathKey(t_url, true)) != host_path_map.end()) {
       if (default_keyword) {
         const TemplateURL* turl_with_host_path =
-            host_path_map[BuildHostPathKey(t_url)];
+            host_path_map[BuildHostPathKey(t_url, true)];
         if (turl_with_host_path)
           model->SetDefaultSearchProvider(turl_with_host_path);
         else
           NOTREACHED();  // BuildHostPathMap should only insert non-null values.
       }
       delete t_url;
       continue;
     }
-    model->Add(t_url);
-    if (default_keyword)
-      model->SetDefaultSearchProvider(t_url);
+    if (t_url->url() && t_url->url()->IsValid()) {
+      model->Add(t_url);
+      if (default_keyword && t_url->url() && t_url->url()->SupportsReplacement())
+        model->SetDefaultSearchProvider(t_url);
+    } else {
+      // Don't add invalid TemplateURLs to the model.
+      delete t_url;
+    }
   }
 }
 
 void ProfileWriter::ShowBookmarkBar() {
   DCHECK(profile_);
 
   PrefService* prefs = profile_->GetPrefs();
   // Check whether the bookmark bar is shown in current pref.
   if (!prefs->GetBoolean(prefs::kShowBookmarkBar)) {
     // Set the pref and notify the notification service.
@@ skipping 296 matching lines @@
   if (!source_path.empty()) {
     ProfileInfo* firefox = new ProfileInfo();
     firefox->description = l10n_util::GetString(IDS_IMPORT_FROM_FIREFOX);
     firefox->browser_type = firefox_type;
     firefox->source_path = source_path;
     firefox->app_path = GetFirefoxInstallPath();
     source_profiles_.push_back(firefox);
   }
 }