Issue metadata
Sign in to add a comment
|
Reporting web_tests fail with NetworkService enabled |
||||||||||||||||||||||||
Issue descriptionWhen NetworkService is enabled, the decision on whether or not to send a report is made by the PermissionController [1] Whereas when the content_shell runs without NetworkService the decision is made by the default NetworkDelegate implementation, which is a dumb pass-through allowing all reports [2] The NetworkService looks correct, as it matches what Chrome browser does [3] So either non-NS content_shell has an incorrect implementation that allows all reports, or we need to match the behavior under NS when content_shell runs in webtests mode. [1] https://chromium.googlesource.com/chromium/src/+/189c388247f195c2bf632a458ec0b1a0d3404275/content/browser/storage_partition_impl.cc#910 [2] https://chromium.googlesource.com/chromium/src/+/189c388247f195c2bf632a458ec0b1a0d3404275/net/base/network_delegate_impl.cc#112 [3] https://chromium.googlesource.com/chromium/src/+/189c388247f195c2bf632a458ec0b1a0d3404275/chrome/browser/net/chrome_network_delegate.cc#395
,
Nov 29
Nice catch. Perhaps these reporting layout tests need to call the setPermission method [1], which ends up at content::LayoutTestPermissionManager and that's called by NetworkContextClient when the network service is enabled. [1] https://cs.chromium.org/chromium/src/third_party/blink/web_tests/http/tests/permissions/chromium/resources/testrunner-helpers.js?rcl=6ba57378eab35effa02c2225aedcdb248bb17f54&l=98
,
Nov 29
Thanks for the pointer!
,
Nov 29
,
Nov 29
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0b9928ad2b2b1c8b4b146d6f03abc0b134a9eac7 commit 0b9928ad2b2b1c8b4b146d6f03abc0b134a9eac7 Author: Eric Roman <eroman@chromium.org> Date: Thu Nov 29 19:59:08 2018 Update web_tests expectations with bug numbers. Bug: 910212 , 729849 Change-Id: Iff0a02015b4b6dabfeff5b3865468df70748ddd6 Reviewed-on: https://chromium-review.googlesource.com/c/1355492 Reviewed-by: John Abd-El-Malek <jam@chromium.org> Commit-Queue: Eric Roman <eroman@chromium.org> Cr-Commit-Position: refs/heads/master@{#612329} [modify] https://crrev.com/0b9928ad2b2b1c8b4b146d6f03abc0b134a9eac7/third_party/blink/web_tests/FlagExpectations/enable-features=NetworkService
,
Dec 4
I played around with testRunner.setPermission(). * Ends up being racy for some of the tests, as the report may have already been queued/sent by the time we call that. * Not a good pattern to be using in web platform tests, as testRunner is specific to the blink test runner, and those changes would need to be upstreamed.
,
Dec 4
,
Jan 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8e3170c00ba458d82a02de29e2a3e742af718e5d commit 8e3170c00ba458d82a02de29e2a3e742af718e5d Author: Eric Roman <eroman@chromium.org> Date: Fri Jan 04 00:11:59 2019 Fix the reporting-api and network-error-logging Web Platform Tests when Network Service is enabled. The reason they failed was they rely on the following behaviors from "content_shell --run-web-tests": (1) Sending reports is allowed for all origins, irrespective of the BACKGROUND_SYNC permission. (2) The timeout for sending reports is 100ms instead of the default (1 minute) This CL makes "content_shell --run-web-tests" with Network Service enabled behave the same way. Bug: 910212 Change-Id: I0756a9605b2aa37647d2c049eb2868499f07e65e Reviewed-on: https://chromium-review.googlesource.com/c/1387449 Commit-Queue: Eric Roman <eroman@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org> Reviewed-by: John Abd-El-Malek <jam@chromium.org> Cr-Commit-Position: refs/heads/master@{#619826} [modify] https://crrev.com/8e3170c00ba458d82a02de29e2a3e742af718e5d/content/shell/browser/shell_content_browser_client.cc [modify] https://crrev.com/8e3170c00ba458d82a02de29e2a3e742af718e5d/content/shell/browser/shell_content_browser_client.h [modify] https://crrev.com/8e3170c00ba458d82a02de29e2a3e742af718e5d/content/shell/browser/shell_url_request_context_getter.cc [modify] https://crrev.com/8e3170c00ba458d82a02de29e2a3e742af718e5d/services/network/network_context.cc [modify] https://crrev.com/8e3170c00ba458d82a02de29e2a3e742af718e5d/services/network/network_context.h [modify] https://crrev.com/8e3170c00ba458d82a02de29e2a3e742af718e5d/services/network/network_service_network_delegate.cc [modify] https://crrev.com/8e3170c00ba458d82a02de29e2a3e742af718e5d/services/network/public/mojom/network_context.mojom [modify] https://crrev.com/8e3170c00ba458d82a02de29e2a3e742af718e5d/third_party/blink/web_tests/FlagExpectations/enable-features=NetworkService
,
Jan 4
|
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by eroman@chromium.org
, Nov 29