[UNO-d] Default isSyncActive to true for devtools [chromium/src : main]
0 views
Skip to first unread message
David Roger (Gerrit)
Sep 23, 2025, 7:59:27 AM (21 hours ago) Sep 23
to Duong Dac, Ergün Erdoğmuş, Wolfgang Beyer, Ankush Singh, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org
Attention needed from Duong Dac
David Roger voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
Attention is currently required from:
- Duong Dac
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Duong Dac (Gerrit)
Sep 23, 2025, 8:51:04 AM (20 hours ago) Sep 23
to David Roger, Ergün Erdoğmuş, Wolfgang Beyer, Ankush Singh, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org
Duong Dac voted Commit-Queue+2![]( "Open in Gerrit")
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 23, 2025, 8:55:32 AM (20 hours ago) Sep 23
to Duong Dac, David Roger, Ergün Erdoğmuş, Wolfgang Beyer, Ankush Singh, chromium...@chromium.org, devtools...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
[UNO-d] Default isSyncActive to true for devtools
Preferences can now be saved to Google account for signed-in users
without enabling Chrome Sync on Win/Mac/Linux (crrev.com/c/6239792,
ChromeOS experiment is in progress). That is, `arePreferencesSynced` can
be true with `isSyncActive` false.
Meanwhile, users cannot toggle devtools settings sync if either
`isSyncActive` or `arePreferencesSynced` is false. Since the
`isSyncActive` condition is redundant on applicable platforms, we will
default `isSyncActive` to true. Setting sync toggle-ability should then
only depend on `arePreferencesSynced`.
Bug: 441694920
Change-Id: I3f27ef04e6c76035487fdfcf6d08f69f28fdac7e
Reviewed-by: David Roger <dro...@chromium.org>
Commit-Queue: Duong Dac <dd...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1519279}
Files:
- M chrome/browser/devtools/BUILD.gn
- M chrome/browser/devtools/devtools_ui_bindings.cc
- M chrome/browser/devtools/devtools_ui_bindings_unittest.cc
Change size: S
Delta: 3 files changed, 11 insertions(+), 3 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by David Roger
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Danil Somsikov (Gerrit)
Sep 23, 2025, 9:01:44 AM (20 hours ago) Sep 23
to Duong Dac, Chromium LUCI CQ, David Roger, Ergün Erdoğmuş, Wolfgang Beyer, Ankush Singh, chromium...@chromium.org, devtools...@chromium.org
Attention needed from Duong Dac
Danil Somsikov added 1 comment![]( "Open in Gerrit")
File chrome/browser/devtools/devtools_ui_bindings.cc
Line 2167, Patchset 6 (Latest):
bool is_signed_in = sync_info.FindBool("accountEmail").has_value() &&Danil Somsikov . unresolved
This line is important for logging. Will this remain true if and only if the user is signed in to Chrome?
Related details
Attention is currently required from:
- Duong Dac
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Duong Dac (Gerrit)
Sep 23, 2025, 9:15:28 AM (20 hours ago) Sep 23
to Chromium LUCI CQ, Danil Somsikov, David Roger, Ergün Erdoğmuş, Wolfgang Beyer, Ankush Singh, chromium...@chromium.org, devtools...@chromium.org
Duong Dac added 1 comment![]( "Open in Gerrit")
File chrome/browser/devtools/devtools_ui_bindings.cc
Line 2167, Patchset 6 (Latest):
bool is_signed_in = sync_info.FindBool("accountEmail").has_value() &&Danil Somsikov . unresolved
This line is important for logging. Will this remain true if and only if the user is signed in to Chrome?
Duong Dac
This is true iff the user **is signed in** and there's no auth error (inferred from `isSyncPaused`).
The previous assumption was stronger: the user has to **have sync enabled**. In practice, users can sign in without turning on sync.
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. |
Ankush Singh (Gerrit)
Sep 23, 2025, 10:57:34 AM (18 hours ago) Sep 23
to Duong Dac, Chromium LUCI CQ, Danil Somsikov, David Roger, Ergün Erdoğmuş, Wolfgang Beyer, chromium...@chromium.org, devtools...@chromium.org
Attention needed from Duong Dac
Ankush Singh added 1 comment![]( "Open in Gerrit")
File chrome/browser/devtools/devtools_ui_bindings.cc
Line 1646, Patchset 6 (Latest):
result.Set("isSyncActive", base::FeatureList::IsEnabled(
switches::kEnablePreferencesAccountStorage) ||
sync_service->IsSyncFeatureActive());Ankush Singh . unresolved
Note that this will return true for all the existing syncing users once the feature is enabled. This is expected maybe?
Related details
Attention is currently required from:
- Duong Dac
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Duong Dac (Gerrit)
Sep 23, 2025, 11:15:15 AM (18 hours ago) Sep 23
to Chromium LUCI CQ, Danil Somsikov, David Roger, Ergün Erdoğmuş, Wolfgang Beyer, Ankush Singh, chromium...@chromium.org, devtools...@chromium.org
Duong Dac added 1 comment![]( "Open in Gerrit")
File chrome/browser/devtools/devtools_ui_bindings.cc
Line 1646, Patchset 6 (Latest):
result.Set("isSyncActive", base::FeatureList::IsEnabled(
switches::kEnablePreferencesAccountStorage) ||
sync_service->IsSyncFeatureActive());Ankush Singh . unresolved
Note that this will return true for all the existing syncing users once the feature is enabled. This is expected maybe?
Duong Dac
That would be expected. The devtools frontend checks this value _and_ `arePreferencesSynced` to allow users toggling saving settings. This change pratically removes the check's dependency on `isSyncActive` for Win/Mac/Linux.
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. |
Ankush Singh (Gerrit)
Sep 23, 2025, 11:19:13 AM (18 hours ago) Sep 23
to Duong Dac, Chromium LUCI CQ, Danil Somsikov, David Roger, Ergün Erdoğmuş, Wolfgang Beyer, chromium...@chromium.org, devtools...@chromium.org
Ankush Singh added 1 comment![]( "Open in Gerrit")
File chrome/browser/devtools/devtools_ui_bindings.cc
Line 1646, Patchset 6 (Latest):
result.Set("isSyncActive", base::FeatureList::IsEnabled(
switches::kEnablePreferencesAccountStorage) ||
sync_service->IsSyncFeatureActive());Ankush Singh . resolved
Duong DacNote that this will return true for all the existing syncing users once the feature is enabled. This is expected maybe?
That would be expected. The devtools frontend checks this value _and_ `arePreferencesSynced` to allow users toggling saving settings. This change pratically removes the check's dependency on `isSyncActive` for Win/Mac/Linux.
Ankush Singh
Acknowledged, thanks for confirming.
Reply all
Reply to author
Forward
0 new messages