Plumb crashpad dumper for pseudo-USS types

If a model (or SyncableService) reports an error, let's plumb it through
to base::debug::DumpWithoutCrashing() via ReportUnrecoverableError(),
which does it only for canary&dev channels, with random sampling. This
achieves feature parity with the directory-based setup and may allow
future investigations if we see datatypes with high error rates.

Bug: 870624
Change-Id: Idb3d00b178167a8b7b9eabb560858197e03aecb2
Reviewed-on: https://chromium-review.googlesource.com/c/1264198Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597035}

chrome/browser/supervised_user/supervised_user_sync_model_type_controller.cc

@@ -12,13 +12,15 @@
 SupervisedUserSyncModelTypeController::SupervisedUserSyncModelTypeController(
     syncer::ModelType type,
     const Profile* profile,
+    const base::RepeatingClosure& dump_stack,
     syncer::SyncClient* sync_client)
     : SyncableServiceBasedModelTypeController(
           type,
           sync_client->GetModelTypeStoreService()->GetStoreFactory(),
           base::BindOnce(&syncer::SyncClient::GetSyncableServiceForType,
                          base::Unretained(sync_client),
-                         type)),
+                         type),
+          dump_stack),
       profile_(profile) {
   DCHECK(type == syncer::SUPERVISED_USER_SETTINGS ||
          type == syncer::SUPERVISED_USER_WHITELISTS);

chrome/browser/supervised_user/supervised_user_sync_model_type_controller.h

@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_SUPERVISED_USER_SUPERVISED_USER_SYNC_MODEL_TYPE_CONTROLLER_H_
 #define CHROME_BROWSER_SUPERVISED_USER_SUPERVISED_USER_SYNC_MODEL_TYPE_CONTROLLER_H_
 
+#include "base/callback_forward.h"
 #include "base/macros.h"
 #include "components/sync/driver/syncable_service_based_model_type_controller.h"
 
@@ -20,9 +21,11 @@ class SupervisedUserSyncModelTypeController
     : public syncer::SyncableServiceBasedModelTypeController {
  public:
   // |sync_client| and |profile| must not be null and must outlive this object.
-  SupervisedUserSyncModelTypeController(syncer::ModelType type,
-                                        const Profile* profile,
-                                        syncer::SyncClient* sync_client);
+  SupervisedUserSyncModelTypeController(
+      syncer::ModelType type,
+      const Profile* profile,
+      const base::RepeatingClosure& dump_stack,
+      syncer::SyncClient* sync_client);
   ~SupervisedUserSyncModelTypeController() override;
 
   // DataTypeController override.

chrome/browser/sync/chrome_sync_client.cc

@@ -288,25 +288,24 @@ ChromeSyncClient::CreateDataTypeControllers(
       component_factory_->CreateCommonDataTypeControllers(
           disabled_types, local_device_info_provider);
 
-  base::RepeatingClosure error_callback = base::BindRepeating(
+  const base::RepeatingClosure dump_stack = base::BindRepeating(
       &syncer::ReportUnrecoverableError, chrome::GetChannel());
 
 #if BUILDFLAG(ENABLE_SUPERVISED_USERS)
   if (base::FeatureList::IsEnabled(switches::kSyncPseudoUSSSupervisedUsers)) {
     controllers.push_back(
         std::make_unique<SupervisedUserSyncModelTypeController>(
-            syncer::SUPERVISED_USER_SETTINGS, profile_, this));
+            syncer::SUPERVISED_USER_SETTINGS, profile_, dump_stack, this));
     controllers.push_back(
         std::make_unique<SupervisedUserSyncModelTypeController>(
-            syncer::SUPERVISED_USER_WHITELISTS, profile_, this));
+            syncer::SUPERVISED_USER_WHITELISTS, profile_, dump_stack, this));
   } else {
     controllers.push_back(
         std::make_unique<SupervisedUserSyncDataTypeController>(
-            syncer::SUPERVISED_USER_SETTINGS, error_callback, this, profile_));
+            syncer::SUPERVISED_USER_SETTINGS, dump_stack, this, profile_));
     controllers.push_back(
         std::make_unique<SupervisedUserSyncDataTypeController>(
-            syncer::SUPERVISED_USER_WHITELISTS, error_callback, this,
-            profile_));
+            syncer::SUPERVISED_USER_WHITELISTS, dump_stack, this, profile_));
   }
 #endif  // BUILDFLAG(ENABLE_SUPERVISED_USERS)
 
