See issue 771598 where a CHECK that probably should have been a DCHECK caused crashes on every startup. We should audit Sync's usage of CHECK and make sure that we only use it when approriate. See https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md#check_dcheck_and-notreached for guidance. skym@skym:~/chromium/linux/src$ grep -r CHECK components/sync/* | grep -v DCHECK | wc -l 159
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a8b1707bb6ceec193bfa312308c2c6041e8629ee commit a8b1707bb6ceec193bfa312308c2c6041e8629ee Author: Sky Malice <skym@chromium.org> Date: Wed Oct 11 15:45:39 2017 [Sync] Remove CHECK from most of sync codebase. In unittests I tried to replaced [D]CHECKs with assert or expect. I did not touch some of the CHECKs in server_connection_manager, ordinal, or unique_position as I was a little worried about exposing attack surfaces in code I didn't understand. Bug: 771811 Change-Id: Idb604e347384959344725762898a52d4c948f4f6 Reviewed-on: https://chromium-review.googlesource.com/706210 Reviewed-by: Pavel Yatsuk <pavely@chromium.org> Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org> Commit-Queue: Sky Malice <skym@chromium.org> Cr-Commit-Position: refs/heads/master@{#507990} [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/chrome/browser/sync/test/integration/search_engines_helper.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/chrome/browser/sync/test/integration/single_client_user_events_sync_test.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/chrome/browser/sync/test/integration/sync_test.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/chrome/browser/sync/test/integration/sync_test.h [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/chrome/browser/sync/test/integration/wifi_credentials_helper_chromeos.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/browser_sync/abstract_profile_sync_service_test.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/browser_sync/profile_sync_service.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/browser_sync/profile_sync_service_bookmark_unittest.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/browser_sync/profile_sync_test_util.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/base/enum_set.h [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/base/weak_handle.h [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/driver/about_sync_util.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/driver/data_type_manager_impl.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/driver/glue/sync_backend_host_impl.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/driver/sync_service_base.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/driver/sync_service_crypto.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/engine/engine_util.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/engine/net/http_bridge.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/engine/sync_backend_registrar.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/engine_impl/apply_control_data_updates.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/engine_impl/commit_util.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/engine_impl/conflict_resolver.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/engine_impl/get_commit_ids.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/engine_impl/loopback_server/loopback_server.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/engine_impl/loopback_server/loopback_server_entity.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/engine_impl/loopback_server/persistent_bookmark_entity.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/engine_impl/loopback_server/persistent_permanent_entity.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/engine_impl/loopback_server/persistent_tombstone_entity.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/engine_impl/sync_manager_impl.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/engine_impl/sync_scheduler_impl.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/engine_impl/syncer_proto_util.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/engine_impl/syncer_util.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/model/sync_error.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/syncable/change_reorder_buffer.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/syncable/directory.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/syncable/model_neutral_mutable_entry.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/syncable/model_type.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/syncable/mutable_entry.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/syncable/syncable_delete_journal.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/syncable/syncable_unittest.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/syncable/syncable_util.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/syncable/write_node.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/test/engine/mock_connection_manager.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/test/engine/mock_connection_manager.h [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/test/engine/test_directory_setter_upper.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/test/engine/test_syncable_utils.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/test/fake_server/fake_server.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/tools/sync_client.cc [modify] https://crrev.com/a8b1707bb6ceec193bfa312308c2c6041e8629ee/components/sync/tools/sync_listen_notifications.cc
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/eed259fab22009019b19e0550f27f9e1e8ca28f7 commit eed259fab22009019b19e0550f27f9e1e8ca28f7 Author: Sky Malice <skym@chromium.org> Date: Sat Nov 18 01:28:30 2017 [Sync] Remove entity DCHECKs from LoopbackServer. Per guidance at about data coming from untrusted sources, such as disk or network, removed DCHECKs when handling data Loopback storage, see https://groups.google.com/a/chromium.org/d/msg/chromium-dev/LU6NWiaSSRc/oolO8fg5AAAJ Instead null entities are returned which are essentailly just dropped. Also includes some minor cleanup, including deletion of un-used fake_server_entity.cc file, consistent usage of un-templated WrapUnique, nullptr for empty unique_ptrs, and pushed test constant back to test file (single_client_wallet_sync_test.cc). Bug: 771811 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs Change-Id: I73045ed11982e6c44d5b623855163e61c1961fa2 Reviewed-on: https://chromium-review.googlesource.com/713556 Reviewed-by: Steven Holte <holte@chromium.org> Reviewed-by: Mike Baxley <baxley@chromium.org> Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org> Commit-Queue: Sky Malice <skym@chromium.org> Cr-Commit-Position: refs/heads/master@{#517667} [modify] https://crrev.com/eed259fab22009019b19e0550f27f9e1e8ca28f7/chrome/browser/sync/test/integration/single_client_wallet_sync_test.cc [modify] https://crrev.com/eed259fab22009019b19e0550f27f9e1e8ca28f7/components/sync/BUILD.gn [modify] https://crrev.com/eed259fab22009019b19e0550f27f9e1e8ca28f7/components/sync/engine_impl/loopback_server/loopback_server.cc [modify] https://crrev.com/eed259fab22009019b19e0550f27f9e1e8ca28f7/components/sync/engine_impl/loopback_server/loopback_server.h [modify] https://crrev.com/eed259fab22009019b19e0550f27f9e1e8ca28f7/components/sync/engine_impl/loopback_server/loopback_server_entity.cc [modify] https://crrev.com/eed259fab22009019b19e0550f27f9e1e8ca28f7/components/sync/engine_impl/loopback_server/loopback_server_entity.h [modify] https://crrev.com/eed259fab22009019b19e0550f27f9e1e8ca28f7/components/sync/engine_impl/loopback_server/loopback_server_unittest.cc [modify] https://crrev.com/eed259fab22009019b19e0550f27f9e1e8ca28f7/components/sync/engine_impl/loopback_server/persistent_bookmark_entity.cc [add] https://crrev.com/eed259fab22009019b19e0550f27f9e1e8ca28f7/components/sync/engine_impl/loopback_server/persistent_bookmark_entity_unittest.cc [modify] https://crrev.com/eed259fab22009019b19e0550f27f9e1e8ca28f7/components/sync/engine_impl/loopback_server/persistent_permanent_entity.cc [add] https://crrev.com/eed259fab22009019b19e0550f27f9e1e8ca28f7/components/sync/engine_impl/loopback_server/persistent_permanent_entity_unittest.cc [modify] https://crrev.com/eed259fab22009019b19e0550f27f9e1e8ca28f7/components/sync/engine_impl/loopback_server/persistent_tombstone_entity.cc [modify] https://crrev.com/eed259fab22009019b19e0550f27f9e1e8ca28f7/components/sync/engine_impl/loopback_server/persistent_tombstone_entity.h [add] https://crrev.com/eed259fab22009019b19e0550f27f9e1e8ca28f7/components/sync/engine_impl/loopback_server/persistent_tombstone_entity_unittest.cc [modify] https://crrev.com/eed259fab22009019b19e0550f27f9e1e8ca28f7/components/sync/engine_impl/loopback_server/persistent_unique_client_entity.cc [modify] https://crrev.com/eed259fab22009019b19e0550f27f9e1e8ca28f7/components/sync/engine_impl/loopback_server/persistent_unique_client_entity.h [add] https://crrev.com/eed259fab22009019b19e0550f27f9e1e8ca28f7/components/sync/engine_impl/loopback_server/persistent_unique_client_entity_unittest.cc [modify] https://crrev.com/eed259fab22009019b19e0550f27f9e1e8ca28f7/components/sync/test/fake_server/android/fake_server_helper_android.cc [delete] https://crrev.com/180875cb9ca35008fc99046e78a5246989b884a2/components/sync/test/fake_server/fake_server_entity.cc [modify] https://crrev.com/eed259fab22009019b19e0550f27f9e1e8ca28f7/ios/chrome/test/app/sync_test_util.mm [modify] https://crrev.com/eed259fab22009019b19e0550f27f9e1e8ca28f7/tools/metrics/histograms/enums.xml [modify] https://crrev.com/eed259fab22009019b19e0550f27f9e1e8ca28f7/tools/metrics/histograms/histograms.xml
Comment 1 by bugdroid1@chromium.org
, Oct 11 2017