Fixes a bug where QuotaPolicyChannelIDStore would ignore some deletes.

net/extras/sqlite/sqlite_channel_id_store.cc

 // Copyright 2014 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 "net/extras/sqlite/sqlite_channel_id_store.h"
 
 #include <set>
 
 #include "base/basictypes.h"
 #include "base/bind.h"
@@ skipping 109 matching lines @@
 
    private:
     OperationType op_;
     DefaultChannelIDStore::ChannelID channel_id_;
   };
 
  private:
   // Batch a channel id operation (add or delete).
   void BatchOperation(PendingOperation::OperationType op,
                       const DefaultChannelIDStore::ChannelID& channel_id);
+  // Prunes the list of pending operations to remove any operations for an
+  // identifier in |server_identifiers|.
+  void PrunePendingOperationsForDeletes(
+      const std::list<std::string>& server_identifiers);
   // Commit our pending operations to the database.
   void Commit();
   // Close() executed on the background task runner.
   void InternalBackgroundClose();
 
   void BackgroundDeleteAllInList(
       const std::list<std::string>& server_identifiers);
 
   void DatabaseErrorCallback(int error, sql::Statement* stmt);
   void KillDatabase();
@@ skipping 330 matching lines @@
         FROM_HERE,
         base::Bind(&Backend::Commit, this),
         base::TimeDelta::FromMilliseconds(kCommitIntervalMs));
   } else if (num_pending == kCommitAfterBatchSize) {
     // We've reached a big enough batch, fire off a commit now.
     background_task_runner_->PostTask(FROM_HERE,
                                       base::Bind(&Backend::Commit, this));
   }
 }
 
+void SQLiteChannelIDStore::Backend::PrunePendingOperationsForDeletes(
+    const std::list<std::string>& server_identifiers) {
+  DCHECK(background_task_runner_->RunsTasksOnCurrentThread());
+  base::AutoLock locked(lock_);

[rohitrao (ping after 24h), 2015/04/06 18:42:22]

I did not see a way to avoid locking around this whole method.  The swap trick
from Commit() doesn't work, because another thread could add new entries while
I'm pruning.

(Although if anyone does actually call Add() at this point, all bets are off,
because that call wouldn't get pruned in any case.  Not sure how to deal with
that.  In this particular case, since we're calling DeleteAllInList() from the
destructor, it's unlikely that this will happen in real life.)

+
+  for (PendingOperationsList::iterator it = pending_.begin();
+       it != pending_.end();) {
+    bool remove =
+        std::find(server_identifiers.begin(), server_identifiers.end(),
+                  (*it)->channel_id().server_identifier()) !=
+        server_identifiers.end();
+
+    if (remove) {
+      scoped_ptr<PendingOperation> po(*it);
+      it = pending_.erase(it);
+      --num_pending_;
+    } else {
+      ++it;
+    }
+  }
+}
+
 void SQLiteChannelIDStore::Backend::Commit() {
   DCHECK(background_task_runner_->RunsTasksOnCurrentThread());
 
   PendingOperationsList ops;
   {
     base::AutoLock locked(lock_);
     pending_.swap(ops);
     num_pending_ = 0;
   }
 
@@ skipping 70 matching lines @@
   db_.reset();
 }
 
 void SQLiteChannelIDStore::Backend::BackgroundDeleteAllInList(
     const std::list<std::string>& server_identifiers) {
   DCHECK(background_task_runner_->RunsTasksOnCurrentThread());
 
   if (!db_.get())
     return;
 
+  // Force a commit of any pending writes before issuing deletes.

[mef, 2015/04/06 20:17:44]

I think this comment is stale.

[rohitrao (ping after 24h), 2015/04/07 14:21:37]

Done.

+  PrunePendingOperationsForDeletes(server_identifiers);
+
   sql::Statement del_smt(db_->GetCachedStatement(
       SQL_FROM_HERE, "DELETE FROM origin_bound_certs WHERE origin=?"));
   if (!del_smt.is_valid()) {
     LOG(WARNING) << "Unable to delete channel ids.";
     return;
   }
 
   sql::Transaction transaction(db_.get());
   if (!transaction.Begin()) {
     LOG(WARNING) << "Unable to delete channel ids.";
@@ skipping 47 matching lines @@
   backend_->SetForceKeepSessionState();
 }
 
 SQLiteChannelIDStore::~SQLiteChannelIDStore() {
   backend_->Close();
   // We release our reference to the Backend, though it will probably still have
   // a reference if the background task runner has not run Close() yet.
 }
 
 }  // namespace net