@@ -319,10 +318,10 @@ ChromeSyncClient::CreateDataTypeControllers(
           syncer::APPS, GetModelTypeStoreService()->GetStoreFactory(),
           base::BindOnce(&ChromeSyncClient::GetSyncableServiceForType,
                          base::Unretained(this), syncer::APPS),
-          profile_));
+          dump_stack, profile_));
     } else {
       controllers.push_back(std::make_unique<ExtensionDataTypeController>(
-          syncer::APPS, error_callback, this, profile_));
+          syncer::APPS, dump_stack, this, profile_));
     }
   }
 
@@ -334,10 +333,10 @@ ChromeSyncClient::CreateDataTypeControllers(
           syncer::EXTENSIONS, GetModelTypeStoreService()->GetStoreFactory(),
           base::BindOnce(&ChromeSyncClient::GetSyncableServiceForType,
                          base::Unretained(this), syncer::EXTENSIONS),
-          profile_));
+          dump_stack, profile_));
     } else {
       controllers.push_back(std::make_unique<ExtensionDataTypeController>(
-          syncer::EXTENSIONS, error_callback, this, profile_));
+          syncer::EXTENSIONS, dump_stack, this, profile_));
     }
   }
 
@@ -345,14 +344,14 @@ ChromeSyncClient::CreateDataTypeControllers(
   // disabled.
   if (!disabled_types.Has(syncer::EXTENSION_SETTINGS)) {
     controllers.push_back(std::make_unique<ExtensionSettingDataTypeController>(
-        syncer::EXTENSION_SETTINGS, error_callback, this, profile_));
+        syncer::EXTENSION_SETTINGS, dump_stack, this, profile_));
   }
 
   // App setting sync is enabled by default.  Register unless explicitly
   // disabled.
   if (!disabled_types.Has(syncer::APP_SETTINGS)) {
     controllers.push_back(std::make_unique<ExtensionSettingDataTypeController>(
-        syncer::APP_SETTINGS, error_callback, this, profile_));
+        syncer::APP_SETTINGS, dump_stack, this, profile_));
   }
 #endif  // BUILDFLAG(ENABLE_EXTENSIONS)
 
@@ -364,10 +363,10 @@ ChromeSyncClient::CreateDataTypeControllers(
           syncer::THEMES, GetModelTypeStoreService()->GetStoreFactory(),
           base::BindOnce(&ChromeSyncClient::GetSyncableServiceForType,
                          base::Unretained(this), syncer::THEMES),
-          profile_));
+          dump_stack, profile_));
     } else {
       controllers.push_back(std::make_unique<ThemeDataTypeController>(
-          error_callback, this, profile_));
+          dump_stack, this, profile_));
     }
   }
 
