Commit 723455a2 authored by Mohamed Amir Yosef's avatar Mohamed Amir Yosef Committed by Commit Bot

[Sync::USS] BookmarkProc. should have weak_ptr_factory for the worker

Before this patch:
BookmarkModelTypeProcessor has one weak_ptr_facotry used for both
the controller and the worker. This resulted in the worker having a
processor ptr while the model error isn't yet propagated.

After this patch:
The BookmarkModelTypeProcessor has dedicated weak_ptr_factory for the
worker such that in case of model error, it gets invalidated directly
while leaving the other weak_ptr_factory for the controller intact.

Bug: 516866
Change-Id: If0d9759e0cba7e5b00879646f9072522cbdb866f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1635511
Auto-Submit: Mohamed Amir Yosef <mamir@chromium.org>
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: default avatarMikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#664724}
parent 7044d791
...@@ -145,7 +145,9 @@ std::string ComputeServerDefinedUniqueTagForDebugging( ...@@ -145,7 +145,9 @@ std::string ComputeServerDefinedUniqueTagForDebugging(
BookmarkModelTypeProcessor::BookmarkModelTypeProcessor( BookmarkModelTypeProcessor::BookmarkModelTypeProcessor(
BookmarkUndoService* bookmark_undo_service) BookmarkUndoService* bookmark_undo_service)
: bookmark_undo_service_(bookmark_undo_service), weak_ptr_factory_(this) {} : bookmark_undo_service_(bookmark_undo_service),
weak_ptr_factory_for_controller_(this),
weak_ptr_factory_for_worker_(this) {}
BookmarkModelTypeProcessor::~BookmarkModelTypeProcessor() { BookmarkModelTypeProcessor::~BookmarkModelTypeProcessor() {
if (bookmark_model_ && bookmark_model_observer_) { if (bookmark_model_ && bookmark_model_observer_) {
...@@ -170,6 +172,7 @@ void BookmarkModelTypeProcessor::ConnectSync( ...@@ -170,6 +172,7 @@ void BookmarkModelTypeProcessor::ConnectSync(
void BookmarkModelTypeProcessor::DisconnectSync() { void BookmarkModelTypeProcessor::DisconnectSync() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
weak_ptr_factory_for_worker_.InvalidateWeakPtrs();
if (!worker_) { if (!worker_) {
return; return;
} }
...@@ -370,7 +373,7 @@ size_t BookmarkModelTypeProcessor::EstimateMemoryUsage() const { ...@@ -370,7 +373,7 @@ size_t BookmarkModelTypeProcessor::EstimateMemoryUsage() const {
base::WeakPtr<syncer::ModelTypeControllerDelegate> base::WeakPtr<syncer::ModelTypeControllerDelegate>
BookmarkModelTypeProcessor::GetWeakPtr() { BookmarkModelTypeProcessor::GetWeakPtr() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
return weak_ptr_factory_.GetWeakPtr(); return weak_ptr_factory_for_controller_.GetWeakPtr();
} }
void BookmarkModelTypeProcessor::OnSyncStarting( void BookmarkModelTypeProcessor::OnSyncStarting(
...@@ -427,7 +430,7 @@ void BookmarkModelTypeProcessor::ConnectIfReady() { ...@@ -427,7 +430,7 @@ void BookmarkModelTypeProcessor::ConnectIfReady() {
} }
activation_context->type_processor = activation_context->type_processor =
std::make_unique<syncer::ModelTypeProcessorProxy>( std::make_unique<syncer::ModelTypeProcessorProxy>(
weak_ptr_factory_.GetWeakPtr(), weak_ptr_factory_for_worker_.GetWeakPtr(),
base::SequencedTaskRunnerHandle::Get()); base::SequencedTaskRunnerHandle::Get());
std::move(start_callback_).Run(std::move(activation_context)); std::move(start_callback_).Run(std::move(activation_context));
} }
...@@ -464,7 +467,8 @@ void BookmarkModelTypeProcessor::OnSyncStopping( ...@@ -464,7 +467,8 @@ void BookmarkModelTypeProcessor::OnSyncStopping(
} }
// Do not let any delayed callbacks to be called. // Do not let any delayed callbacks to be called.
weak_ptr_factory_.InvalidateWeakPtrs(); weak_ptr_factory_for_controller_.InvalidateWeakPtrs();
weak_ptr_factory_for_worker_.InvalidateWeakPtrs();
} }
void BookmarkModelTypeProcessor::NudgeForCommitIfNeeded() { void BookmarkModelTypeProcessor::NudgeForCommitIfNeeded() {
......
...@@ -163,7 +163,12 @@ class BookmarkModelTypeProcessor : public syncer::ModelTypeProcessor, ...@@ -163,7 +163,12 @@ class BookmarkModelTypeProcessor : public syncer::ModelTypeProcessor,
std::unique_ptr<BookmarkModelObserverImpl> bookmark_model_observer_; std::unique_ptr<BookmarkModelObserverImpl> bookmark_model_observer_;
base::WeakPtrFactory<BookmarkModelTypeProcessor> weak_ptr_factory_; // WeakPtrFactory for this processor for ModelTypeController.
base::WeakPtrFactory<BookmarkModelTypeProcessor>
weak_ptr_factory_for_controller_;
// WeakPtrFactory for this processor which will be sent to sync thread.
base::WeakPtrFactory<BookmarkModelTypeProcessor> weak_ptr_factory_for_worker_;
DISALLOW_COPY_AND_ASSIGN(BookmarkModelTypeProcessor); DISALLOW_COPY_AND_ASSIGN(BookmarkModelTypeProcessor);
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment