Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2723953002:
[sync] skeleton implementation of TypedURLSyncBridge (Closed)
|
Created:
3 years, 9 months ago by Gang Wu Modified:
3 years, 8 months ago CC:
chromium-reviews, marq+watch_chromium.org, pkl (ping after 24h if needed), noyau+watch_chromium.org, sync-reviews_chromium.org, sdefresne+watch_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
Description[sync] skeleton implementation of TypedURLSyncBridge
TypedURLSyncBridge is the USS(unified sync and storage) equivalent of
the TypedUrlSyncableService.
This CL is for adding TypedURLSyncBridge class but not implement any
function yet.
This CL is Also addressing missing parts for bug 558320. CL is
https://codereview.chromium.org/1647833006
BUG=697960
Review-Url: https://codereview.chromium.org/2723953002
Cr-Commit-Position: refs/heads/master@{#461901}
Committed: https://chromium.googlesource.com/chromium/src/+/69ae3f7272abd5f3c8b5da163916396a3c2f83e4
Patch Set 1 #
Total comments: 12
Patch Set 2 : skym review #Patch Set 3 : rebase and address missing parts for bug 558320 #
Total comments: 16
Patch Set 4 : skym review #Patch Set 5 : rebase #Messages
Total messages: 72 (61 generated)
The CQ bit was checked by gangwu@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
Description was changed from ========== for IOS done controller added remove unneeded DEPS first save deps try DEPS issue self review typo update comments add unittests BUG= ========== to ========== [sync] skeleton implementation of TypedURLSyncBridge TypedURLSyncBridge is the USS(unified sync and storage) equivalent of the TypedUrlSyncableService. The goal is for identical functionality, but with reduced memory usage due to unifying the storage of sync metadata and typed url data. BUG=697960 ==========
gangwu@chromium.org changed reviewers: + brettw@chromium.org
The CQ bit was checked by gangwu@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
The CQ bit was checked by gangwu@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...) win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...) win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by gangwu@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #2 (id:60001) has been deleted
Patchset #1 (id:40001) has been deleted
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
gangwu@chromium.org changed reviewers: + skym@chromium.org - brettw@chromium.org
PTAL
I'm worried about thread correctness in the controller. Would be nice if you could explain how it should work any why that's safe. https://codereview.chromium.org/2723953002/diff/80001/chrome/browser/sync/chr... File chrome/browser/sync/chrome_sync_client.cc (right): https://codereview.chromium.org/2723953002/diff/80001/chrome/browser/sync/chr... chrome/browser/sync/chrome_sync_client.cc:461: // codepath is executed on backend thread while HistoryServiceFactory I'm not sure I agree with you that the way the code is currently set up in this CL, will actually call this on anything other than the UI thread. https://codereview.chromium.org/2723953002/diff/80001/components/browser_sync... File components/browser_sync/profile_sync_components_factory_impl.cc (right): https://codereview.chromium.org/2723953002/diff/80001/components/browser_sync... components/browser_sync/profile_sync_components_factory_impl.cc:215: sync_client_, db_thread_)); db_thread_ ? Are you sure that's right? I don't see any direct references to the db thread in the existing typed urls code. https://codereview.chromium.org/2723953002/diff/80001/components/history/core... File components/history/core/browser/history_backend.h (right): https://codereview.chromium.org/2723953002/diff/80001/components/history/core... components/history/core/browser/history_backend.h:401: TypedURLSyncBridge* GetTypedURLSyncBridge() const; Why is GetTypedUrlSyncableService() virtual and this one not? Are we going to have to slap virtual on this to make unit tests work? https://codereview.chromium.org/2723953002/diff/80001/components/history/core... components/history/core/browser/history_backend.h:896: // depending on switches::kSyncUSSAutocomplete. Defined after observers_ autocomplete? https://codereview.chromium.org/2723953002/diff/80001/components/history/core... File components/history/core/browser/typed_url_sync_bridge.h (right): https://codereview.chromium.org/2723953002/diff/80001/components/history/core... components/history/core/browser/typed_url_sync_bridge.h:36: // "autofill_entry|$name|$value" where $name and $value are escaped. This is an autocomplete comment, and is not applicable here. https://codereview.chromium.org/2723953002/diff/80001/components/history/core... components/history/core/browser/typed_url_sync_bridge.h:40: // where $name and $value are escaped. Same
The CQ bit was checked by gangwu@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...) ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
The CQ bit was checked by gangwu@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
Description was changed from ========== [sync] skeleton implementation of TypedURLSyncBridge TypedURLSyncBridge is the USS(unified sync and storage) equivalent of the TypedUrlSyncableService. The goal is for identical functionality, but with reduced memory usage due to unifying the storage of sync metadata and typed url data. BUG=697960 ========== to ========== [sync] skeleton implementation of TypedURLSyncBridge TypedURLSyncBridge is the USS(unified sync and storage) equivalent of the TypedUrlSyncableService. The goal is for identical functionality, but with reduced memory usage due to unifying the storage of sync metadata and typed url data. BUG=697960 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation ==========
Description was changed from ========== [sync] skeleton implementation of TypedURLSyncBridge TypedURLSyncBridge is the USS(unified sync and storage) equivalent of the TypedUrlSyncableService. The goal is for identical functionality, but with reduced memory usage due to unifying the storage of sync metadata and typed url data. BUG=697960 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation ========== to ========== [sync] skeleton implementation of TypedURLSyncBridge TypedURLSyncBridge is the USS(unified sync and storage) equivalent of the TypedUrlSyncableService. The goal is for identical functionality, but with reduced memory usage due to unifying the storage of sync metadata and typed url data. BUG=697960 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_site_isolation ==========
Patchset #3 (id:120001) has been deleted
Patchset #3 (id:140001) has been deleted
The CQ bit was checked by gangwu@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #3 (id:160001) has been deleted
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
This update also added missing part for cl https://codereview.chromium.org/1647833006/ https://codereview.chromium.org/2723953002/diff/80001/chrome/browser/sync/chr... File chrome/browser/sync/chrome_sync_client.cc (right): https://codereview.chromium.org/2723953002/diff/80001/chrome/browser/sync/chr... chrome/browser/sync/chrome_sync_client.cc:461: // codepath is executed on backend thread while HistoryServiceFactory On 2017/03/03 17:41:09, skym wrote: > I'm not sure I agree with you that the way the code is currently set up in this > CL, will actually call this on anything other than the UI thread. Done. https://codereview.chromium.org/2723953002/diff/80001/components/browser_sync... File components/browser_sync/profile_sync_components_factory_impl.cc (right): https://codereview.chromium.org/2723953002/diff/80001/components/browser_sync... components/browser_sync/profile_sync_components_factory_impl.cc:215: sync_client_, db_thread_)); On 2017/03/03 17:41:09, skym wrote: > db_thread_ ? Are you sure that's right? I don't see any direct references to the > db thread in the existing typed urls code. will add real implementation later, so add TODO here. https://codereview.chromium.org/2723953002/diff/80001/components/history/core... File components/history/core/browser/history_backend.h (right): https://codereview.chromium.org/2723953002/diff/80001/components/history/core... components/history/core/browser/history_backend.h:401: TypedURLSyncBridge* GetTypedURLSyncBridge() const; On 2017/03/03 17:41:09, skym wrote: > Why is GetTypedUrlSyncableService() virtual and this one not? Are we going to > have to slap virtual on this to make unit tests work? I am not going to add unittest in this CL, so did not add virtual in this CL. I will add virtual when unittest needed. https://codereview.chromium.org/2723953002/diff/80001/components/history/core... components/history/core/browser/history_backend.h:896: // depending on switches::kSyncUSSAutocomplete. Defined after observers_ On 2017/03/03 17:41:09, skym wrote: > autocomplete? Done. https://codereview.chromium.org/2723953002/diff/80001/components/history/core... File components/history/core/browser/typed_url_sync_bridge.h (right): https://codereview.chromium.org/2723953002/diff/80001/components/history/core... components/history/core/browser/typed_url_sync_bridge.h:36: // "autofill_entry|$name|$value" where $name and $value are escaped. On 2017/03/03 17:41:09, skym wrote: > This is an autocomplete comment, and is not applicable here. Done. https://codereview.chromium.org/2723953002/diff/80001/components/history/core... components/history/core/browser/typed_url_sync_bridge.h:40: // where $name and $value are escaped. On 2017/03/03 17:41:09, skym wrote: > Same Done.
lgtm https://codereview.chromium.org/2723953002/diff/180001/chrome/browser/sync/ch... File chrome/browser/sync/chrome_sync_client.cc (right): https://codereview.chromium.org/2723953002/diff/180001/chrome/browser/sync/ch... chrome/browser/sync/chrome_sync_client.cc:500: history::HistoryService* history = HistoryServiceFactory::GetForProfile( So, every other place we access the HistoryService in this file uses EXPLICIT_ACCESS. It is odd at this place is different. My understanding is that we haven't gotten to the bottom of how this should work. I'd prefer you to add a TODO and just return an empty weak ptr until we get to the bottom of this, similar to what you did in profile_sync_components_factory_impl.cc https://codereview.chromium.org/2723953002/diff/180001/components/history/cor... File components/history/core/browser/history_backend.cc (right): https://codereview.chromium.org/2723953002/diff/180001/components/history/cor... components/history/core/browser/history_backend.cc:225: base::BindRepeating(base::IgnoreResult(&DumpWithoutCrashing)))); Ugh. Okay, this is kind of a land mine that we're setting ourselves up for. Typically we want to report 1% of errors on canary/dev. But this reports 100% on all channels. We ultimately need to thread channel information into here similar to what Max proposed in https://codereview.chromium.org/2647113002/. I really wish we had a more convenient way of solving this. Can you at least throw a big TODO in here explaining that this is mainly for initial testing and needs to be fixed before launch? https://codereview.chromium.org/2723953002/diff/180001/components/history/cor... File components/history/core/browser/history_backend.h (right): https://codereview.chromium.org/2723953002/diff/180001/components/history/cor... components/history/core/browser/history_backend.h:897: // it unregisters itself as observer during destruction. Can you add a small TODO to the bridge to make sure we don't forget to add this? https://codereview.chromium.org/2723953002/diff/180001/components/history/cor... File components/history/core/browser/history_service.h (right): https://codereview.chromium.org/2723953002/diff/180001/components/history/cor... components/history/core/browser/history_service.h:36: #include "components/history/core/browser/typed_url_sync_bridge.h" Is there a reason this file needs either the bridge or syncable service's .h file to be included? Why not forward declare? https://codereview.chromium.org/2723953002/diff/180001/components/history/cor... File components/history/core/browser/typed_url_sync_bridge.h (right): https://codereview.chromium.org/2723953002/diff/180001/components/history/cor... components/history/core/browser/typed_url_sync_bridge.h:35: // be the url (URLRow::url()::spec()). I'm still finding these comments kind of odd. I should have pushed back on these from autocomplete's bridge's .h. "This is used to identify the entity on the server" is not strictly true either. What do you think of moving this to the definition of the method in the .cc and changing to something like // Must be exactly the value of GURL::spec() for backwards comparability with the previous (Directory + SyncableService) iteration of sync integration. This can be large but it is assumed that this is not held in memory at steady state. https://codereview.chromium.org/2723953002/diff/180001/components/history/cor... components/history/core/browser/typed_url_sync_bridge.h:37: // Generate a string key uniquely identifying |entity_data| in the context of Same as above, lets move to .cc, and what do you think of the following text: // Prefer to use URLRow::id() to uniquely identify entities when coordinating with sync because it has a significantly low memory cost then a URL. https://codereview.chromium.org/2723953002/diff/180001/components/history/cor... components/history/core/browser/typed_url_sync_bridge.h:60: base::SequenceChecker sequence_checker_; Can you add a comment about why we're using a sequence checker and not a thread checker?
Also, can you update the description of this CL to make it clear exactly what's being added in this CL and what is not?
https://codereview.chromium.org/2723953002/diff/180001/components/history/cor... File components/history/core/browser/typed_url_sync_bridge.h (right): https://codereview.chromium.org/2723953002/diff/180001/components/history/cor... components/history/core/browser/typed_url_sync_bridge.h:37: // Generate a string key uniquely identifying |entity_data| in the context of On 2017/03/07 18:08:04, skym wrote: > Same as above, lets move to .cc, and what do you think of the following text: > > // Prefer to use URLRow::id() to uniquely identify entities when coordinating > with sync because it has a significantly low memory cost then a URL. This returns a string right.. Are we representing this number as base-10? Or something else more efficient?
https://codereview.chromium.org/2723953002/diff/180001/components/history/cor... File components/history/core/browser/typed_url_sync_bridge.h (right): https://codereview.chromium.org/2723953002/diff/180001/components/history/cor... components/history/core/browser/typed_url_sync_bridge.h:35: // be the url (URLRow::url()::spec()). On 2017/03/07 18:08:04, skym wrote: > I'm still finding these comments kind of odd. I should have pushed back on these > from autocomplete's bridge's .h. "This is used to identify the entity on the > server" is not strictly true either. > > What do you think of moving this to the definition of the method in the .cc and > changing to something like > > // Must be exactly the value of GURL::spec() for backwards comparability with > the previous (Directory + SyncableService) iteration of sync integration. This > can be large but it is assumed that this is not held in memory at steady state. I just posted a CL to make autocomplete's comments a little better and more inline of how i'm suggesting these comments should look. https://codereview.chromium.org/2737023002/
The CQ bit was checked by gangwu@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== [sync] skeleton implementation of TypedURLSyncBridge TypedURLSyncBridge is the USS(unified sync and storage) equivalent of the TypedUrlSyncableService. The goal is for identical functionality, but with reduced memory usage due to unifying the storage of sync metadata and typed url data. BUG=697960 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_site_isolation ========== to ========== [sync] skeleton implementation of TypedURLSyncBridge TypedURLSyncBridge is the USS(unified sync and storage) equivalent of the TypedUrlSyncableService. This CL is for adding TypedURLSyncBridge class but not implement any function yet. BUG=697960 ==========
Patchset #5 (id:220001) has been deleted
Description was changed from ========== [sync] skeleton implementation of TypedURLSyncBridge TypedURLSyncBridge is the USS(unified sync and storage) equivalent of the TypedUrlSyncableService. This CL is for adding TypedURLSyncBridge class but not implement any function yet. BUG=697960 ========== to ========== [sync] skeleton implementation of TypedURLSyncBridge TypedURLSyncBridge is the USS(unified sync and storage) equivalent of the TypedUrlSyncableService. This CL is for adding TypedURLSyncBridge class but not implement any function yet. This CL is Also addressing missing parts for bug 558320. CL is https://codereview.chromium.org/1647833006 BUG=697960 ==========
Patchset #5 (id:240001) has been deleted
The CQ bit was checked by gangwu@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
gangwu@chromium.org changed reviewers: + brettw@chromium.org
hi brettw, PTAL https://codereview.chromium.org/2723953002/diff/180001/chrome/browser/sync/ch... File chrome/browser/sync/chrome_sync_client.cc (right): https://codereview.chromium.org/2723953002/diff/180001/chrome/browser/sync/ch... chrome/browser/sync/chrome_sync_client.cc:500: history::HistoryService* history = HistoryServiceFactory::GetForProfile( On 2017/03/07 18:08:04, skym wrote: > So, every other place we access the HistoryService in this file uses > EXPLICIT_ACCESS. It is odd at this place is different. My understanding is that > we haven't gotten to the bottom of how this should work. > > I'd prefer you to add a TODO and just return an empty weak ptr until we get to > the bottom of this, similar to what you did in > profile_sync_components_factory_impl.cc Done. https://codereview.chromium.org/2723953002/diff/180001/components/history/cor... File components/history/core/browser/history_backend.cc (right): https://codereview.chromium.org/2723953002/diff/180001/components/history/cor... components/history/core/browser/history_backend.cc:225: base::BindRepeating(base::IgnoreResult(&DumpWithoutCrashing)))); On 2017/03/07 18:08:04, skym wrote: > Ugh. Okay, this is kind of a land mine that we're setting ourselves up for. > Typically we want to report 1% of errors on canary/dev. But this reports 100% on > all channels. We ultimately need to thread channel information into here similar > to what Max proposed in https://codereview.chromium.org/2647113002/. I really > wish we had a more convenient way of solving this. > > Can you at least throw a big TODO in here explaining that this is mainly for > initial testing and needs to be fixed before launch? Done. https://codereview.chromium.org/2723953002/diff/180001/components/history/cor... File components/history/core/browser/history_backend.h (right): https://codereview.chromium.org/2723953002/diff/180001/components/history/cor... components/history/core/browser/history_backend.h:897: // it unregisters itself as observer during destruction. On 2017/03/07 18:08:04, skym wrote: > Can you add a small TODO to the bridge to make sure we don't forget to add this? Done. https://codereview.chromium.org/2723953002/diff/180001/components/history/cor... File components/history/core/browser/history_service.h (right): https://codereview.chromium.org/2723953002/diff/180001/components/history/cor... components/history/core/browser/history_service.h:36: #include "components/history/core/browser/typed_url_sync_bridge.h" On 2017/03/07 18:08:04, skym wrote: > Is there a reason this file needs either the bridge or syncable service's .h > file to be included? Why not forward declare? chrome_sync_client.cc call history->GetTypedUrlSyncableService()->AsWeakPtr(), for ->AsWeakPtr() function we need to include .h file here. https://codereview.chromium.org/2723953002/diff/180001/components/history/cor... File components/history/core/browser/typed_url_sync_bridge.h (right): https://codereview.chromium.org/2723953002/diff/180001/components/history/cor... components/history/core/browser/typed_url_sync_bridge.h:35: // be the url (URLRow::url()::spec()). On 2017/03/07 19:25:22, skym wrote: > On 2017/03/07 18:08:04, skym wrote: > > I'm still finding these comments kind of odd. I should have pushed back on > these > > from autocomplete's bridge's .h. "This is used to identify the entity on the > > server" is not strictly true either. > > > > What do you think of moving this to the definition of the method in the .cc > and > > changing to something like > > > > // Must be exactly the value of GURL::spec() for backwards comparability with > > the previous (Directory + SyncableService) iteration of sync integration. This > > can be large but it is assumed that this is not held in memory at steady > state. > > I just posted a CL to make autocomplete's comments a little better and more > inline of how i'm suggesting these comments should look. > https://codereview.chromium.org/2737023002/ Done. https://codereview.chromium.org/2723953002/diff/180001/components/history/cor... components/history/core/browser/typed_url_sync_bridge.h:37: // Generate a string key uniquely identifying |entity_data| in the context of On 2017/03/07 18:53:51, skym wrote: > On 2017/03/07 18:08:04, skym wrote: > > Same as above, lets move to .cc, and what do you think of the following text: > > > > // Prefer to use URLRow::id() to uniquely identify entities when coordinating > > with sync because it has a significantly low memory cost then a URL. > > This returns a string right.. Are we representing this number as base-10? Or > something else more efficient? base-10 https://codereview.chromium.org/2723953002/diff/180001/components/history/cor... components/history/core/browser/typed_url_sync_bridge.h:60: base::SequenceChecker sequence_checker_; On 2017/03/07 18:08:04, skym wrote: > Can you add a comment about why we're using a sequence checker and not a thread > checker? comment added, more detail here. https://codereview.chromium.org/2591643004
lgtm
The CQ bit was checked by gangwu@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by gangwu@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from skym@chromium.org Link to the patchset: https://codereview.chromium.org/2723953002/#ps260001 (title: "rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 260001, "attempt_start_ts": 1491349063790150,
"parent_rev": "304189c3d5089ead05a2dfb3360e141d2b92eda0", "commit_rev":
"69ae3f7272abd5f3c8b5da163916396a3c2f83e4"}
Message was sent while issue was closed.
Description was changed from ========== [sync] skeleton implementation of TypedURLSyncBridge TypedURLSyncBridge is the USS(unified sync and storage) equivalent of the TypedUrlSyncableService. This CL is for adding TypedURLSyncBridge class but not implement any function yet. This CL is Also addressing missing parts for bug 558320. CL is https://codereview.chromium.org/1647833006 BUG=697960 ========== to ========== [sync] skeleton implementation of TypedURLSyncBridge TypedURLSyncBridge is the USS(unified sync and storage) equivalent of the TypedUrlSyncableService. This CL is for adding TypedURLSyncBridge class but not implement any function yet. This CL is Also addressing missing parts for bug 558320. CL is https://codereview.chromium.org/1647833006 BUG=697960 Review-Url: https://codereview.chromium.org/2723953002 Cr-Commit-Position: refs/heads/master@{#461901} Committed: https://chromium.googlesource.com/chromium/src/+/69ae3f7272abd5f3c8b5da163916... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:260001) as https://chromium.googlesource.com/chromium/src/+/69ae3f7272abd5f3c8b5da163916... |
