Fix for non-sticky defaults bug. Added a bunch of tests.

chrome/browser/webdata/web_intents_table.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/webdata/web_intents_table.h"
 #include <string>
 
 #include "base/i18n/case_conversion.h"
 #include "base/logging.h"
 #include "base/string_util.h"
@@ skipping 72 matching lines @@
                       " UNIQUE (action, scheme, type, url_pattern))")) {
       return false;
     }
     if (!db_->Execute("CREATE INDEX IF NOT EXISTS web_intents_default_index"
                       " ON web_intents_defaults (action)"))
       return false;
 
     if (!db_->Execute("CREATE INDEX IF NOT EXISTS web_intents_default_index"
                       " ON web_intents_defaults (scheme)"))
       return false;
+  } else {
+    // For a while, 'scheme' was set to null when setting a web_intents_defaults
+    // row. Due to a longstanding SQLite bug, a NULL entry in a unique key field
+    // is always considered distinct from any other NULL entry in that field,
+    // rendering the uniqueness obsolete. Get rid of any such entries.

[Scott Hess - ex-Googler, 2012/09/14 19:37:47]

I think this mis-represents the problem, one NULL is not the same as another
NULL in case of UNIQUE for the same reason that x = NULL is not true if x is
NULL (which is why you say "x IS NULL" when making that test).

[Greg Billock, 2012/09/14 21:32:33]

I see what you're saying, and that's what I'm trying to communicate here: if the
values are null, then you get duplicates where you thought you shouldn't. I'll
try to reword it.

On 2012/09/14 19:37:47, shess wrote:

[Greg Billock, 2012/09/17 18:07:21]

OK, rephrased. Are you OK with this approach, Scott? I am accounting for the gap
elsewhere, so I could just remove this.

On 2012/09/14 21:32:33, Greg Billock wrote:

+    if (!db_->Execute("DELETE FROM web_intents_defaults WHERE scheme IS NULL"))
+      return false;
   }
 
+
   return true;
 }
 
 // TODO(jhawkins): Figure out Sync story.
 bool WebIntentsTable::IsSyncable() {
   return false;
 }
 
 // Updates the table by way of renaming the old tables, rerunning
 // the Init method, then selecting old values into the new tables.
@@ skipping 188 matching lines @@
   }
 
   return s.Succeeded();
 
 }
 
 bool WebIntentsTable::SetDefaultService(
     const DefaultWebIntentService& default_service) {
   sql::Statement s(db_->GetUniqueStatement(
       "INSERT OR REPLACE INTO web_intents_defaults "
-      "(action, type, url_pattern, user_date, suppression, service_url) "
-      "VALUES (?, ?, ?, ?, ?, ?)"));
+      "(action, type, url_pattern, user_date, suppression,"
+      " service_url, scheme) "
+      "VALUES (?, ?, ?, ?, ?, ?, ?)"));
   s.BindString16(0, default_service.action);
   s.BindString16(1, default_service.type);
   s.BindString(2, default_service.url_pattern.GetAsString());
   s.BindInt(3, default_service.user_date);
   s.BindInt64(4, default_service.suppression);
   s.BindString(5, default_service.service_url);
+  s.BindString16(6, default_service.scheme);
 
   return s.Run();
 }
 
 bool WebIntentsTable::RemoveDefaultService(
     const DefaultWebIntentService& default_service) {
   sql::Statement s(db_->GetUniqueStatement(
       "DELETE FROM web_intents_defaults "
       "WHERE action = ? AND type = ? AND url_pattern = ?"));
   s.BindString16(0, default_service.action);
   s.BindString16(1, default_service.type);
   s.BindString(2, default_service.url_pattern.GetAsString());
 
   return s.Run();
 }
 
 bool WebIntentsTable::RemoveServiceDefaults(const GURL& service_url) {
   sql::Statement s(db_->GetUniqueStatement(
       "DELETE FROM web_intents_defaults WHERE service_url = ?"));
   s.BindString(0, service_url.spec());
 
   return s.Run();
 }