@@ -376,11 +375,11 @@ ChromeSyncClient::CreateDataTypeControllers(
   if (!disabled_types.Has(syncer::SEARCH_ENGINES)) {
     if (base::FeatureList::IsEnabled(switches::kSyncPseudoUSSSearchEngines)) {
       controllers.push_back(std::make_unique<SearchEngineModelTypeController>(
-          error_callback, GetModelTypeStoreService()->GetStoreFactory(),
+          dump_stack, GetModelTypeStoreService()->GetStoreFactory(),
           TemplateURLServiceFactory::GetForProfile(profile_)));
     } else {
       controllers.push_back(std::make_unique<SearchEngineDataTypeController>(
-          error_callback, this,
+          dump_stack, this,
           TemplateURLServiceFactory::GetForProfile(profile_)));
     }
   }
@@ -392,10 +391,11 @@ ChromeSyncClient::CreateDataTypeControllers(
         std::make_unique<syncer::SyncableServiceBasedModelTypeController>(
             syncer::APP_LIST, GetModelTypeStoreService()->GetStoreFactory(),
             base::BindOnce(&ChromeSyncClient::GetSyncableServiceForType,
-                           base::Unretained(this), syncer::APP_LIST)));
+                           base::Unretained(this), syncer::APP_LIST),
+            dump_stack));
   } else {
     controllers.push_back(std::make_unique<AsyncDirectoryTypeController>(
-        syncer::APP_LIST, error_callback, this, syncer::GROUP_UI,
+        syncer::APP_LIST, dump_stack, this, syncer::GROUP_UI,
         base::CreateSingleThreadTaskRunnerWithTraits({BrowserThread::UI})));
   }
 #endif  // BUILDFLAG(ENABLE_APP_LIST)
@@ -408,10 +408,11 @@ ChromeSyncClient::CreateDataTypeControllers(
           std::make_unique<syncer::SyncableServiceBasedModelTypeController>(
               syncer::DICTIONARY, GetModelTypeStoreService()->GetStoreFactory(),
               base::BindOnce(&ChromeSyncClient::GetSyncableServiceForType,
-                             base::Unretained(this), syncer::DICTIONARY)));
+                             base::Unretained(this), syncer::DICTIONARY),
+              dump_stack));
     } else {
       controllers.push_back(std::make_unique<AsyncDirectoryTypeController>(
-          syncer::DICTIONARY, error_callback, this, syncer::GROUP_UI,
+          syncer::DICTIONARY, dump_stack, this, syncer::GROUP_UI,
           base::CreateSingleThreadTaskRunnerWithTraits({BrowserThread::UI})));
     }
   }
@@ -422,12 +423,12 @@ ChromeSyncClient::CreateDataTypeControllers(
           switches::kEnableWifiCredentialSync) &&
       !disabled_types.Has(syncer::WIFI_CREDENTIALS)) {
     controllers.push_back(std::make_unique<AsyncDirectoryTypeController>(
-        syncer::WIFI_CREDENTIALS, error_callback, this, syncer::GROUP_UI,
+        syncer::WIFI_CREDENTIALS, dump_stack, this, syncer::GROUP_UI,
         base::CreateSingleThreadTaskRunnerWithTraits({BrowserThread::UI})));
   }
   if (arc::IsArcAllowedForProfile(profile_)) {
     controllers.push_back(std::make_unique<ArcPackageSyncDataTypeController>(
-        syncer::ARC_PACKAGE, error_callback, this, profile_));
+        syncer::ARC_PACKAGE, dump_stack, this, profile_));
   }
 #endif  // defined(OS_CHROMEOS)
 

chrome/browser/sync/glue/extension_model_type_controller.cc

@@ -15,11 +15,13 @@ ExtensionModelTypeController::ExtensionModelTypeController(
     syncer::ModelType type,
     syncer::OnceModelTypeStoreFactory store_factory,
     SyncableServiceProvider syncable_service_provider,
+    const base::RepeatingClosure& dump_stack,
     Profile* profile)
     : SyncableServiceBasedModelTypeController(
           type,
           std::move(store_factory),
-          std::move(syncable_service_provider)),
+          std::move(syncable_service_provider),
+          dump_stack),
       profile_(profile) {
   DCHECK(type == syncer::EXTENSIONS || type == syncer::APPS ||
          type == syncer::THEMES);

chrome/browser/sync/glue/extension_model_type_controller.h

@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_SYNC_GLUE_EXTENSION_MODEL_TYPE_CONTROLLER_H_
 #define CHROME_BROWSER_SYNC_GLUE_EXTENSION_MODEL_TYPE_CONTROLLER_H_
 
+#include "base/callback_forward.h"
 #include "base/macros.h"
 #include "components/sync/driver/syncable_service_based_model_type_controller.h"
 
@@ -22,6 +23,7 @@ class ExtensionModelTypeController
       syncer::ModelType type,
       syncer::OnceModelTypeStoreFactory store_factory,
       SyncableServiceProvider syncable_service_provider,
+      const base::RepeatingClosure& dump_stack,
       Profile* profile);
   ~ExtensionModelTypeController() override;
 

