Skip to content

Commit

Permalink
SyncableServiceBasedModelTypeController: Handle null SyncableService
Browse files Browse the repository at this point in the history
It turns out that in some cases, the SyncableService passed into
SyncableServiceBasedModelTypeController may be null - in particular,
this may happen for HistoryDeleteDirectivesModelTypeController if the
HistoryService is null (which can happen e.g. if its database
initialization failed).
This CL fixes the issue by handling null within
SyncableServiceBasedModelTypeController's ControllerDelegate.

(cherry picked from commit e145a12)

Bug: 1394815
Change-Id: I959ed706c3feb12b318b5861866a945b392195eb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4075804
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1078663}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4083728
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Srinivas Sista <srinivassista@chromium.org>
Owners-Override: Srinivas Sista <srinivassista@chromium.org>
Cr-Commit-Position: refs/branch-heads/5359@{#1104}
Cr-Branched-From: 27d3765-refs/heads/main@{#1058933}
  • Loading branch information
Marc Treib authored and Chromium LUCI CQ committed Dec 6, 2022
1 parent 0f5d741 commit 8568b91
Showing 1 changed file with 21 additions and 1 deletion.
Expand Up @@ -26,7 +26,8 @@ class ControllerDelegate : public ModelTypeControllerDelegate {
: type_(type), dump_stack_(dump_stack) {
DCHECK(store_factory);

// The |syncable_service| can be null in tests.
// The |syncable_service| can be null in some cases (e.g. when the
// underlying service failed to initialize), and in tests.
if (syncable_service) {
bridge_ = std::make_unique<SyncableServiceBasedBridge>(
type_, std::move(store_factory),
Expand All @@ -43,24 +44,43 @@ class ControllerDelegate : public ModelTypeControllerDelegate {

void OnSyncStarting(const DataTypeActivationRequest& request,
StartCallback callback) override {
if (!bridge_) {
// TODO(crbug.com/1394815): Consider running `callback` here to avoid
// blocking the Sync machinery.
return;
}
GetBridgeDelegate()->OnSyncStarting(request, std::move(callback));
}

void OnSyncStopping(SyncStopMetadataFate metadata_fate) override {
if (!bridge_) {
return;
}
GetBridgeDelegate()->OnSyncStopping(metadata_fate);
}

void GetAllNodesForDebugging(AllNodesCallback callback) override {
if (!bridge_) {
// TODO(crbug.com/1394815): Consider running `callback` here.
return;
}
GetBridgeDelegate()->GetAllNodesForDebugging(std::move(callback));
}

void GetTypeEntitiesCountForDebugging(
base::OnceCallback<void(const TypeEntitiesCount&)> callback)
const override {
if (!bridge_) {
// TODO(crbug.com/1394815): Consider running `callback` here.
return;
}
GetBridgeDelegate()->GetTypeEntitiesCountForDebugging(std::move(callback));
}

void RecordMemoryUsageAndCountsHistograms() override {
if (!bridge_) {
return;
}
GetBridgeDelegate()->RecordMemoryUsageAndCountsHistograms();
}

Expand Down

0 comments on commit 8568b91

Please sign in to comment.