Simplify Settings WebUI C++ flags and subpages for Ad Privacy UX Deprecation [chromium/src : main]
0 views
Skip to first unread message
Abe Boujane (Gerrit)
9:50 AM (11 hours ago) 9:50 AM
to Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, Permissions Reviews, SLSA Policy Verification Service, asvitkine...@chromium.org, droger+w...@chromium.org, rainhar...@chromium.org
Abe Boujane added 23 comments![]( "Open in Gerrit")
File chrome/browser/privacy_sandbox/privacy_sandbox_ad_privacy_deprecation_browsertest.cc
Line 100, Patchset 14 (Latest):
IN_PROC_BROWSER_TEST_F(PrivacySandboxAdPrivacyDeprecationDisabledTest,
SettingsRoutesRedirect) {
content::WebContents* web_contents =
browser()->tab_strip_model()->GetActiveWebContents();
GURL base_settings_url("chrome://settings/");
for (const char* url_string : kAdPrivacyUrls) {
ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), GURL(url_string)));
EXPECT_EQ(web_contents->GetLastCommittedURL(), base_settings_url);
}
}Abe Boujane . unresolved
test no longer needed. Delete it.
File chrome/browser/resources/settings/BUILD.gn
File chrome/browser/resources/settings/lazy_load.ts
File chrome/browser/resources/settings/privacy_page/privacy_page.ts
File chrome/browser/resources/settings/privacy_page/privacy_page_index.html
File chrome/browser/resources/settings/privacy_page/privacy_page_index.ts
File chrome/browser/resources/settings/route.ts
File chrome/browser/resources/settings/router.ts
File chrome/browser/resources/settings/settings.ts
File chrome/browser/ui/webui/settings/settings_ui.cc
Line 495, Patchset 14 (Latest):
html_source->AddBoolean(
"isPrivacySandboxRestricted",
privacy_sandbox_service->IsPrivacySandboxRestricted());Abe Boujane . unresolved
why is this still needed here? should it be removed?
Line 501, Patchset 14 (Latest):
html_source->AddBoolean(
"isPrivacySandboxAdPrivacyUxDeprecationEnabled",
base::FeatureList::IsEnabled(
privacy_sandbox::kPrivacySandboxAdPrivacyUxDeprecation));Abe Boujane . unresolved
why is this still needed here? shouldn't it be removed?
File chrome/test/data/webui/settings/privacy_page_test.ts
Line 195, Patchset 14 (Latest):
suite('PrivacySandboxAdPrivacyUxDeprecationEnabled', function() {Abe Boujane . unresolved
shouldn't this entire suite be removed?
File chrome/test/data/webui/settings/settings_browsertest.cc
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Abe Boujane (Gerrit)
10:16 AM (11 hours ago) 10:16 AM
to Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, Permissions Reviews, SLSA Policy Verification Service, asvitkine...@chromium.org, droger+w...@chromium.org, rainhar...@chromium.org
Abe Boujane added 14 comments![]( "Open in Gerrit")
File chrome/browser/privacy_sandbox/privacy_sandbox_ad_privacy_deprecation_browsertest.cc
Line 100, Patchset 14:
IN_PROC_BROWSER_TEST_F(PrivacySandboxAdPrivacyDeprecationDisabledTest,
SettingsRoutesRedirect) {
content::WebContents* web_contents =
browser()->tab_strip_model()->GetActiveWebContents();
GURL base_settings_url("chrome://settings/");
for (const char* url_string : kAdPrivacyUrls) {
ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), GURL(url_string)));
EXPECT_EQ(web_contents->GetLastCommittedURL(), base_settings_url);
}
}Abe Boujane . resolved
test no longer needed. Delete it.
Abe Boujane
Done
File chrome/browser/resources/settings/BUILD.gn
File chrome/browser/resources/settings/privacy_page/privacy_page.ts
File chrome/browser/resources/settings/privacy_page/privacy_page_index.html
File chrome/browser/resources/settings/privacy_page/privacy_page_index.ts
File chrome/browser/resources/settings/route.ts
File chrome/browser/resources/settings/router.ts
File chrome/browser/ui/webui/settings/settings_ui.cc
Line 493, Patchset 15 (Latest):
// Required for Third Party Cookies settings subpage (cookies_page.ts).Abe Boujane . unresolved
remove this comment.
Line 495, Patchset 14:
html_source->AddBoolean(
"isPrivacySandboxRestricted",
privacy_sandbox_service->IsPrivacySandboxRestricted());Abe Boujane . resolved
why is this still needed here? should it be removed?
Abe Boujane
Done
Line 501, Patchset 14:
html_source->AddBoolean(
"isPrivacySandboxAdPrivacyUxDeprecationEnabled",
base::FeatureList::IsEnabled(
privacy_sandbox::kPrivacySandboxAdPrivacyUxDeprecation));Abe Boujane . resolved
why is this still needed here? shouldn't it be removed?
Abe Boujane
Done
File chrome/test/data/webui/settings/privacy_page_test.ts
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Abe Boujane (Gerrit)
10:24 AM (11 hours ago) 10:24 AM
to Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, Permissions Reviews, SLSA Policy Verification Service, asvitkine...@chromium.org, droger+w...@chromium.org, rainhar...@chromium.org
Abe Boujane added 8 comments![]( "Open in Gerrit")
File chrome/browser/resources/settings/privacy_page/privacy_page.ts
Abe Boujane . resolved
Abe Boujaneremove empty line
Done
Line 308, Patchset 14:
'privacy20:ads-click-old';Abe Boujane . unresolved
where does this come from? should it be cleaned up if no longer used?
File chrome/browser/ui/webui/settings/settings_ui.cc
Abe Boujane . resolved
Abe Boujaneremove empty line
Done
Line 493, Patchset 15:
// Required for Third Party Cookies settings subpage (cookies_page.ts).Abe Boujane . resolved
Abe Boujaneremove this comment.
Done
File chrome/test/data/webui/settings/privacy_page_test.ts
Abe Boujane . resolved
Abe Boujaneremove empty line
Done
Line 195, Patchset 14:
suite('PrivacySandboxAdPrivacyUxDeprecationEnabled', function() {Abe Boujane . resolved
Abe Boujaneshouldn't this entire suite be removed?
Done
File chrome/test/data/webui/settings/settings_browsertest.cc
Abe Boujane . resolved
Abe Boujaneremove empty line
Done
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Abe Boujane (Gerrit)
10:33 AM (10 hours ago) 10:33 AM
to Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, Permissions Reviews, SLSA Policy Verification Service, asvitkine...@chromium.org, droger+w...@chromium.org, rainhar...@chromium.org
Abe Boujane added 5 comments![]( "Open in Gerrit")
File chrome/browser/resources/settings/lazy_load.ts
File chrome/browser/resources/settings/privacy_page/privacy_page.ts
Abe Boujane . resolved
Abe Boujaneremove empty line
Done
Abe Boujane . resolved
Abe Boujaneremove empty line
Done
Line 308, Patchset 14:
'privacy20:ads-click-old';Abe Boujane . resolved
where does this come from? should it be cleaned up if no longer used?
Abe Boujane . resolved
Abe Boujaneremove empty line
Done
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Abe Boujane (Gerrit)
11:53 AM (9 hours ago) 11:53 AM
to Rebekah Potter, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, Permissions Reviews, SLSA Policy Verification Service, asvitkine...@chromium.org, droger+w...@chromium.org, rainhar...@chromium.org
Attention needed from Rebekah Potter
Abe Boujane voted Commit-Queue+1![]( "Open in Gerrit")
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Rebekah Potter (Gerrit)
3:33 PM (5 hours ago) 3:33 PM
to Abe Boujane, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, Permissions Reviews, SLSA Policy Verification Service, asvitkine...@chromium.org, droger+w...@chromium.org, rainhar...@chromium.org
Attention needed from Abe Boujane
Rebekah Potter added 7 comments![]( "Open in Gerrit")
File chrome/browser/privacy_sandbox/privacy_sandbox_ad_privacy_deprecation_browsertest.cc
Line 99, Patchset 21 (Latest):
Rebekah Potter . unresolved
no need for 2 blank lines at end of namespace
File chrome/browser/resources/settings/BUILD.gn
File chrome/browser/resources/settings/lazy_load.ts
File chrome/browser/resources/settings/privacy_page/privacy_guide/privacy_guide_completion_fragment.ts
Line 154, Patchset 21 (Parent):
protected getAdsClickIcon_(): string {Rebekah Potter . unresolved
Do we also need to remove the HTML template chunk that is referencing this? https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/resources/settings/privacy_page/privacy_guide/privacy_guide_completion_fragment.html;l=54
File chrome/browser/ui/webui/settings/settings_ui.cc
Line 517, Patchset 21 (Parent):
html_source->AddBoolean("isPrivacySandboxAdPrivacyUxDeprecationEnabled",
is_ad_privacy_ux_deprecation_enabled);Rebekah Potter . unresolved
Still referenced in cookies_page:
https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/resources/settings/privacy_page/cookies_page.ts;l=129
File chrome/test/data/webui/settings/privacy_page_index_test.ts
File chrome/test/data/webui/settings/privacy_page_test.ts
Line 31, Patchset 21 (Latest):
loadTimeData.overrideValues({
});Rebekah Potter . unresolved
Remove unnecessary call
Related details
Attention is currently required from:
- Abe Boujane
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Abe Boujane (Gerrit)
6:16 PM (3 hours ago) 6:16 PM
to Rebekah Potter, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, Permissions Reviews, SLSA Policy Verification Service, asvitkine...@chromium.org, droger+w...@chromium.org, rainhar...@chromium.org
Attention needed from Rebekah Potter
Abe Boujane added 7 comments![]( "Open in Gerrit")
File chrome/browser/privacy_sandbox/privacy_sandbox_ad_privacy_deprecation_browsertest.cc
no need for 2 blank lines at end of namespace
Abe Boujane
Done
File chrome/browser/resources/settings/BUILD.gn
File chrome/browser/resources/settings/lazy_load.ts
File chrome/browser/resources/settings/privacy_page/privacy_guide/privacy_guide_completion_fragment.ts
Line 154, Patchset 21 (Parent):
protected getAdsClickIcon_(): string {Rebekah Potter . resolved
Do we also need to remove the HTML template chunk that is referencing this? https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/resources/settings/privacy_page/privacy_guide/privacy_guide_completion_fragment.html;l=54
Abe Boujane
yes this is being removed in the parent cl. (https://chromium-review.git.corp.google.com/c/chromium/src/+/8130860/11/chrome/browser/resources/settings/privacy_page/privacy_guide/privacy_guide_completion_fragment.html) I think I'll actually make the privacy_guide completion fragment.ts change in the parent cl since that one is supposed to contain the privacy guide component.
File chrome/browser/ui/webui/settings/settings_ui.cc
Line 517, Patchset 21 (Parent):
html_source->AddBoolean("isPrivacySandboxAdPrivacyUxDeprecationEnabled",
is_ad_privacy_ux_deprecation_enabled);Rebekah Potter . resolved
Still referenced in cookies_page:
https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/resources/settings/privacy_page/cookies_page.ts;l=129
Abe Boujane
Great catch! Thank you!
File chrome/test/data/webui/settings/privacy_page_index_test.ts
File chrome/test/data/webui/settings/privacy_page_test.ts
Line 31, Patchset 21:
loadTimeData.overrideValues({
});Rebekah Potter . resolved
Abe BoujaneRemove unnecessary call
Done
Related details
Attention is currently required from:
- Rebekah Potter
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Rebekah Potter (Gerrit)
7:20 PM (2 hours ago) 7:20 PM
to Abe Boujane, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, Permissions Reviews, SLSA Policy Verification Service, asvitkine...@chromium.org, droger+w...@chromium.org, rainhar...@chromium.org
Attention needed from Abe Boujane
Rebekah Potter voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Abe Boujane
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages