New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 647875 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

[USS] Changes during encryption appear to cause crashes

Project Member Reported by s...@chromium.org, Sep 16 2016

Issue description

IN_PROC_BROWSER_TEST_F(TwoClientUssSyncTest, Encryption) {
  ASSERT_TRUE(SetupSync());
  TestModelTypeService* model1 = GetModelTypeService(0);
  TestModelTypeService* model2 = GetModelTypeService(1);
  ASSERT_TRUE(EnableEncryption(0));
  model1->WriteItem(kKey1, kValue1);
  ASSERT_TRUE(DataChecker(model2, kKey1, kValue1).Wait());
}

[112587:112659:0916/164438:FATAL:sync_scheduler_impl.cc(444)] Check failed: !syncer_->IsSyncing(). 
#0 0x7fbb084195ce base::debug::StackTrace::StackTrace()
#1 0x7fbb08486c0f logging::LogMessage::~LogMessage()
#2 0x00000585fbac syncer::SyncSchedulerImpl::ScheduleNudgeImpl()
#3 0x00000585ce13 syncer::SyncSchedulerImpl::ScheduleLocalNudge()
#4 0x0000057a3156 syncer::SyncManagerImpl::RequestNudgeForDataTypes()
#5 0x0000057a3f4f syncer::SyncManagerImpl::NudgeForCommit()
#6 0x0000058484ca syncer_v2::ModelTypeWorker::UpdateCryptographer()
#7 0x000005837c8a syncer::ModelTypeRegistry::OnEncryptionStateChanged()
#8 0x000005837b96 syncer::ModelTypeRegistry::OnEncryptedTypesChanged()
#9 0x00000589f09c syncer::SyncEncryptionHandlerImpl::EnableEncryptEverythingImpl()
#10 0x0000058a1eaa syncer::SyncEncryptionHandlerImpl::UpdateEncryptedTypesFromNigori()
#11 0x000005899360 syncer::SyncEncryptionHandlerImpl::ApplyNigoriUpdateImpl()
#12 0x00000589f402 syncer::SyncEncryptionHandlerImpl::ApplyNigoriUpdate()
#13 0x0000058d3b1e syncer::ApplyNigoriUpdate()
#14 0x0000058d3611 syncer::ApplyControlDataUpdates()
#15 0x00000586bbdf syncer::Syncer::DownloadAndApplyUpdates()
#16 0x00000586b743 syncer::Syncer::NormalSyncShare()
#17 0x000005860f37 syncer::SyncSchedulerImpl::DoNudgeSyncCycleJob()
#18 0x0000058639e6 syncer::SyncSchedulerImpl::TrySyncCycleJobImpl()
#19 0x0000007c6142 _ZN4base8internal13FunctorTraitsIMN11google_apis19UrlFetchRequestBaseEFvvEvE6InvokeIRKNS_7WeakPtrINS2_5drive30SingleBatchableDelegateRequestEEEJEEEvS5_OT_DpOT0_
#20 0x000005869ada _ZN4base8internal12InvokeHelperILb1EvE8MakeItSoIRKMN6syncer17SyncSchedulerImplEFvvERKNS_7WeakPtrIS5_EEJEEEvOT_OT0_DpOT1_
#21 0x000005869a62 _ZN4base8internal7InvokerINS0_9BindStateIMN6syncer17SyncSchedulerImplEFvvEJNS_7WeakPtrIS4_EEEEEFvvEE7RunImplIRKS6_RKSt5tupleIJS8_EEJLm0EEEEvOT_OT0_NS_13IndexSequenceIJXspT1_EEEE
#22 0x0000058699ac _ZN4base8internal7InvokerINS0_9BindStateIMN6syncer17SyncSchedulerImplEFvvEJNS_7WeakPtrIS4_EEEEEFvvEE3RunEPNS0_13BindStateBaseE
#23 0x7fbb083e9a4b base::internal::RunMixin<>::Run()
#24 0x7fbb0841eecb base::debug::TaskAnnotator::RunTask()
#25 0x7fbb084ac49c base::MessageLoop::RunTask()
#26 0x7fbb084ac734 base::MessageLoop::DeferOrRunPendingTask()
#27 0x7fbb084ac9fe base::MessageLoop::DoWork()
#28 0x7fbb084c3e13 base::MessagePumpDefault::Run()
#29 0x7fbb084abe7f base::MessageLoop::RunHandler()
#30 0x7fbb08551664 base::RunLoop::Run()
#31 0x7fbb085f6b78 base::Thread::Run()
#32 0x7fbb085f731e base::Thread::ThreadMain()
#33 0x7fbb085de62a base::(anonymous namespace)::ThreadFunc()
#34 0x7fbb08c55184 start_thread
#35 0x7fbaf1fe937d clone

 

Comment 1 by s...@chromium.org, Sep 20 2016

This crash is caused by trying to synchronously start a sync cycle while in an existing sync cycle, which is what the CHECK is guarding against.

The band-aid fix is to just slap a PostTask in the middle somewhere. Because this the sync cycle runs as a blocking task on the sync thread, the new task will not be picked up until after the current cycle is completed.

So I started trying to figure out where's the best place to put the PostTask, the ModelTypeRegistry already has a way to get weak ptrs, and will only cause 1 posted task for all data types, while putting it in worker would mean n posted tasks. However, the worker has slightly more context to feel better about knowing that it should PostTask.

However, the more I thought about this, I'm starting to wonder if maybe we should instead just stop nudging when the worker gets new encrypted data. Git blaming when this logic was added, it was long ago. We may be able to simply not nudge here.

Comment 2 by s...@chromium.org, Sep 20 2016

Encountered another error when turning on encryption that looks like

[38511:38593:0920/122942:FATAL:model_type_worker.cc(300)] Check failed: !sync_entity->has_id_string(). 
#0 0x7ff12f1f8c6e base::debug::StackTrace::StackTrace()
#1 0x7ff12f26749f logging::LogMessage::~LogMessage()
#2 0x000005867d26 syncer_v2::ModelTypeWorker::AdjustCommitProto()
#3 0x000005867a5f syncer_v2::ModelTypeWorker::GetContribution()
#4 0x000005868033 syncer_v2::ModelTypeWorker::GetContribution()
#5 0x0000058f70a1 syncer::CommitProcessor::GatherCommitContributions()
#6 0x0000058f3846 syncer::Commit::Init()
#7 0x000005889b87 syncer::Syncer::BuildAndPostCommits()
#8 0x000005889642 syncer::Syncer::NormalSyncShare()
#9 0x00000587ec17 syncer::SyncSchedulerImpl::DoNudgeSyncCycleJob()
#10 0x0000058816c6 syncer::SyncSchedulerImpl::TrySyncCycleJobImpl()
#11 0x0000007c98e2 _ZN4base8internal13FunctorTraitsIMN11google_apis19UrlFetchRequestBaseEFvvEvE6InvokeIRKNS_7WeakPtrINS2_5drive30SingleBatchableDelegateRequestEEEJEEEvS5_OT_DpOT0_
#12 0x0000058877ba _ZN4base8internal12InvokeHelperILb1EvE8MakeItSoIRKMN6syncer17SyncSchedulerImplEFvvERKNS_7WeakPtrIS5_EEJEEEvOT_OT0_DpOT1_
#13 0x000005887742 _ZN4base8internal7InvokerINS0_9BindStateIMN6syncer17SyncSchedulerImplEFvvEJNS_7WeakPtrIS4_EEEEEFvvEE7RunImplIRKS6_RKSt5tupleIJS8_EEJLm0EEEEvOT_OT0_NS_13IndexSequenceIJXspT1_EEEE
#14 0x00000588768c _ZN4base8internal7InvokerINS0_9BindStateIMN6syncer17SyncSchedulerImplEFvvEJNS_7WeakPtrIS4_EEEEEFvvEE3RunEPNS0_13BindStateBaseE
#15 0x7ff12f1c90eb base::internal::RunMixin<>::Run()
#16 0x7ff12f1fe5e1 base::debug::TaskAnnotator::RunTask()
#17 0x7ff12f28cd2c base::MessageLoop::RunTask()
#18 0x7ff12f28cfc4 base::MessageLoop::DeferOrRunPendingTask()
#19 0x7ff12f28d28e base::MessageLoop::DoWork()
#20 0x7ff12f2a46a3 base::MessagePumpDefault::Run()
#21 0x7ff12f28c70f base::MessageLoop::RunHandler()
#22 0x7ff12f331ba4 base::RunLoop::Run()
#23 0x7ff12f3d7bf8 base::Thread::Run()
#24 0x7ff12f3d839e base::Thread::ThreadMain()
#25 0x7ff12f3bf4fa base::(anonymous namespace)::ThreadFunc()
#26 0x7ff12fa38184 start_thread
#27 0x7ff118d3a37d clone

What seems to happen is that, because of encryption being turned on right after an entity is created, a given processor will try to commit the same entity twice. And both will have kUncommittedVersion as their base_version_. While the second commit is processed by the worker, it blows up as above. Before it gets to this point we need to update it to hold the last commit's version.

Comment 3 by s...@chromium.org, Sep 22 2016

Max pointed out that I was using the old style of encryption. Once I switched to a custom passphrase, I immediately ran into a crash in the controller. Stack trace below. Turns out the problem is that the ModelAssociationManager was tracking a subset of model types, only the ones that didn't get stopped because of encryption. The DataTypeManagerImpl on the other hand calls RegisterWithBackend for all the controllers with types in |last_requested_types_|.

[89517:89517:0922/112809:FATAL:non_blocking_data_type_controller.cc(141)] Check failed: activation_context_. 
#0 0x7f52177f52be base::debug::StackTrace::StackTrace()
#1 0x7f521786337f logging::LogMessage::~LogMessage()
#2 0x000005939a43 sync_driver_v2::NonBlockingDataTypeController::RegisterWithBackend()
#3 0x0000058396b1 sync_driver::DataTypeManagerImpl::RegisterTypesWithBackend()
#4 0x00000583a591 sync_driver::DataTypeManagerImpl::OnAllDataTypesReadyForConfigure()
#5 0x000005869c26 sync_driver::ModelAssociationManager::NotifyDelegateIfReadyForConfigure()
#6 0x000005869244 sync_driver::ModelAssociationManager::LoadEnabledTypes()
#7 0x0000058689f0 sync_driver::ModelAssociationManager::Initialize()
#8 0x0000058393df sync_driver::DataTypeManagerImpl::Restart()
#9 0x0000058387e5 sync_driver::DataTypeManagerImpl::ConfigureImpl()
#10 0x000005838385 sync_driver::DataTypeManagerImpl::Configure()
#11 0x000003d55572 ProfileSyncService::OnPassphraseRequired()
#12 0x0000058535b9 browser_sync::SyncBackendHostImpl::NotifyPassphraseRequired()
#13 0x00000592b375 _ZN4base8internal13FunctorTraitsIMN12browser_sync19SyncBackendHostImplEFvN6syncer24PassphraseRequiredReasonEN7sync_pb13EncryptedDataEEvE6InvokeIRKNS_7WeakPtrIS3_EEJRKS5_RKS7_EEEvS9_OT_DpOT0_
#14 0x00000592b234 _ZN4base8internal12InvokeHelperILb1EvE8MakeItSoIRKMN12browser_sync19SyncBackendHostImplEFvN6syncer24PassphraseRequiredReasonEN7sync_pb13EncryptedDataEERKNS_7WeakPtrIS5_EEJRKS7_RKS9_EEEvOT_OT0_DpOT1_
#15 0x00000592b194 _ZN4base8internal7InvokerINS0_9BindStateIMN12browser_sync19SyncBackendHostImplEFvN6syncer24PassphraseRequiredReasonEN7sync_pb13EncryptedDataEEJNS_7WeakPtrIS4_EES6_S8_EEEFvvEE7RunImplIRKSA_RKSt5tupleIJSC_S6_S8_EEJLm0ELm1ELm2EEEEvOT_OT0_NS_13IndexSequenceIJXspT1_EEEE
#16 0x00000592b06c _ZN4base8internal7InvokerINS0_9BindStateIMN12browser_sync19SyncBackendHostImplEFvN6syncer24PassphraseRequiredReasonEN7sync_pb13EncryptedDataEEJNS_7WeakPtrIS4_EES6_S8_EEEFvvEE3RunEPNS0_13BindStateBaseE
#17 0x7f52177c573b base::internal::RunMixin<>::Run()
#18 0x7f52177fac31 base::debug::TaskAnnotator::RunTask()
#19 0x7f52178886ec base::MessageLoop::RunTask()
#20 0x7f5217888984 base::MessageLoop::DeferOrRunPendingTask()
#21 0x7f5217888c4e base::MessageLoop::DoWork()
#22 0x7f52178a0566 base::MessagePumpGlib::Run()
#23 0x7f52178880cf base::MessageLoop::RunHandler()
#24 0x7f521792cf14 base::RunLoop::Run()
#25 0x000001d9b7d2 StatusChangeChecker::StartBlockingWait()
#26 0x000001d709ec MultiClientStatusChangeChecker::Wait()
#27 0x000001dac978 sync_integration_test_util::AwaitPassphraseRequired()
#28 0x0000006e2d02 TwoClientUssSyncTest_Encryption_Test::RunTestOnMainThread()
#29 0x000001dca0e8 InProcessBrowserTest::RunTestOnMainThreadLoop()
#30 0x00000541ff13 content::BrowserTestBase::ProxyRunTestOnMainThreadLoop()
#31 0x00000075356d _ZN4base8internal13FunctorTraitsIMN18OAuth2TokenService7FetcherEFvvEvE6InvokeIPS3_JEEEvS5_OT_DpOT0_
#32 0x000000753491 _ZN4base8internal12InvokeHelperILb0EvE8MakeItSoIRKMN18OAuth2TokenService7FetcherEFvvEJPS5_EEEvOT_DpOT0_
#33 0x000005421467 _ZN4base8internal7InvokerINS0_9BindStateIMN7content15BrowserTestBaseEFvvEJNS0_17UnretainedWrapperIS4_EEEEEFvvEE7RunImplIRKS6_RKSt5tupleIJS8_EEJLm0EEEEvOT_OT0_NS_13IndexSequenceIJXspT1_EEEE
#34 0x0000054213ac _ZN4base8internal7InvokerINS0_9BindStateIMN7content15BrowserTestBaseEFvvEJNS0_17UnretainedWrapperIS4_EEEEEFvvEE3RunEPNS0_13BindStateBaseE
#35 0x00000077ea3b base::internal::RunMixin<>::Run()
#36 0x00000290e9da ChromeBrowserMainParts::PreMainMessageLoopRunImpl()
#37 0x00000290d520 ChromeBrowserMainParts::PreMainMessageLoopRun()
#38 0x7f52118aced1 content::BrowserMainLoop::PreMainMessageLoopRun()
#39 0x7f5210f5d02d _ZN4base8internal13FunctorTraitsIMN7content12ChildProcessEFvvEvE6InvokeIPS3_JEEEvS5_OT_DpOT0_
#40 0x7f52118b4e01 _ZN4base8internal12InvokeHelperILb0EiE8MakeItSoIRKMN7content15BrowserMainLoopEFivEJPS5_EEEiOT_DpOT0_
#41 0x7f52118b4da7 _ZN4base8internal7InvokerINS0_9BindStateIMN7content15BrowserMainLoopEFivEJNS0_17UnretainedWrapperIS4_EEEEEFivEE7RunImplIRKS6_RKSt5tupleIJS8_EEJLm0EEEEiOT_OT0_NS_13IndexSequenceIJXspT1_EEEE
#42 0x7f52118b4cec _ZN4base8internal7InvokerINS0_9BindStateIMN7content15BrowserMainLoopEFivEJNS0_17UnretainedWrapperIS4_EEEEEFivEE3RunEPNS0_13BindStateBaseE
#43 0x7f5210dc8eeb base::internal::RunMixin<>::Run()
#44 0x7f52123ebd5b content::StartupTaskRunner::RunAllTasksNow()
#45 0x7f52118aad04 content::BrowserMainLoop::CreateStartupTasks()
#46 0x7f52118b7d7d content::BrowserMainRunnerImpl::Initialize()
#47 0x7f52118a6fbf content::BrowserMain()
#48 0x7f5213617a56 content::RunNamedProcessTypeMain()
#49 0x7f5213619ee2 content::ContentMainRunnerImpl::Run()
#50 0x7f5213616af2 content::ContentMain()
#51 0x00000541fc16 content::BrowserTestBase::SetUp()
#52 0x000001dc893e InProcessBrowserTest::SetUp()
#53 0x000001dad91b SyncTest::SetUp()
#54 0x000001eae51a testing::internal::HandleSehExceptionsInMethodIfSupported<>()
#55 0x000001e9f97e testing::internal::HandleExceptionsInMethodIfSupported<>()
#56 0x000001e94683 testing::Test::Run()
#57 0x000001e94e78 testing::TestInfo::Run()
#58 0x000001e9541a testing::TestCase::Run()
#59 0x000001e9a76c testing::internal::UnitTestImpl::RunAllTests()
#60 0x000001eb2caa testing::internal::HandleSehExceptionsInMethodIfSupported<>()
#61 0x000001ea10ee testing::internal::HandleExceptionsInMethodIfSupported<>()