components/browser_sync/profile_sync_components_factory_impl.cc

@@ -138,7 +138,7 @@ ProfileSyncComponentsFactoryImpl::CreateCommonDataTypeControllers(
     syncer::ModelTypeSet disabled_types,
     syncer::LocalDeviceInfoProvider* local_device_info_provider) {
   syncer::DataTypeController::TypeVector controllers;
-  base::Closure error_callback =
+  const base::RepeatingClosure dump_stack =
       base::BindRepeating(&syncer::ReportUnrecoverableError, channel_);
 
   // TODO(stanisc): can DEVICE_INFO be one of disabled datatypes?
@@ -166,7 +166,7 @@ ProfileSyncComponentsFactoryImpl::CreateCommonDataTypeControllers(
       } else {
         controllers.push_back(
             std::make_unique<AutofillProfileDataTypeController>(
-                db_thread_, error_callback, sync_client_,
+                db_thread_, dump_stack, sync_client_,
                 web_data_service_on_disk_));
       }
     }
@@ -183,7 +183,7 @@ ProfileSyncComponentsFactoryImpl::CreateCommonDataTypeControllers(
       } else {
         controllers.push_back(
             std::make_unique<AutofillWalletDataTypeController>(
-                syncer::AUTOFILL_WALLET_DATA, db_thread_, error_callback,
+                syncer::AUTOFILL_WALLET_DATA, db_thread_, dump_stack,
                 sync_client_, web_data_service_on_disk_));
       }
     }
@@ -201,7 +201,7 @@ ProfileSyncComponentsFactoryImpl::CreateCommonDataTypeControllers(
       } else {
         controllers.push_back(
             std::make_unique<AutofillWalletDataTypeController>(
-                syncer::AUTOFILL_WALLET_METADATA, db_thread_, error_callback,
+                syncer::AUTOFILL_WALLET_METADATA, db_thread_, dump_stack,
                 sync_client_, web_data_service_on_disk_));
       }
     }
@@ -221,7 +221,7 @@ ProfileSyncComponentsFactoryImpl::CreateCommonDataTypeControllers(
                                   sync_client_->GetFaviconService()))));
     } else {
       controllers.push_back(std::make_unique<BookmarkDataTypeController>(
-          error_callback, sync_client_));
+          dump_stack, sync_client_));
     }
   }
 
@@ -244,12 +244,12 @@ ProfileSyncComponentsFactoryImpl::CreateCommonDataTypeControllers(
               switches::kSyncPseudoUSSHistoryDeleteDirectives)) {
         controllers.push_back(
             std::make_unique<HistoryDeleteDirectivesModelTypeController>(
-                sync_client_));
+                dump_stack, sync_client_));
 
       } else {
         controllers.push_back(
             std::make_unique<HistoryDeleteDirectivesDataTypeController>(
-                error_callback, sync_client_));
+                dump_stack, sync_client_));
       }
     }
 
@@ -270,7 +270,7 @@ ProfileSyncComponentsFactoryImpl::CreateCommonDataTypeControllers(
                 history_disabled_pref_));
       } else {
         controllers.push_back(std::make_unique<SessionDataTypeController>(
-            error_callback, sync_client_, local_device_info_provider,
+            dump_stack, sync_client_, local_device_info_provider,
             history_disabled_pref_));
       }
     }
@@ -285,14 +285,16 @@ ProfileSyncComponentsFactoryImpl::CreateCommonDataTypeControllers(
                 sync_client_->GetModelTypeStoreService()->GetStoreFactory(),
                 base::BindOnce(&syncer::SyncClient::GetSyncableServiceForType,
                                base::Unretained(sync_client_),
-                               syncer::FAVICON_IMAGES)));
+                               syncer::FAVICON_IMAGES),
+                dump_stack));
         controllers.push_back(
             std::make_unique<SyncableServiceBasedModelTypeController>(
                 syncer::FAVICON_TRACKING,
                 sync_client_->GetModelTypeStoreService()->GetStoreFactory(),
                 base::BindOnce(&syncer::SyncClient::GetSyncableServiceForType,
                                base::Unretained(sync_client_),
-                               syncer::FAVICON_TRACKING)));
+                               syncer::FAVICON_TRACKING),
+                dump_stack));
       } else {
         controllers.push_back(std::make_unique<AsyncDirectoryTypeController>(
             syncer::FAVICON_IMAGES, base::RepeatingClosure(), sync_client_,
@@ -308,7 +310,7 @@ ProfileSyncComponentsFactoryImpl::CreateCommonDataTypeControllers(
   // disabled.
   if (!disabled_types.Has(syncer::PASSWORDS)) {
     controllers.push_back(std::make_unique<PasswordDataTypeController>(
-        error_callback, sync_client_,
+        dump_stack, sync_client_,
         sync_client_->GetPasswordStateChangedCallback(), password_store_));
   }
 
@@ -324,10 +326,11 @@ ProfileSyncComponentsFactoryImpl::CreateCommonDataTypeControllers(
               sync_client_->GetModelTypeStoreService()->GetStoreFactory(),
               base::BindOnce(&syncer::SyncClient::GetSyncableServiceForType,
                              base::Unretained(sync_client_),
-                             syncer::PREFERENCES)));
+                             syncer::PREFERENCES),
+              dump_stack));
     } else {
       controllers.push_back(std::make_unique<AsyncDirectoryTypeController>(
-          syncer::PREFERENCES, error_callback, sync_client_, syncer::GROUP_UI,
+          syncer::PREFERENCES, dump_stack, sync_client_, syncer::GROUP_UI,
           ui_thread_));
     }
   }
@@ -341,10 +344,11 @@ ProfileSyncComponentsFactoryImpl::CreateCommonDataTypeControllers(
               sync_client_->GetModelTypeStoreService()->GetStoreFactory(),
               base::BindOnce(&syncer::SyncClient::GetSyncableServiceForType,
                              base::Unretained(sync_client_),
-                             syncer::PRIORITY_PREFERENCES)));
+                             syncer::PRIORITY_PREFERENCES),
+              dump_stack));
     } else {
       controllers.push_back(std::make_unique<AsyncDirectoryTypeController>(
-          syncer::PRIORITY_PREFERENCES, error_callback, sync_client_,
+          syncer::PRIORITY_PREFERENCES, dump_stack, sync_client_,
           syncer::GROUP_UI, ui_thread_));
     }
   }

components/history/core/browser/sync/history_delete_directives_model_type_controller.cc

@@ -14,13 +14,16 @@
 namespace browser_sync {
 
 HistoryDeleteDirectivesModelTypeController::
-    HistoryDeleteDirectivesModelTypeController(syncer::SyncClient* sync_client)
+    HistoryDeleteDirectivesModelTypeController(
+        const base::RepeatingClosure& dump_stack,
+        syncer::SyncClient* sync_client)
     : SyncableServiceBasedModelTypeController(
           syncer::HISTORY_DELETE_DIRECTIVES,
           sync_client->GetModelTypeStoreService()->GetStoreFactory(),
           base::BindOnce(&syncer::SyncClient::GetSyncableServiceForType,
                          base::Unretained(sync_client),
-                         syncer::HISTORY_DELETE_DIRECTIVES)),
+                         syncer::HISTORY_DELETE_DIRECTIVES),
+          dump_stack),
       sync_client_(sync_client) {}
 
 HistoryDeleteDirectivesModelTypeController::

components/history/core/browser/sync/history_delete_directives_model_type_controller.h

@@ -5,6 +5,7 @@
 #ifndef COMPONENTS_HISTORY_CORE_BROWSER_SYNC_HISTORY_DELETE_DIRECTIVES_MODEL_TYPE_CONTROLLER_H_
 #define COMPONENTS_HISTORY_CORE_BROWSER_SYNC_HISTORY_DELETE_DIRECTIVES_MODEL_TYPE_CONTROLLER_H_
 
+#include "base/callback_forward.h"
 #include "base/macros.h"
 #include "components/sync/driver/sync_service_observer.h"
 #include "components/sync/driver/syncable_service_based_model_type_controller.h"
@@ -22,7 +23,8 @@ class HistoryDeleteDirectivesModelTypeController
       public syncer::SyncServiceObserver {
  public:
   // |sync_client| must not be null and must outlive this object.
-  explicit HistoryDeleteDirectivesModelTypeController(
+  HistoryDeleteDirectivesModelTypeController(
+      const base::RepeatingClosure& dump_stack,
       syncer::SyncClient* sync_client);
   ~HistoryDeleteDirectivesModelTypeController() override;
 

components/sync/driver/syncable_service_based_model_type_controller.cc

@@ -22,10 +22,12 @@ class ControllerDelegate : public ModelTypeControllerDelegate {
 
   ControllerDelegate(ModelType type,
                      OnceModelTypeStoreFactory store_factory,
-                     SyncableServiceProvider syncable_service_provider)
+                     SyncableServiceProvider syncable_service_provider,
+                     const base::RepeatingClosure& dump_stack)
       : type_(type),
         store_factory_(std::move(store_factory)),
-        syncable_service_provider_(std::move(syncable_service_provider)) {
+        syncable_service_provider_(std::move(syncable_service_provider)),
+        dump_stack_(dump_stack) {
     DCHECK(store_factory_);
     DCHECK(syncable_service_provider_);
   }
@@ -62,9 +64,8 @@ class ControllerDelegate : public ModelTypeControllerDelegate {
       DCHECK(syncable_service);
       bridge_ = std::make_unique<SyncableServiceBasedBridge>(
           type_, std::move(store_factory_),
-          std::make_unique<ClientTagBasedModelTypeProcessor>(
-              type_,
-              /*dump_stack=*/base::RepeatingClosure()),
+          std::make_unique<ClientTagBasedModelTypeProcessor>(type_,
+                                                             dump_stack_),
           syncable_service.get());
     }
     return bridge_->change_processor()->GetControllerDelegate().get();
@@ -73,6 +74,7 @@ class ControllerDelegate : public ModelTypeControllerDelegate {
   const ModelType type_;
   OnceModelTypeStoreFactory store_factory_;
   SyncableServiceProvider syncable_service_provider_;
+  const base::RepeatingClosure dump_stack_;
   std::unique_ptr<ModelTypeSyncBridge> bridge_;
 
   DISALLOW_COPY_AND_ASSIGN(ControllerDelegate);
@@ -84,12 +86,14 @@ SyncableServiceBasedModelTypeController::
     SyncableServiceBasedModelTypeController(
         ModelType type,
         OnceModelTypeStoreFactory store_factory,
-        SyncableServiceProvider syncable_service_provider)
+        SyncableServiceProvider syncable_service_provider,
+        const base::RepeatingClosure& dump_stack)
     : ModelTypeController(type,
                           std::make_unique<ControllerDelegate>(
                               type,
                               std::move(store_factory),
-                              std::move(syncable_service_provider))) {}
+                              std::move(syncable_service_provider),
+                              dump_stack)) {}
 
 SyncableServiceBasedModelTypeController::
     ~SyncableServiceBasedModelTypeController() {}

components/sync/driver/syncable_service_based_model_type_controller.h

@@ -28,7 +28,8 @@ class SyncableServiceBasedModelTypeController : public ModelTypeController {
   SyncableServiceBasedModelTypeController(
       ModelType type,
       OnceModelTypeStoreFactory store_factory,
-      SyncableServiceProvider syncable_service_provider);
+      SyncableServiceProvider syncable_service_provider,
+      const base::RepeatingClosure& dump_stack);
   ~SyncableServiceBasedModelTypeController() override;
 
  private: