FileSystem: Prevent iterator from being deleted after database.

storage/browser/fileapi/sandbox_directory_database.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 "storage/browser/fileapi/sandbox_directory_database.h"
 
 #include <math.h>
 #include <stddef.h>
 #include <stdint.h>
 #include <algorithm>
@@ skipping 826 matching lines @@
     UMA_HISTOGRAM_ENUMERATION(kInitStatusHistogramLabel,
                               INIT_STATUS_IO_ERROR, INIT_STATUS_MAX);
   } else {
     UMA_HISTOGRAM_ENUMERATION(kInitStatusHistogramLabel,
                               INIT_STATUS_UNKNOWN_ERROR, INIT_STATUS_MAX);
   }
 }
 
 bool SandboxDirectoryDatabase::StoreDefaultValues() {
   // Verify that this is a totally new database, and initialize it.
-  scoped_ptr<leveldb::Iterator> iter(db_->NewIterator(leveldb::ReadOptions()));
-  iter->SeekToFirst();
-  if (iter->Valid()) {  // DB was not empty--we shouldn't have been called.
-    LOG(ERROR) << "File system origin database is corrupt!";
-    return false;
+  {
+    scoped_ptr<leveldb::Iterator> iter(

[nhiroki, 2016/01/27 00:48:00]

Can you add a comment about why we need to wrap these lines with {}?

[cmumford, 2016/01/27 16:33:56]

Done.

+        db_->NewIterator(leveldb::ReadOptions()));
+    iter->SeekToFirst();
+    if (iter->Valid()) {  // DB was not empty--we shouldn't have been called.
+      LOG(ERROR) << "File system origin database is corrupt!";
+      return false;
+    }
   }
   // This is always the first write into the database.  If we ever add a
   // version number, it should go in this transaction too.
   FileInfo root;
   root.parent_id = 0;
   root.modification_time = base::Time::Now();
   leveldb::WriteBatch batch;
   if (!AddFileInfoHelper(root, 0, &batch))
     return false;
   batch.Put(LastFileIdKey(), base::Int64ToString(0));
   batch.Put(LastIntegerKey(), base::Int64ToString(-1));
   leveldb::Status status = db_->Write(leveldb::WriteOptions(), &batch);
   if (!status.ok()) {
     HandleError(FROM_HERE, status);

[nhiroki, 2016/01/27 00:48:00]

Can you add a comment in the header file like this?

// Close the database. Before this, all iterators associated with the database
must be deleted.
void HandleError(const tracked_objects::Location& from_here,
                 const leveldb::Status& status);

[cmumford, 2016/01/27 16:33:56]

Done.

     return false;
   }
   return true;
 }
 
 bool SandboxDirectoryDatabase::GetLastFileId(FileId* file_id) {
   if (!Init(REPAIR_ON_CORRUPTION))
     return false;
   DCHECK(file_id);
   std::string id_string;
@@ skipping 67 matching lines @@
 
 void SandboxDirectoryDatabase::HandleError(
     const tracked_objects::Location& from_here,
     const leveldb::Status& status) {
   LOG(ERROR) << "SandboxDirectoryDatabase failed at: "
              << from_here.ToString() << " with error: " << status.ToString();
   db_.reset();
 }
 
 }  // namespace storage