Remove AttributionReportingCrossAppWeb runtime and content features [chromium/src : main]
5 views
Skip to first unread message
Andrew Paseltiner (Gerrit)
Nov 5, 2024, 3:42:18 PM11/5/24
to John Delaney, Nan Lin, AyeAye, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, fuzzin...@chromium.org, android-web...@chromium.org, apaselti...@chromium.org, asvitkine...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, feature-co...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading...@chromium.org, network-ser...@chromium.org
Attention needed from John Delaney and Nan Lin
Andrew Paseltiner voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Attention is currently required from:
- John Delaney
- Nan Lin
Submit Requirements:
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andrew Paseltiner (Gerrit)
Nov 6, 2024, 8:22:22 AM11/6/24
to John Delaney, Nan Lin, AyeAye, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, fuzzin...@chromium.org, android-web...@chromium.org, apaselti...@chromium.org, asvitkine...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, feature-co...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading...@chromium.org, network-ser...@chromium.org
Attention needed from John Delaney and Nan Lin
Andrew Paseltiner added 1 comment![]( "Open in Gerrit")
File content/browser/attribution_reporting/cross_app_web_attribution_browsertest.cc
Line 94, Patchset 2:
TestCase{
.conversion_measurement_enabled = true,
.expected = true,
},Andrew Paseltiner . resolved
Andrew Paseltiner@lin...@chromium.org This test case is failing with this CL: The feature policy apparently doesn't include `attribution-reporting` (line 109). Any ideas what's going on here?
Separately, it probably makes sense to rename this file and the associated test fixtures and/or move this to WPT.
Acknowledged
Related details
Attention is currently required from:
- John Delaney
- Nan Lin
Submit Requirements:
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nan Lin (Gerrit)
Nov 7, 2024, 2:26:39 PM11/7/24
to Andrew Paseltiner, John Delaney, AyeAye, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, fuzzin...@chromium.org, android-web...@chromium.org, apaselti...@chromium.org, asvitkine...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, feature-co...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading...@chromium.org, network-ser...@chromium.org
Attention needed from Andrew Paseltiner and John Delaney
Nan Lin added 1 comment![]( "Open in Gerrit")
File third_party/blink/renderer/core/frame/attribution_src_loader.cc
Line 693, Patchset 6 (Parent):
kAttributionReportingCrossAppWebSupportHeader);Nan Lin . unresolved
Is it intended that we still keep the enum (https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/public/mojom/use_counter/metrics/web_feature.mojom;l=4159)?
Related details
Attention is currently required from:
- Andrew Paseltiner
- John Delaney
Submit Requirements:
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andrew Paseltiner (Gerrit)
Nov 7, 2024, 2:36:05 PM11/7/24
to John Delaney, Nan Lin, AyeAye, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, fuzzin...@chromium.org, android-web...@chromium.org, apaselti...@chromium.org, asvitkine...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, feature-co...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading...@chromium.org, network-ser...@chromium.org
Attention needed from John Delaney and Nan Lin
Andrew Paseltiner added 1 comment![]( "Open in Gerrit")
File third_party/blink/renderer/core/frame/attribution_src_loader.cc
Line 693, Patchset 6 (Parent):
kAttributionReportingCrossAppWebSupportHeader);Nan Lin . resolved
Is it intended that we still keep the enum (https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/public/mojom/use_counter/metrics/web_feature.mojom;l=4159)?
Attention is currently required from:
- John Delaney
- Nan Lin
Submit Requirements:
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nan Lin (Gerrit)
Nov 7, 2024, 2:57:59 PM11/7/24
to Andrew Paseltiner, John Delaney, AyeAye, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, blink-re...@chromium.org, fuzzin...@chromium.org, android-web...@chromium.org, apaselti...@chromium.org, asvitkine...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, feature-co...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading...@chromium.org, network-ser...@chromium.org
Attention needed from Andrew Paseltiner and John Delaney
Nan Lin voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Andrew Paseltiner
- John Delaney
Submit Requirements:
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andrew Paseltiner (Gerrit)
Nov 7, 2024, 3:31:36 PM11/7/24
to Nan Lin, John Delaney, AyeAye, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, blink-re...@chromium.org, fuzzin...@chromium.org, android-web...@chromium.org, apaselti...@chromium.org, asvitkine...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, feature-co...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading...@chromium.org, network-ser...@chromium.org
Attention needed from John Delaney
Andrew Paseltiner voted Commit-Queue+1![]( "Open in Gerrit")
Attention is currently required from:
- John Delaney
Submit Requirements:
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
John Delaney (Gerrit)
Nov 11, 2024, 11:47:09 AM11/11/24
to Andrew Paseltiner, Nan Lin, AyeAye, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, blink-re...@chromium.org, fuzzin...@chromium.org, android-web...@chromium.org, apaselti...@chromium.org, asvitkine...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, feature-co...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading...@chromium.org, network-ser...@chromium.org
Attention needed from Andrew Paseltiner
John Delaney voted and added 1 comment![]( "Open in Gerrit")
Attention is currently required from:
- Andrew Paseltiner
Submit Requirements:
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andrew Paseltiner (Gerrit)
Nov 21, 2024, 9:10:26 AM11/21/24
to Kinuko Yasuda, Nico Weber, Peter Conn, John Delaney, Nan Lin, AyeAye, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, blink-re...@chromium.org, fuzzin...@chromium.org, android-web...@chromium.org, apaselti...@chromium.org, asvitkine...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, feature-co...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading...@chromium.org, network-ser...@chromium.org
Attention needed from Kinuko Yasuda, Nico Weber and Peter Conn
Andrew Paseltiner voted Commit-Queue+1![]( "Open in Gerrit")
Attention is currently required from:
- Kinuko Yasuda
- Nico Weber
- Peter Conn
Submit Requirements:
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nico Weber (Gerrit)
Nov 21, 2024, 9:15:41 AM11/21/24
to Andrew Paseltiner, Kinuko Yasuda, Nico Weber, Peter Conn, John Delaney, Nan Lin, AyeAye, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, blink-re...@chromium.org, fuzzin...@chromium.org, android-web...@chromium.org, apaselti...@chromium.org, asvitkine...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, feature-co...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading...@chromium.org, network-ser...@chromium.org
Attention needed from Andrew Paseltiner, Kinuko Yasuda and Peter Conn
Nico Weber added 3 comments![]( "Open in Gerrit")
Patchset-level comments
File third_party/blink/renderer/core/frame/attribution_src_loader.cc
Line 791, Patchset 9 (Parent):
kAttributionReportingCrossAppWebSupportHeader);Nico Weber . unresolved
Don't you want to unconditionally count here now, instead of removing the count?
File third_party/blink/renderer/core/page/create_window.cc
Line 76, Patchset 9 (Latest):
RuntimeEnabledFeatures::AttributionReportingEnabled(dom_window);Nico Weber . unresolved
The lhs is `(x || true)`, so the behavior-preserving change is to change this to just `= dom_window` I think.
Maybe the LHS was wrong and wanted to use `&&` instead of `||`. If that's the case, we should change that in a separate CL, probably before landing this one here.
Related details
Attention is currently required from:
- Andrew Paseltiner
- Kinuko Yasuda
- Peter Conn
Submit Requirements:
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andrew Paseltiner (Gerrit)
Nov 21, 2024, 9:25:41 AM11/21/24
to Kinuko Yasuda, Nico Weber, Peter Conn, John Delaney, Nan Lin, AyeAye, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, blink-re...@chromium.org, fuzzin...@chromium.org, android-web...@chromium.org, apaselti...@chromium.org, asvitkine...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, feature-co...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading...@chromium.org, network-ser...@chromium.org
Attention needed from John Delaney, Kinuko Yasuda, Nan Lin and Peter Conn
Andrew Paseltiner added 2 comments![]( "Open in Gerrit")
File third_party/blink/renderer/core/frame/attribution_src_loader.cc
Line 791, Patchset 9 (Parent):
kAttributionReportingCrossAppWebSupportHeader);Nico Weber . unresolved
Don't you want to unconditionally count here now, instead of removing the count?
Andrew Paseltiner
I guess I'm not really sure what the purpose of this count is from a Blink perspective.
CC @lin...@chromium.org who authored this logic in http://crrev.com/c/5249677, and also @john...@chromium.org for clarification.
File third_party/blink/renderer/core/page/create_window.cc
Line 76, Patchset 9 (Latest):
RuntimeEnabledFeatures::AttributionReportingEnabled(dom_window);Nico Weber . unresolved
The lhs is `(x || true)`, so the behavior-preserving change is to change this to just `= dom_window` I think.
Maybe the LHS was wrong and wanted to use `&&` instead of `||`. If that's the case, we should change that in a separate CL, probably before landing this one here.
Andrew Paseltiner
This is a good question.
CC @lin...@chromium.org who authored this logic in http://crrev.com/c/4467529, and also @john...@chromium.org for clarification.
Related details
Attention is currently required from:
- John Delaney
- Kinuko Yasuda
- Nan Lin
- Peter Conn
Peter Conn (Gerrit)
Nov 21, 2024, 9:44:33 AM11/21/24
to Andrew Paseltiner, Kinuko Yasuda, Nico Weber, John Delaney, Nan Lin, AyeAye, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, blink-re...@chromium.org, fuzzin...@chromium.org, android-web...@chromium.org, apaselti...@chromium.org, asvitkine...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, feature-co...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading...@chromium.org, network-ser...@chromium.org
Attention needed from Andrew Paseltiner, John Delaney, Kinuko Yasuda and Nan Lin
Peter Conn added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Attention is currently required from:
- Andrew Paseltiner
- John Delaney
- Kinuko Yasuda
- Nan Lin
Nan Lin (Gerrit)
Dec 5, 2024, 12:41:42 PM12/5/24
to Andrew Paseltiner, Kinuko Yasuda, Nico Weber, Peter Conn, John Delaney, AyeAye, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, blink-re...@chromium.org, fuzzin...@chromium.org, android-web...@chromium.org, apaselti...@chromium.org, asvitkine...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, feature-co...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading...@chromium.org, network-ser...@chromium.org
Attention needed from Andrew Paseltiner, John Delaney and Kinuko Yasuda
Nan Lin voted and added 2 comments![]( "Open in Gerrit")
Votes added by Nan Lin
| Code-Review | +1 |
2 comments
File third_party/blink/renderer/core/frame/attribution_src_loader.cc
Line 791, Patchset 9 (Parent):
kAttributionReportingCrossAppWebSupportHeader);Nico Weber . unresolved
Andrew PaseltinerDon't you want to unconditionally count here now, instead of removing the count?
I guess I'm not really sure what the purpose of this count is from a Blink perspective.
CC @lin...@chromium.org who authored this logic in http://crrev.com/c/5249677, and also @john...@chromium.org for clarification.
Nan Lin
When the feature was default enabled, the support header will be sent for all attribution-eligible requests. Therefore there's no need for a separate user counter, and we can simply refer to `kAttributionReportingAPIAll` for API usage.
File third_party/blink/renderer/core/page/create_window.cc
Line 76, Patchset 9 (Latest):
RuntimeEnabledFeatures::AttributionReportingEnabled(dom_window);Nico Weber . unresolved
Andrew PaseltinerThe lhs is `(x || true)`, so the behavior-preserving change is to change this to just `= dom_window` I think.
Maybe the LHS was wrong and wanted to use `&&` instead of `||`. If that's the case, we should change that in a separate CL, probably before landing this one here.
This is a good question.
CC @lin...@chromium.org who authored this logic in http://crrev.com/c/4467529, and also @john...@chromium.org for clarification.
Nan Lin
This was added when we were running OT for web-based ARA and cross app web ARA separately, and the intention was to allow testing cross app web ARA with only the token for cross app web ARA OT.
After the OTs ended and was removed, the runtime feature is gated by this base::Feature as well as `kConversionMeasurement` (see https://source.chromium.org/chromium/chromium/src/+/main:content/child/runtime_features.cc;l=660). Therefore, we still need the condition on the main runtime feature which is gated by `kConversionMeasurement`.
Related details
Attention is currently required from:
- Andrew Paseltiner
- John Delaney
- Kinuko Yasuda
Jason Chase (Gerrit)
Dec 5, 2024, 1:29:09 PM12/5/24
to feature-co...@chromium.org
Attention needed from John Delaney, Kinuko Yasuda and Nico Weber
Andrew Paseltiner has uploaded the change for review![]( "Open in Gerrit")
Jason Chase removed feature-co...@chromium.org from reviewers of this change.
Commit message
Remove AttributionReportingCrossAppWeb runtime and content features
These have been enabled by default since M125, so this is a simple
mechanical removal.
This CL contains no intentional web-visible behavioral changes.
Change-Id: I470821384145bbef92fc1a173a17181ed855a80d
Change diff
Change information
Files:
- M android_webview/java/src/org/chromium/android_webview/common/ProductionSupportedFlagList.java
- M android_webview/javatests/src/org/chromium/android_webview/test/AttributionReportingTest.java
- M components/attribution_reporting/parse_info_header_fuzzer.cc
- M components/attribution_reporting/registration_info.cc
- M components/attribution_reporting/registration_info.h
- M components/attribution_reporting/registration_info_unittest.cc
- M content/browser/attribution_reporting/attribution_data_host_manager_impl.cc
- M content/browser/attribution_reporting/attribution_data_host_manager_impl_unittest.cc
- M content/browser/attribution_reporting/attribution_host.cc
- M content/browser/attribution_reporting/attribution_manager_impl.cc
- M content/browser/attribution_reporting/attribution_src_browsertest.cc
- M content/browser/attribution_reporting/attributions_browsertest.cc
- D content/browser/attribution_reporting/cross_app_web_attribution_browsertest.cc
- M content/browser/attribution_reporting/interop/runner.cc
- M content/browser/fenced_frame/fenced_frame_browsertest.cc
- M content/browser/loader/keep_alive_attribution_request_helper_unittest.cc
- M content/child/runtime_features.cc
- M content/public/common/content_switch_dependent_feature_overrides.cc
- M content/test/BUILD.gn
- M services/network/attribution/attribution_request_helper.cc
- M services/network/attribution/attribution_request_helper_unittest.cc
- M services/network/public/cpp/features.cc
- M services/network/public/cpp/features.h
- M testing/variations/fieldtrial_testing_config.json
- M third_party/blink/public/mojom/use_counter/metrics/web_feature.mojom
- M third_party/blink/renderer/core/fetch/request_init.idl
- M third_party/blink/renderer/core/frame/attribution_src_loader.cc
- M third_party/blink/renderer/core/frame/attribution_src_loader_test.cc
- M third_party/blink/renderer/core/html/html_attributionsrc_element_utils.idl
- M third_party/blink/renderer/core/page/create_window.cc
- M third_party/blink/renderer/core/permissions_policy/permissions_policy_features.json5
- M third_party/blink/renderer/core/xmlhttprequest/xml_http_request.idl
- M third_party/blink/renderer/platform/runtime_enabled_features.json5
- M tools/metrics/histograms/enums.xml
Change size: L
Delta: 34 files changed, 85 insertions(+), 686 deletions(-)
Related details
Attention is currently required from:
- John Delaney
- Kinuko Yasuda
- Nico Weber
Submit Requirements:
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages