Implement IsEntityDataValid() on HistorySyncBridge [chromium/src : main]
0 views
Skip to first unread message
Marc Treib (Gerrit)
Sep 25, 2025, 9:07:41 AM (2 days ago) Sep 25
to Marc Treib, Chromium Sync Reviews, Chromium LUCI CQ, chromium...@chromium.org
Attention needed from Chromium Sync Reviews
New activity on the change![]( "Open in Gerrit")
Related details
Attention is currently required from:
- Chromium Sync Reviews
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
gwsq (Gerrit)
Sep 25, 2025, 9:10:01 AM (2 days ago) Sep 25
to Marc Treib, Chromium Sync Reviews, Ankush Singh, Chromium LUCI CQ, chromium...@chromium.org
Attention needed from Ankush Singh
Message from gwsq![]( "Open in Gerrit")
Reviewer source(s):
ankus...@google.com is from context(googleclient/chrome/chromium_gwsq/components/sync/config.gwsq)
Related details
Attention is currently required from:
- Ankush Singh
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ankush Singh (Gerrit)
Sep 25, 2025, 12:26:24 PM (2 days ago) Sep 25
to Marc Treib, Chromium Sync Reviews, Chromium LUCI CQ, chromium...@chromium.org
Attention needed from Marc Treib
Ankush Singh voted and added 2 comments![]( "Open in Gerrit")
Votes added by Ankush Singh
| Code-Review | +1 |
2 comments
Patchset-level comments
File components/history/core/browser/sync/history_sync_bridge.cc
Line 771, Patchset 1 (Latest):
DCHECK(entity_data.specifics.has_history())Ankush Singh . unresolved
Do we think it's risky making it a CHECK?
Related details
Attention is currently required from:
- Marc Treib
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Marc Treib (Gerrit)
Sep 25, 2025, 12:36:43 PM (2 days ago) Sep 25
to Marc Treib, Ankush Singh, Chromium Sync Reviews, Chromium LUCI CQ, chromium...@chromium.org
Marc Treib voted and added 2 comments![]( "Open in Gerrit")
Votes added by Marc Treib
| Commit-Queue | +1 |
2 comments
Patchset-level comments
File components/history/core/browser/sync/history_sync_bridge.cc
Do we think it's risky making it a CHECK?
Marc Treib
No, I had just blindly copied this from GetStorageKey/GetClientTag. Changed to `CHECK` for all 3 of them (and also removed the message since it's obvious anyway, and to avoid bloating release binaries with these strings).
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Marc Treib (Gerrit)
Sep 25, 2025, 1:25:20 PM (2 days ago) Sep 25
to Marc Treib, Ankush Singh, Chromium Sync Reviews, Chromium LUCI CQ, chromium...@chromium.org
Marc Treib voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Sep 25, 2025, 1:28:00 PM (2 days ago) Sep 25
to Marc Treib, Ankush Singh, Chromium Sync Reviews, chromium...@chromium.org
Chromium LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
1 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: components/history/core/browser/sync/history_sync_bridge.cc
Insertions: 3, Deletions: 6.
The diff is too large to show. Please review the diff.
```
Change information
Commit message:
Implement IsEntityDataValid() on HistorySyncBridge
The implementation just checks that visit_time_windows_epoch_micros is
populated (since this is what client tag and storage key are derived
from).
Bug: 40889096
Change-Id: Ic1b2def993b6249d28c444604eb4976feb6603bd
Commit-Queue: Marc Treib <tr...@chromium.org>
Reviewed-by: Ankush Singh <ankus...@google.com>
Cr-Commit-Position: refs/heads/main@{#1520712}
Files:
- M components/history/core/browser/sync/history_sync_bridge.cc
- M components/history/core/browser/sync/history_sync_bridge.h
Change size: S
Delta: 2 files changed, 12 insertions(+), 4 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Ankush Singh
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages