Mechanical modernization of some storage/*/database code

* Use range-based for loops
* Use auto (sparingly) to replace ugly iterator naming
* Trust return value optimization to handle vectors

Change-Id: I14ef9fd8d9904b943b0e2d64188d3be5f63ef23d
Reviewed-on: https://chromium-review.googlesource.com/930163
Commit-Queue: Joshua Bell <jsbell@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543865}

storage/browser/database/database_quota_client.cc

@@ -43,10 +43,8 @@ void GetOriginsOnDBThread(DatabaseTracker* db_tracker,
                           std::set<url::Origin>* origins_ptr) {
   std::vector<std::string> origin_identifiers;
   if (db_tracker->GetAllOriginIdentifiers(&origin_identifiers)) {
-    for (std::vector<std::string>::const_iterator iter =
-         origin_identifiers.begin();
-         iter != origin_identifiers.end(); ++iter) {
-      origins_ptr->insert(storage::GetOriginFromIdentifier(*iter));
+    for (const auto& identifier : origin_identifiers) {
+      origins_ptr->insert(storage::GetOriginFromIdentifier(identifier));
     }
   }
 }
@@ -56,12 +54,10 @@ void GetOriginsForHostOnDBThread(DatabaseTracker* db_tracker,
                                  const std::string& host) {
   std::vector<std::string> origin_identifiers;
   if (db_tracker->GetAllOriginIdentifiers(&origin_identifiers)) {
-    for (std::vector<std::string>::const_iterator iter =
-         origin_identifiers.begin();
-         iter != origin_identifiers.end(); ++iter) {
-      if (host ==
-          net::GetHostOrSpecFromURL(storage::GetOriginURLFromIdentifier(*iter)))
-        origins_ptr->insert(storage::GetOriginFromIdentifier(*iter));
+    for (const auto& identifier : origin_identifiers) {
+      url::Origin origin = storage::GetOriginFromIdentifier(identifier);
+      if (host == net::GetHostOrSpecFromURL(origin.GetURL()))
+        origins_ptr->insert(origin);
     }
   }
 }

storage/browser/database/database_quota_client_unittest.cc

@@ -58,14 +58,14 @@ class MockDatabaseTracker : public DatabaseTracker {
 
   bool GetAllOriginIdentifiers(
       std::vector<std::string>* origins_identifiers) override {
-    for (auto& iter : mock_origin_infos_)
-      origins_identifiers->push_back(iter.second.GetOriginIdentifier());
+    for (const auto& origin_info : mock_origin_infos_)
+      origins_identifiers->push_back(origin_info.second.GetOriginIdentifier());
     return true;
   }
 
   bool GetAllOriginsInfo(std::vector<OriginInfo>* origins_info) override {
-    for (auto& iter : mock_origin_infos_)
-      origins_info->push_back(OriginInfo(iter.second));
+    for (const auto& origin_info : mock_origin_infos_)
+      origins_info->push_back(OriginInfo(origin_info.second));
     return true;
   }
 

storage/browser/database/database_tracker.cc

@@ -54,10 +54,8 @@ OriginInfo::~OriginInfo() = default;
 
 void OriginInfo::GetAllDatabaseNames(
     std::vector<base::string16>* databases) const {
-  for (DatabaseInfoMap::const_iterator it = database_info_.begin();
-       it != database_info_.end(); it++) {
-    databases->push_back(it->first);
-  }
+  for (const auto& pair : database_info_)
+    databases->push_back(pair.first);
 }
 
 int64_t OriginInfo::GetDatabaseSize(const base::string16& database_name) const {
@@ -190,23 +188,16 @@ void DatabaseTracker::CloseDatabases(const DatabaseConnections& connections) {
   }
 
   // When being closed by this route, there's a chance that
-  // the tracker missed some DatabseModified calls. This method is used
+  // the tracker missed some DatabaseModified calls. This method is used
   // when a renderer crashes to cleanup its open resources.
   // We need to examine what we have in connections for the
   // size of each open databases and notify any differences between the
   // actual file sizes now.
-  std::vector<std::pair<std::string, base::string16> > open_dbs;
-  connections.ListConnections(&open_dbs);
-  for (std::vector<std::pair<std::string, base::string16> >::iterator it =
-           open_dbs.begin(); it != open_dbs.end(); ++it)
-    UpdateOpenDatabaseSizeAndNotify(it->first, it->second);
-
-  std::vector<std::pair<std::string, base::string16> > closed_dbs;
-  database_connections_.RemoveConnections(connections, &closed_dbs);
-  for (std::vector<std::pair<std::string, base::string16> >::iterator it =
-           closed_dbs.begin(); it != closed_dbs.end(); ++it) {
-    DeleteDatabaseIfNeeded(it->first, it->second);
-  }
+  for (auto& pair : connections.ListConnections())
+    UpdateOpenDatabaseSizeAndNotify(pair.first, pair.second);
+
+  for (auto& pair : database_connections_.RemoveConnections(connections))
+    DeleteDatabaseIfNeeded(pair.first, pair.second);
 }
 
 void DatabaseTracker::DeleteDatabaseIfNeeded(
@@ -336,9 +327,8 @@ bool DatabaseTracker::GetAllOriginsInfo(
   if (!GetAllOriginIdentifiers(&origins))
     return false;
 
-  for (std::vector<std::string>::const_iterator it = origins.begin();
-       it != origins.end(); it++) {
-    CachedOriginInfo* origin_info = GetCachedOriginInfo(*it);
+  for (const auto& origin : origins) {
+    CachedOriginInfo* origin_info = GetCachedOriginInfo(origin);
     if (!origin_info) {
       // Restore 'origins_info' to its initial state.
       origins_info->clear();
@@ -567,18 +557,17 @@ DatabaseTracker::CachedOriginInfo* DatabaseTracker::MaybeGetCachedOriginInfo(
 
     CachedOriginInfo& origin_info = origins_info_map_[origin_identifier];
     origin_info.SetOriginIdentifier(origin_identifier);
-    for (std::vector<DatabaseDetails>::const_iterator it = details.begin();
-         it != details.end(); it++) {
+    for (const auto& db : details) {
       int64_t db_file_size;
-      if (database_connections_.IsDatabaseOpened(
-              origin_identifier, it->database_name)) {
+      if (database_connections_.IsDatabaseOpened(origin_identifier,
+                                                 db.database_name)) {
         db_file_size = database_connections_.GetOpenDatabaseSize(
-            origin_identifier, it->database_name);
+            origin_identifier, db.database_name);
       } else {
-        db_file_size = GetDBFileSize(origin_identifier, it->database_name);
+        db_file_size = GetDBFileSize(origin_identifier, db.database_name);
       }
-      origin_info.SetDatabaseSize(it->database_name, db_file_size);
-      origin_info.SetDatabaseDescription(it->database_name, it->description);
+      origin_info.SetDatabaseSize(db.database_name, db_file_size);
+      origin_info.SetDatabaseDescription(db.database_name, db.description);
     }
   }
 
@@ -656,11 +645,9 @@ void DatabaseTracker::ScheduleDatabasesForDeletion(
 
   if (!callback.is_null())
     deletion_callbacks_.push_back(std::make_pair(callback, databases));
-  for (DatabaseSet::const_iterator origin = databases.begin();
-       origin != databases.end(); ++origin) {
-    for (std::set<base::string16>::const_iterator db = origin->second.begin();
-         db != origin->second.end(); ++db)
-      ScheduleDatabaseForDeletion(origin->first, *db);
+  for (const auto& origin_dbs : databases) {
+    for (const base::string16& db : origin_dbs.second)
+      ScheduleDatabaseForDeletion(origin_dbs.first, db);
   }
 }
 
@@ -676,7 +663,7 @@ int DatabaseTracker::DeleteDatabase(const std::string& origin_identifier,
     if (!callback.is_null()) {
       DatabaseSet set;
       set[origin_identifier].insert(database_name);
-      deletion_callbacks_.push_back(std::make_pair(callback, set));
+      deletion_callbacks_.emplace_back(callback, set);
     }
     ScheduleDatabaseForDeletion(origin_identifier, database_name);
     return net::ERR_IO_PENDING;
@@ -698,32 +685,29 @@ int DatabaseTracker::DeleteDataModifiedSince(
   if (!databases_table_->GetAllOriginIdentifiers(&origins_identifiers))
     return net::ERR_FAILED;
   int rv = net::OK;
-  for (std::vector<std::string>::const_iterator origin =
-           origins_identifiers.begin();
-       origin != origins_identifiers.end(); ++origin) {
+  for (const auto& origin : origins_identifiers) {
     if (special_storage_policy_.get() &&
         special_storage_policy_->IsStorageProtected(
-            storage::GetOriginURLFromIdentifier(*origin))) {
+            storage::GetOriginURLFromIdentifier(origin))) {
       continue;
     }
 
     std::vector<DatabaseDetails> details;
-    if (!databases_table_->GetAllDatabaseDetailsForOriginIdentifier(*origin,
+    if (!databases_table_->GetAllDatabaseDetailsForOriginIdentifier(origin,
                                                                     &details))
       rv = net::ERR_FAILED;
-    for (std::vector<DatabaseDetails>::const_iterator db = details.begin();
-         db != details.end(); ++db) {
-      base::FilePath db_file = GetFullDBFilePath(*origin, db->database_name);
+    for (const DatabaseDetails& db : details) {
+      base::FilePath db_file = GetFullDBFilePath(origin, db.database_name);
       base::File::Info file_info;
       base::GetFileInfo(db_file, &file_info);
       if (file_info.last_modified < cutoff)
         continue;
 
       // Check if the database is opened by any renderer.
-      if (database_connections_.IsDatabaseOpened(*origin, db->database_name))
-        to_be_deleted[*origin].insert(db->database_name);
+      if (database_connections_.IsDatabaseOpened(origin, db.database_name))
+        to_be_deleted[origin].insert(db.database_name);
       else
-        DeleteClosedDatabase(*origin, db->database_name);
+        DeleteClosedDatabase(origin, db.database_name);
     }
   }
 
@@ -749,13 +733,12 @@ int DatabaseTracker::DeleteDataForOrigin(
   if (!databases_table_->
           GetAllDatabaseDetailsForOriginIdentifier(origin, &details))
     return net::ERR_FAILED;
-  for (std::vector<DatabaseDetails>::const_iterator db = details.begin();
-       db != details.end(); ++db) {
+  for (const auto& db : details) {
     // Check if the database is opened by any renderer.
-    if (database_connections_.IsDatabaseOpened(origin, db->database_name))
-      to_be_deleted[origin].insert(db->database_name);
+    if (database_connections_.IsDatabaseOpened(origin, db.database_name))
+      to_be_deleted[origin].insert(db.database_name);
     else
-      DeleteClosedDatabase(origin, db->database_name);
+      DeleteClosedDatabase(origin, db.database_name);
   }
 
   if (!to_be_deleted.empty()) {
@@ -817,10 +800,8 @@ void DatabaseTracker::DeleteIncognitoDBDirectory() {
   DCHECK(task_runner_->RunsTasksInCurrentSequence());
   is_initialized_ = false;
 
-  for (FileHandlesMap::iterator it = incognito_file_handles_.begin();
-       it != incognito_file_handles_.end(); it++) {
-    delete it->second;
-  }
+  for (auto& pair : incognito_file_handles_)
+    delete pair.second;
 
   base::FilePath incognito_db_dir =
       profile_path_.Append(kIncognitoDatabaseDirectoryName);
@@ -844,28 +825,24 @@ void DatabaseTracker::ClearSessionOnlyOrigins() {
   std::vector<std::string> origin_identifiers;
   GetAllOriginIdentifiers(&origin_identifiers);
 
-  for (std::vector<std::string>::iterator origin =
-           origin_identifiers.begin();
-       origin != origin_identifiers.end(); ++origin) {
-    GURL origin_url = storage::GetOriginURLFromIdentifier(*origin);
+  for (const auto& origin : origin_identifiers) {
+    GURL origin_url = storage::GetOriginURLFromIdentifier(origin);
     if (!special_storage_policy_->IsStorageSessionOnly(origin_url))
       continue;
     if (special_storage_policy_->IsStorageProtected(origin_url))
       continue;
     storage::OriginInfo origin_info;
     std::vector<base::string16> databases;
-    GetOriginInfo(*origin, &origin_info);
+    GetOriginInfo(origin, &origin_info);
     origin_info.GetAllDatabaseNames(&databases);
 
-    for (std::vector<base::string16>::iterator database = databases.begin();
-         database != databases.end(); ++database) {
-      base::File file(GetFullDBFilePath(*origin, *database),
-                      base::File::FLAG_OPEN_ALWAYS |
-                          base::File::FLAG_SHARE_DELETE |
-                          base::File::FLAG_DELETE_ON_CLOSE |
-                          base::File::FLAG_READ);
+    for (const auto& database : databases) {
+      base::File file(
+          GetFullDBFilePath(origin, database),
+          base::File::FLAG_OPEN_ALWAYS | base::File::FLAG_SHARE_DELETE |
+              base::File::FLAG_DELETE_ON_CLOSE | base::File::FLAG_READ);
     }
-    DeleteOrigin(*origin, true);
+    DeleteOrigin(origin, true);
   }
 }
 

storage/browser/database/database_util.cc

@@ -17,11 +17,9 @@ namespace {
 
 bool IsSafeSuffix(const base::string16& suffix) {
   base::char16 prev_c = 0;
-  for (base::string16::const_iterator it = suffix.begin();
-      it < suffix.end(); ++it) {
-    base::char16 c = *it;
-    if (!(base::IsAsciiAlpha(c) || base::IsAsciiDigit(c) ||
-          c == '-' || c == '.' || c == '_')) {
+  for (const base::char16 c : suffix) {
+    if (!(base::IsAsciiAlpha(c) || base::IsAsciiDigit(c) || c == '-' ||
+          c == '.' || c == '_')) {
       return false;
     }
     if (c == '.' && prev_c == '.')

storage/common/database/database_connections.cc

@@ -58,21 +58,21 @@ void DatabaseConnections::RemoveAllConnections() {
   connections_.clear();
 }
 
-void DatabaseConnections::RemoveConnections(
-    const DatabaseConnections& connections,
-    std::vector<std::pair<std::string, base::string16> >* closed_dbs) {
-  for (OriginConnections::const_iterator origin_it =
-           connections.connections_.begin();
-       origin_it != connections.connections_.end();
-       origin_it++) {
-    const DBConnections& db_connections = origin_it->second;
-    for (DBConnections::const_iterator db_it = db_connections.begin();
-         db_it != db_connections.end(); db_it++) {
-      if (RemoveConnectionsHelper(origin_it->first, db_it->first,
-                                  db_it->second.first))
-        closed_dbs->push_back(std::make_pair(origin_it->first, db_it->first));
+std::vector<std::pair<std::string, base::string16>>
+DatabaseConnections::RemoveConnections(const DatabaseConnections& connections) {
+  std::vector<std::pair<std::string, base::string16>> closed_dbs;
+  for (const auto& origin_connections_pair : connections.connections_) {
+    const DBConnections& db_connections = origin_connections_pair.second;
+    for (const auto& count_size_pair : db_connections) {
+      if (RemoveConnectionsHelper(origin_connections_pair.first,
+                                  count_size_pair.first,
+                                  count_size_pair.second.first)) {
+        closed_dbs.emplace_back(origin_connections_pair.first,
+                                count_size_pair.first);
+      }
     }
   }
+  return closed_dbs;
 }
 
 int64_t DatabaseConnections::GetOpenDatabaseSize(
@@ -90,18 +90,15 @@ void DatabaseConnections::SetOpenDatabaseSize(
   connections_[origin_identifier][database_name].second = size;
 }
 
-void DatabaseConnections::ListConnections(
-    std::vector<std::pair<std::string, base::string16> > *list) const {
-  for (OriginConnections::const_iterator origin_it =
-           connections_.begin();
-       origin_it != connections_.end();
-       origin_it++) {
-    const DBConnections& db_connections = origin_it->second;
-    for (DBConnections::const_iterator db_it = db_connections.begin();
-         db_it != db_connections.end(); db_it++) {
-      list->push_back(std::make_pair(origin_it->first, db_it->first));
-    }
+std::vector<std::pair<std::string, base::string16>>
+DatabaseConnections::ListConnections() const {
+  std::vector<std::pair<std::string, base::string16>> list;
+  for (const auto& origin_connections_pair : connections_) {
+    const DBConnections& db_connections = origin_connections_pair.second;
+    for (const auto& count_size_pair : db_connections)
+      list.emplace_back(origin_connections_pair.first, count_size_pair.first);
   }
+  return list;
 }
 
 bool DatabaseConnections::RemoveConnectionsHelper(

storage/common/database/database_connections.h

@@ -42,9 +42,8 @@ class STORAGE_COMMON_EXPORT DatabaseConnections {
                         const base::string16& database_name);
 
   void RemoveAllConnections();
-  void RemoveConnections(
-      const DatabaseConnections& connections,
-      std::vector<std::pair<std::string, base::string16> >* closed_dbs);
+  std::vector<std::pair<std::string, base::string16>> RemoveConnections(
+      const DatabaseConnections& connections);
 
   // Database sizes can be kept only if IsDatabaseOpened returns true.
   int64_t GetOpenDatabaseSize(const std::string& origin_identifier,
@@ -54,8 +53,7 @@ class STORAGE_COMMON_EXPORT DatabaseConnections {
                            int64_t size);
 
   // Returns a list of the connections, <origin_id, name>.
-  void ListConnections(
-      std::vector<std::pair<std::string, base::string16> > *list) const;
+  std::vector<std::pair<std::string, base::string16>> ListConnections() const;
 
  private:
   // Mapping from name to <openCount, size>

storage/common/database/database_connections_unittest.cc

@@ -70,8 +70,8 @@ TEST(DatabaseConnectionsTest, DatabaseConnectionsTest) {
   another.AddConnection(kOriginId, kName);
   another.AddConnection(kOriginId, kName2);
 
-  std::vector<std::pair<std::string, base::string16> > closed_dbs;
-  connections.RemoveConnections(another, &closed_dbs);
+  std::vector<std::pair<std::string, base::string16>> closed_dbs =
+      connections.RemoveConnections(another);
   EXPECT_EQ(1u, closed_dbs.size());
   EXPECT_EQ(kOriginId, closed_dbs[0].first);
   EXPECT_EQ(kName2, closed_dbs[0].second);