Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(264)

Issue 2269103003: [Sync] Split error handling code out of DataTypeControllers. (Closed)

Created:
4 years, 4 months ago by maxbogue
Modified:
4 years, 3 months ago
Reviewers:
skym, pavely
CC:
chromium-reviews, rouslan+autofill_chromium.org, browser-components-watch_chromium.org, jdonnelly+autofillwatch_chromium.org, vabr+watchlistautofill_chromium.org, estade+watch_chromium.org, sync-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[Sync] Split error handling code out of DataTypeControllers. - Move DataTypeErrorHandler* to //components/sync/api - Add DataTypeErrorHandlerImpl - Add DataTypeController::CreateErrorHandler - Factor some shared logic out of DTCs into DTEHI - Pass DataTypeErrorHandler as a unique_ptr instead of raw pointer Once this patch lands, the final step will be converting DTCs to be NonThreadSafe and SupportsWeakPtr, instead of RefCounted. BUG=638771

Patch Set 1 #

Patch Set 2 : Self-review + fix tests. #

Patch Set 3 : Fix build issue. #

Total comments: 31

Patch Set 4 : Address comments. #

Patch Set 5 : Try fixing Arc. #

Patch Set 6 : Fix most tests. #

Patch Set 7 : Better comment for DTEHI ui_thread_ member. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+542 lines, -559 lines) Patch
M chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc View 1 2 3 4 2 chunks +2 lines, -2 lines 0 comments Download
M components/autofill/core/browser/autofill_wallet_data_type_controller.cc View 1 1 chunk +4 lines, -8 lines 0 comments Download
M components/autofill/core/browser/webdata/autofill_data_type_controller_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M components/browser_sync/browser/profile_sync_components_factory_impl.h View 2 chunks +2 lines, -1 line 0 comments Download
M components/browser_sync/browser/profile_sync_components_factory_impl.cc View 3 chunks +6 lines, -6 lines 0 comments Download
M components/browser_sync/browser/profile_sync_service_bookmark_unittest.cc View 1 5 chunks +12 lines, -7 lines 0 comments Download
M components/browser_sync/browser/profile_sync_service_typed_url_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M components/history/core/browser/history_delete_directives_data_type_controller.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M components/history/core/browser/typed_url_data_type_controller.cc View 1 2 chunks +4 lines, -2 lines 0 comments Download
M components/search_engines/BUILD.gn View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M components/search_engines/search_engine_data_type_controller_unittest.cc View 1 2 chunks +2 lines, -1 line 0 comments Download
M components/sync/BUILD.gn View 1 2 3 4 4 chunks +5 lines, -3 lines 0 comments Download
A + components/sync/api/data_type_error_handler.h View 1 2 chunks +12 lines, -8 lines 0 comments Download
A components/sync/api/data_type_error_handler_impl.h View 1 2 3 4 5 6 1 chunk +53 lines, -0 lines 0 comments Download
A components/sync/api/data_type_error_handler_impl.cc View 1 1 chunk +46 lines, -0 lines 0 comments Download
A + components/sync/api/data_type_error_handler_mock.h View 1 2 chunks +9 lines, -7 lines 0 comments Download
A + components/sync/api/data_type_error_handler_mock.cc View 1 3 chunks +6 lines, -3 lines 0 comments Download
M components/sync/api/fake_model_type_change_processor.h View 2 chunks +4 lines, -2 lines 0 comments Download
M components/sync/api/fake_model_type_change_processor.cc View 1 chunk +1 line, -1 line 0 comments Download
M components/sync/api/model_type_change_processor.h View 1 2 chunks +4 lines, -3 lines 0 comments Download
M components/sync/api/model_type_service.h View 1 2 chunks +4 lines, -6 lines 0 comments Download
M components/sync/api/model_type_service.cc View 2 chunks +5 lines, -3 lines 0 comments Download
M components/sync/api/model_type_service_unittest.cc View 3 chunks +4 lines, -4 lines 0 comments Download
D components/sync/core/data_type_error_handler.h View 1 chunk +0 lines, -38 lines 0 comments Download
M components/sync/core/shared_model_type_processor.h View 3 chunks +5 lines, -4 lines 0 comments Download
M components/sync/core/shared_model_type_processor.cc View 1 6 chunks +6 lines, -6 lines 0 comments Download
M components/sync/core/shared_model_type_processor_unittest.cc View 6 chunks +11 lines, -4 lines 0 comments Download
D components/sync/core/test/data_type_error_handler_mock.h View 1 chunk +0 lines, -38 lines 0 comments Download
M components/sync/core_impl/model_type_connector_proxy_unittest.cc View 3 chunks +2 lines, -3 lines 0 comments Download
D components/sync/core_impl/test/data_type_error_handler_mock.cc View 1 chunk +0 lines, -35 lines 0 comments Download
M components/sync/device_info/device_info_data_type_controller_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M components/sync/device_info/device_info_service_unittest.cc View 3 chunks +3 lines, -5 lines 0 comments Download
M components/sync/driver/change_processor.h View 3 chunks +6 lines, -3 lines 0 comments Download
M components/sync/driver/change_processor.cc View 4 5 2 chunks +6 lines, -3 lines 0 comments Download
M components/sync/driver/change_processor_mock.h View 1 2 chunks +3 lines, -9 lines 0 comments Download
M components/sync/driver/change_processor_mock.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M components/sync/driver/data_type_controller.h View 1 2 3 6 chunks +8 lines, -13 lines 0 comments Download
M components/sync/driver/data_type_controller.cc View 1 1 chunk +0 lines, -10 lines 0 comments Download
M components/sync/driver/data_type_controller_mock.h View 1 chunk +1 line, -1 line 0 comments Download
M components/sync/driver/data_type_manager_impl_unittest.cc View 1 2 chunks +5 lines, -1 line 0 comments Download
M components/sync/driver/fake_data_type_controller.h View 2 chunks +2 lines, -2 lines 0 comments Download
M components/sync/driver/fake_data_type_controller.cc View 3 chunks +10 lines, -6 lines 0 comments Download
M components/sync/driver/fake_generic_change_processor.h View 2 chunks +2 lines, -1 line 0 comments Download
M components/sync/driver/fake_generic_change_processor.cc View 1 chunk +1 line, -1 line 0 comments Download
M components/sync/driver/frontend_data_type_controller.h View 1 2 3 4 chunks +6 lines, -12 lines 0 comments Download
M components/sync/driver/frontend_data_type_controller.cc View 5 chunks +17 lines, -29 lines 0 comments Download
M components/sync/driver/frontend_data_type_controller_mock.h View 1 2 chunks +0 lines, -4 lines 0 comments Download
M components/sync/driver/frontend_data_type_controller_unittest.cc View 1 5 chunks +12 lines, -23 lines 0 comments Download
M components/sync/driver/generic_change_processor.h View 1 3 chunks +2 lines, -2 lines 0 comments Download
M components/sync/driver/generic_change_processor.cc View 1 16 chunks +33 lines, -33 lines 0 comments Download
M components/sync/driver/generic_change_processor_factory.h View 1 2 chunks +2 lines, -2 lines 0 comments Download
M components/sync/driver/generic_change_processor_factory.cc View 2 chunks +5 lines, -3 lines 0 comments Download
M components/sync/driver/generic_change_processor_unittest.cc View 5 chunks +3 lines, -4 lines 0 comments Download
M components/sync/driver/model_association_manager.h View 1 chunk +2 lines, -1 line 0 comments Download
M components/sync/driver/model_association_manager.cc View 1 chunk +3 lines, -2 lines 0 comments Download
M components/sync/driver/model_association_manager_unittest.cc View 1 3 chunks +7 lines, -2 lines 0 comments Download
M components/sync/driver/non_blocking_data_type_controller.h View 2 chunks +3 lines, -7 lines 0 comments Download
M components/sync/driver/non_blocking_data_type_controller.cc View 4 chunks +13 lines, -26 lines 0 comments Download
M components/sync/driver/non_blocking_data_type_controller_unittest.cc View 2 chunks +2 lines, -1 line 0 comments Download
M components/sync/driver/non_ui_data_type_controller.h View 3 chunks +3 lines, -2 lines 0 comments Download
M components/sync/driver/non_ui_data_type_controller.cc View 5 chunks +14 lines, -20 lines 0 comments Download
M components/sync/driver/non_ui_data_type_controller_mock.h View 1 2 3 1 chunk +0 lines, -2 lines 0 comments Download
M components/sync/driver/non_ui_data_type_controller_unittest.cc View 1 13 chunks +28 lines, -33 lines 0 comments Download
M components/sync/driver/non_ui_model_type_controller_unittest.cc View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M components/sync/driver/proxy_data_type_controller.h View 2 chunks +3 lines, -4 lines 0 comments Download
M components/sync/driver/proxy_data_type_controller.cc View 1 chunk +6 lines, -5 lines 0 comments Download
M components/sync/driver/shared_change_processor.h View 1 6 chunks +9 lines, -7 lines 0 comments Download
M components/sync/driver/shared_change_processor.cc View 8 chunks +10 lines, -10 lines 0 comments Download
M components/sync/driver/shared_change_processor_unittest.cc View 4 chunks +4 lines, -4 lines 0 comments Download
M components/sync/driver/sync_api_component_factory.h View 1 3 chunks +2 lines, -2 lines 0 comments Download
M components/sync/driver/sync_api_component_factory_mock.h View 2 chunks +5 lines, -8 lines 0 comments Download
M components/sync/driver/sync_api_component_factory_mock.cc View 2 chunks +7 lines, -3 lines 0 comments Download
M components/sync/driver/ui_data_type_controller.h View 3 chunks +4 lines, -4 lines 0 comments Download
M components/sync/driver/ui_data_type_controller.cc View 3 chunks +12 lines, -10 lines 0 comments Download
M components/sync/driver/ui_data_type_controller_unittest.cc View 1 2 3 4 5 chunks +22 lines, -4 lines 0 comments Download
M components/sync/driver/ui_model_type_controller_unittest.cc View 2 chunks +4 lines, -1 line 0 comments Download
M components/sync_bookmarks/bookmark_change_processor.h View 3 chunks +6 lines, -4 lines 0 comments Download
M components/sync_bookmarks/bookmark_change_processor.cc View 1 2 3 4 5 11 chunks +13 lines, -13 lines 0 comments Download
M components/sync_bookmarks/bookmark_data_type_controller.cc View 1 chunk +1 line, -1 line 0 comments Download
M components/sync_bookmarks/bookmark_data_type_controller_unittest.cc View 3 chunks +0 lines, -3 lines 0 comments Download
M components/sync_bookmarks/bookmark_model_associator.h View 4 chunks +4 lines, -3 lines 0 comments Download
M components/sync_bookmarks/bookmark_model_associator.cc View 1 5 chunks +6 lines, -6 lines 0 comments Download
M components/sync_sessions/session_data_type_controller.cc View 1 2 chunks +3 lines, -1 line 0 comments Download
M components/sync_sessions/session_data_type_controller_unittest.cc View 2 chunks +3 lines, -1 line 0 comments Download
M components/sync_sessions/sessions_sync_manager.h View 1 1 chunk +0 lines, -1 line 0 comments Download

Dependent Patchsets:

Messages

Total messages: 34 (27 generated)
maxbogue
Pavel, Sky, PTAL!
4 years, 3 months ago (2016-08-24 21:00:02 UTC) #12
skym
https://codereview.chromium.org/2269103003/diff/40001/components/sync/BUILD.gn File components/sync/BUILD.gn (right): https://codereview.chromium.org/2269103003/diff/40001/components/sync/BUILD.gn#newcode25 components/sync/BUILD.gn:25: "api/data_type_error_handler.h", This should be in base/ https://codereview.chromium.org/2269103003/diff/40001/components/sync/BUILD.gn#newcode676 components/sync/BUILD.gn:676: "api/data_type_error_handler_mock.cc", ...
4 years, 3 months ago (2016-08-24 23:21:32 UTC) #13
pavely
lgtm https://codereview.chromium.org/2269103003/diff/40001/components/sync/driver/frontend_data_type_controller.h File components/sync/driver/frontend_data_type_controller.h (right): https://codereview.chromium.org/2269103003/diff/40001/components/sync/driver/frontend_data_type_controller.h#newcode88 components/sync/driver/frontend_data_type_controller.h:88: virtual void OnUnrecoverableError(const syncer::SyncError& error); Does OnUnrecoverableError need ...
4 years, 3 months ago (2016-08-25 01:31:55 UTC) #16
maxbogue
https://codereview.chromium.org/2269103003/diff/40001/components/sync/BUILD.gn File components/sync/BUILD.gn (right): https://codereview.chromium.org/2269103003/diff/40001/components/sync/BUILD.gn#newcode25 components/sync/BUILD.gn:25: "api/data_type_error_handler.h", On 2016/08/24 23:21:32, skym wrote: > This should ...
4 years, 3 months ago (2016-08-25 23:52:09 UTC) #19
skym
https://codereview.chromium.org/2269103003/diff/40001/components/sync/api/data_type_error_handler_impl.cc File components/sync/api/data_type_error_handler_impl.cc (right): https://codereview.chromium.org/2269103003/diff/40001/components/sync/api/data_type_error_handler_impl.cc#newcode28 components/sync/api/data_type_error_handler_impl.cc:28: ui_thread_->PostTask(error.location(), base::Bind(sync_callback_, error)); On 2016/08/25 23:52:08, maxbogue wrote: > ...
4 years, 3 months ago (2016-08-26 15:46:30 UTC) #22
maxbogue
PTAL. This change introduces a ref-cycle between DTC and SharedChangeProcessor that will become obsolete with ...
4 years, 3 months ago (2016-08-30 18:16:26 UTC) #30
maxbogue
4 years, 3 months ago (2016-09-07 00:12:25 UTC) #34
This was landed as part of http://crrev.com/2289143003.

Powered by Google App Engine
This is Rietveld 408576698