Comment 4 by s...@chromium.org, Sep 22 2016

Unfortunately even after my earlier fix for updating version in the tracker, it seems we still hit the same DCHECK. There seems to  be a race, I'm encountering it a little less than half the time with an integration case that looks like

IN_PROC_BROWSER_TEST_F(TwoClientUssSyncTest, Encryption) {
  ASSERT_TRUE(SetupSync());
  TestModelTypeService* model1 = GetModelTypeService(0);
  TestModelTypeService* model2 = GetModelTypeService(1);

  model1->WriteItem(kKey1, kValue1);
  model2->WriteItem(kKey2, kValue1);

  SetEncryptionPassphrase(0, kValidPassphrase, ProfileSyncService::EXPLICIT);
  ASSERT_TRUE(AwaitPassphraseAccepted(GetSyncService((0))));

  ASSERT_TRUE(AwaitPassphraseRequired(GetSyncService((1))));
  ASSERT_TRUE(SetDecryptionPassphrase(1, kValidPassphrase));
  ASSERT_TRUE(AwaitPassphraseAccepted(GetSyncService((1))));

  model1->WriteItem(kKey3, kValue1);
  model2->WriteItem(kKey4, kValue1);

  // This never returns.
  ASSERT_TRUE(DataChecker(model2, kKey5, kValue1).Wait());
}


