Recreate the WebData database on a catastrophic SQL error

components/webdata/common/web_database_service.cc

 // Copyright 2013 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 "components/webdata/common/web_database_service.h"
 
 #include <stddef.h>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 119 matching lines @@
 void WebDatabaseService::RegisterDBErrorCallback(
     const DBLoadErrorCallback& callback) {
   error_callbacks_.push_back(callback);
 }
 
 void WebDatabaseService::OnDatabaseLoadDone(sql::InitStatus status,
                                             const std::string& diagnostics) {
   if (status == sql::INIT_OK) {
     db_loaded_ = true;
 
-    for (const auto& loaded_callback : loaded_callbacks_) {
+    while (!loaded_callbacks_.empty()) {
+      auto loaded_callback = loaded_callbacks_.back();

[Daniel Erat, 2016/08/09 21:05:41]

same comment as below

[afakhry, 2016/08/09 21:36:17]

Done.

+      loaded_callbacks_.pop_back();
       if (!loaded_callback.is_null())
         loaded_callback.Run();
     }
-
-    loaded_callbacks_.clear();
   } else {
     // Notify that the database load failed.
-    for (const auto& error_callback : error_callbacks_) {
+    while (!error_callbacks_.empty()) {
+      // The profile error callback is a message box that runs in a nested run
+      // loop. While it's being displayed, other OnDatabaseLoadDone() will run
+      // (posted from WebDatabaseBackend::Delegate::DBLoaded()). We need to make
+      // sure that after the callback running the message box returns, it checks
+      // |error_callbacks_| before it accesses it.
+      auto error_callback = error_callbacks_.back();

[Daniel Erat, 2016/08/09 21:05:41]

the c++ rules around auto and references seem arcane and terrify me. is this
safe or a use-after-free? my initial concern is that since back() usually
returns a reference, the 'auto' type here may be a reference as well, in which
case it may no longer be valid after the pop_back() call on the next line.

in any case, i think our rules for 'auto' say that it can only be used when the
variable's type is obvious, which isn't the case here (since error_callbacks_ is
declared in the header), so please switch this to use an explicit non-reference
callback type.

[afakhry, 2016/08/09 21:36:17]

Yes, absolutely. Thanks!
Done.

+      error_callbacks_.pop_back();
       if (!error_callback.is_null())
         error_callback.Run(status, diagnostics);
     }
-
-    error_callbacks_.clear();
   }
 }