[91210:91284:0922/114535:FATAL:model_type_worker.cc(310)] Check failed: sync_entity->has_id_string(). 
#0 0x7f1056dbd2be base::debug::StackTrace::StackTrace()
#1 0x7f1056e2b37f logging::LogMessage::~LogMessage()
#2 0x0000058b50b2 syncer_v2::ModelTypeWorker::AdjustCommitProto()
#3 0x0000058b4cdf syncer_v2::ModelTypeWorker::GetContribution()
#4 0x0000058b52b3 syncer_v2::ModelTypeWorker::GetContribution()
#5 0x0000059446e1 syncer::CommitProcessor::GatherCommitContributions()
#6 0x000005940e86 syncer::Commit::Init()
#7 0x0000058d6e07 syncer::Syncer::BuildAndPostCommits()
#8 0x0000058d68c2 syncer::Syncer::NormalSyncShare()
#9 0x0000058cbe97 syncer::SyncSchedulerImpl::DoNudgeSyncCycleJob()
#10 0x0000058ce946 syncer::SyncSchedulerImpl::TrySyncCycleJobImpl()
#11 0x0000007ca662 _ZN4base8internal13FunctorTraitsIMN11google_apis19UrlFetchRequestBaseEFvvEvE6InvokeIRKNS_7WeakPtrINS2_5drive30SingleBatchableDelegateRequestEEEJEEEvS5_OT_DpOT0_
#12 0x0000058d4a3a _ZN4base8internal12InvokeHelperILb1EvE8MakeItSoIRKMN6syncer17SyncSchedulerImplEFvvERKNS_7WeakPtrIS5_EEJEEEvOT_OT0_DpOT1_
#13 0x0000058d49c2 _ZN4base8internal7InvokerINS0_9BindStateIMN6syncer17SyncSchedulerImplEFvvEJNS_7WeakPtrIS4_EEEEEFvvEE7RunImplIRKS6_RKSt5tupleIJS8_EEJLm0EEEEvOT_OT0_NS_13IndexSequenceIJXspT1_EEEE
#14 0x0000058d490c _ZN4base8internal7InvokerINS0_9BindStateIMN6syncer17SyncSchedulerImplEFvvEJNS_7WeakPtrIS4_EEEEEFvvEE3RunEPNS0_13BindStateBaseE
#15 0x7f1056d8d73b base::internal::RunMixin<>::Run()
#16 0x7f1056dc2c31 base::debug::TaskAnnotator::RunTask()
#17 0x7f1056e506ec base::MessageLoop::RunTask()
#18 0x7f1056e50984 base::MessageLoop::DeferOrRunPendingTask()
#19 0x7f1056e50c4e base::MessageLoop::DoWork()
#20 0x7f1056e679a3 base::MessagePumpDefault::Run()
#21 0x7f1056e500cf base::MessageLoop::RunHandler()
#22 0x7f1056ef4f14 base::RunLoop::Run()
#23 0x7f1056f9b4e8 base::Thread::Run()
#24 0x7f1056f9bc8e base::Thread::ThreadMain()
#25 0x7f1056f82dea base::(anonymous namespace)::ThreadFunc()
#26 0x7f10575fc184 start_thread
#27 0x7f104072437d clone

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 22 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/946cbb1044cabe81b6a6758f9140089e60fc1950

commit 946cbb1044cabe81b6a6758f9140089e60fc1950
Author: skym <skym@chromium.org>
Date: Thu Sep 22 21:45:54 2016

[Sync] Fixing two bugs in the worker revealed by trying to add an encryption integration test.

The first bug was we were trying to start a nested cycle cycle, which is not allowed. The logic in the worker that tries to nudge in response to encryption changing is super old, and doesn't make any sense, so this was fixed by simply not nudging. If there needs to be a commit in response to encryption changing, the processor will drive this.

The second issue was that in certain cases the worker would try to commit something with an id but not a version. This isn't supposed to be able to happen. Turns out two quick commits could get us into this state because both would be created by the processor before getting valid version data. However, the WorkerEntityTracker can detect this is happening and has enough information to make things right.

BUG= 647875 

Review-Url: https://codereview.chromium.org/2350803005
Cr-Commit-Position: refs/heads/master@{#420471}

[modify] https://crrev.com/946cbb1044cabe81b6a6758f9140089e60fc1950/components/sync/core/processor_entity_tracker_unittest.cc
[modify] https://crrev.com/946cbb1044cabe81b6a6758f9140089e60fc1950/components/sync/engine_impl/model_type_worker.cc
[modify] https://crrev.com/946cbb1044cabe81b6a6758f9140089e60fc1950/components/sync/engine_impl/model_type_worker_unittest.cc
[modify] https://crrev.com/946cbb1044cabe81b6a6758f9140089e60fc1950/components/sync/engine_impl/worker_entity_tracker.cc
[modify] https://crrev.com/946cbb1044cabe81b6a6758f9140089e60fc1950/components/sync/engine_impl/worker_entity_tracker_unittest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Sep 23 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cbb25ab10573f470c79ef624c1896a3830cc185f

commit cbb25ab10573f470c79ef624c1896a3830cc185f
Author: skym <skym@chromium.org>
Date: Fri Sep 23 19:42:16 2016

[Sync] Removed passphrase helper methods, removed ((n))
pattern, and fixed lint violations.

BUG= 647875 

Review-Url: https://codereview.chromium.org/2365763002
Cr-Commit-Position: refs/heads/master@{#420698}

[modify] https://crrev.com/cbb25ab10573f470c79ef624c1896a3830cc185f/chrome/browser/sync/test/integration/migration_test.cc
[modify] https://crrev.com/cbb25ab10573f470c79ef624c1896a3830cc185f/chrome/browser/sync/test/integration/password_manager_setting_migrator_helper.cc
[modify] https://crrev.com/cbb25ab10573f470c79ef624c1896a3830cc185f/chrome/browser/sync/test/integration/passwords_helper.cc
[modify] https://crrev.com/cbb25ab10573f470c79ef624c1896a3830cc185f/chrome/browser/sync/test/integration/passwords_helper.h
[modify] https://crrev.com/cbb25ab10573f470c79ef624c1896a3830cc185f/chrome/browser/sync/test/integration/single_client_app_list_sync_test.cc
[modify] https://crrev.com/cbb25ab10573f470c79ef624c1896a3830cc185f/chrome/browser/sync/test/integration/single_client_apps_sync_test.cc
[modify] https://crrev.com/cbb25ab10573f470c79ef624c1896a3830cc185f/chrome/browser/sync/test/integration/single_client_arc_package_sync_test.cc
[modify] https://crrev.com/cbb25ab10573f470c79ef624c1896a3830cc185f/chrome/browser/sync/test/integration/single_client_bookmarks_sync_test.cc
[modify] https://crrev.com/cbb25ab10573f470c79ef624c1896a3830cc185f/chrome/browser/sync/test/integration/single_client_dictionary_sync_test.cc
[modify] https://crrev.com/cbb25ab10573f470c79ef624c1896a3830cc185f/chrome/browser/sync/test/integration/single_client_directory_sync_test.cc
[modify] https://crrev.com/cbb25ab10573f470c79ef624c1896a3830cc185f/chrome/browser/sync/test/integration/single_client_extensions_sync_test.cc
[modify] https://crrev.com/cbb25ab10573f470c79ef624c1896a3830cc185f/chrome/browser/sync/test/integration/single_client_password_manager_setting_migrator_service_sync_test.cc
[modify] https://crrev.com/cbb25ab10573f470c79ef624c1896a3830cc185f/chrome/browser/sync/test/integration/single_client_passwords_sync_test.cc
[modify] https://crrev.com/cbb25ab10573f470c79ef624c1896a3830cc185f/chrome/browser/sync/test/integration/single_client_preferences_sync_test.cc
[modify] https://crrev.com/cbb25ab10573f470c79ef624c1896a3830cc185f/chrome/browser/sync/test/integration/single_client_search_engines_sync_test.cc
[modify] https://crrev.com/cbb25ab10573f470c79ef624c1896a3830cc185f/chrome/browser/sync/test/integration/single_client_sessions_sync_test.cc
[modify] https://crrev.com/cbb25ab10573f470c79ef624c1896a3830cc185f/chrome/browser/sync/test/integration/single_client_themes_sync_test.cc
[modify] https://crrev.com/cbb25ab10573f470c79ef624c1896a3830cc185f/chrome/browser/sync/test/integration/single_client_typed_urls_sync_test.cc
[modify] https://crrev.com/cbb25ab10573f470c79ef624c1896a3830cc185f/chrome/browser/sync/test/integration/single_client_wifi_credentials_sync_test.cc
[modify] https://crrev.com/cbb25ab10573f470c79ef624c1896a3830cc185f/chrome/browser/sync/test/integration/sync_auth_test.cc
[modify] https://crrev.com/cbb25ab10573f470c79ef624c1896a3830cc185f/chrome/browser/sync/test/integration/sync_exponential_backoff_test.cc
[modify] https://crrev.com/cbb25ab10573f470c79ef624c1896a3830cc185f/chrome/browser/sync/test/integration/sync_extension_helper.cc
[modify] https://crrev.com/cbb25ab10573f470c79ef624c1896a3830cc185f/chrome/browser/sync/test/integration/sync_test.h
[modify] https://crrev.com/cbb25ab10573f470c79ef624c1896a3830cc185f/chrome/browser/sync/test/integration/two_client_app_list_sync_test.cc
[modify] https://crrev.com/cbb25ab10573f470c79ef624c1896a3830cc185f/chrome/browser/sync/test/integration/two_client_bookmarks_sync_test.cc
[modify] https://crrev.com/cbb25ab10573f470c79ef624c1896a3830cc185f/chrome/browser/sync/test/integration/two_client_passwords_sync_test.cc
[modify] https://crrev.com/cbb25ab10573f470c79ef624c1896a3830cc185f/chrome/browser/sync/test/integration/two_client_search_engines_sync_test.cc
[modify] https://crrev.com/cbb25ab10573f470c79ef624c1896a3830cc185f/chrome/browser/sync/test/integration/two_client_sessions_sync_test.cc
[modify] https://crrev.com/cbb25ab10573f470c79ef624c1896a3830cc185f/chrome/browser/sync/test/integration/two_client_typed_urls_sync_test.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Sep 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a842013d816fd4a6c5855bffd65a0a7e27cfdd02

commit a842013d816fd4a6c5855bffd65a0a7e27cfdd02
Author: skym <skym@chromium.org>
Date: Tue Sep 27 03:49:01 2016

[Sync] When a data type is not initialized, also should not be registered with backend.

The switch from queue to deque is to allow iterating over the data structure.

BUG= 647875 

Review-Url: https://codereview.chromium.org/2360283003
Cr-Commit-Position: refs/heads/master@{#421095}

[modify] https://crrev.com/a842013d816fd4a6c5855bffd65a0a7e27cfdd02/components/sync/driver/data_type_manager_impl.cc
[modify] https://crrev.com/a842013d816fd4a6c5855bffd65a0a7e27cfdd02/components/sync/driver/data_type_manager_impl.h
[modify] https://crrev.com/a842013d816fd4a6c5855bffd65a0a7e27cfdd02/components/sync/driver/data_type_manager_impl_unittest.cc

Comment 8 by s...@chromium.org, Oct 7 2016

I think Comment#4 was actually a mistake on my part, from a branch that didn't have the fix https://codereview.chromium.org/2350803005 . I can no longer repro with the pasted code.
Project Member

Comment 9 by bugdroid1@chromium.org, Oct 20 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5afeb69d7607d1d31b98b16d1cfcfc98dd2b0f0b

commit 5afeb69d7607d1d31b98b16d1cfcfc98dd2b0f0b
Author: skym <skym@chromium.org>
Date: Thu Oct 20 20:09:29 2016

[Sync] Adding integration tests for USS encryption and fixing a worker bug.

The worker bug was that we stored undecryptable updates in memory, while continuing to store progress marker changes to disk. To make dealing with this issue more difficult, OnCryptographerUpdated() is called twice while incorporating an update Nigori node, the first containing an old cryptographer that does not have new pending keys.

Do address these issues, updates are no longer propagated to the processor on cryptographer change or when the cryptographer has pending keys. Adding EncryptionAcceptedApplyUpdates() to allow forcing propagation after, for example, a passphrase is entered successfully.

BUG= 647875 

Review-Url: https://chromiumcodereview.appspot.com/2401083003
Cr-Commit-Position: refs/heads/master@{#426576}

[modify] https://crrev.com/5afeb69d7607d1d31b98b16d1cfcfc98dd2b0f0b/chrome/browser/sync/test/integration/two_client_uss_sync_test.cc
[modify] https://crrev.com/5afeb69d7607d1d31b98b16d1cfcfc98dd2b0f0b/components/browser_sync/profile_sync_service.cc
[modify] https://crrev.com/5afeb69d7607d1d31b98b16d1cfcfc98dd2b0f0b/components/sync/engine/sync_encryption_handler.h
[modify] https://crrev.com/5afeb69d7607d1d31b98b16d1cfcfc98dd2b0f0b/components/sync/engine_impl/model_type_registry.cc
[modify] https://crrev.com/5afeb69d7607d1d31b98b16d1cfcfc98dd2b0f0b/components/sync/engine_impl/model_type_worker.cc
[modify] https://crrev.com/5afeb69d7607d1d31b98b16d1cfcfc98dd2b0f0b/components/sync/engine_impl/model_type_worker.h
[modify] https://crrev.com/5afeb69d7607d1d31b98b16d1cfcfc98dd2b0f0b/components/sync/engine_impl/model_type_worker_unittest.cc
[modify] https://crrev.com/5afeb69d7607d1d31b98b16d1cfcfc98dd2b0f0b/components/sync/test/engine/single_type_mock_server.cc
[modify] https://crrev.com/5afeb69d7607d1d31b98b16d1cfcfc98dd2b0f0b/components/sync/test/engine/single_type_mock_server.h
[modify] https://crrev.com/5afeb69d7607d1d31b98b16d1cfcfc98dd2b0f0b/tools/metrics/histograms/histograms.xml

Comment 10 by s...@chromium.org, Oct 26 2016

Status: Fixed (was: Started)

Sign in to add a comment