Pass content_browsertests when NetS13nSW is on |
|||||
Issue descriptionSimilar to issue 860361 we want to make browser tests pass. It seems we have only one failure (CrossSiteDocumentBlockingServiceWorkerTest.NoNetwork). According to [1] we may just need to skip the test. The test is failing with --enable-features=NetworkService as well. [1] https://chromium-review.googlesource.com/c/chromium/src/+/879348/12/content/browser/loader/cross_site_document_resource_handler.cc#587 --- CrossSiteDocumentBlockingServiceWorkerTest.NoNetwork Histogram "SiteIsolation.XSD.Browser.Action" does not have the right number of samples (1) in the expected bucket (0). It has (0). Stack trace: #0 0x000000c277fc testing::internal::UnitTestImpl::CurrentOsStackTraceExceptTop() #1 0x000000c271d9 testing::internal::AssertHelper::operator=() #2 0x000000d78198 base::HistogramTester::CheckBucketCount() #3 0x000000d78669 base::HistogramTester::ExpectBucketCount() #4 0x0000007bdf68 content::(anonymous namespace)::InspectHistograms() #5 0x0000007c33d1 content::CrossSiteDocumentBlockingServiceWorkerTest_NoNetwork_Test::RunTestOnMainThread() #6 0x000000d42780 content::BrowserTestBase::ProxyRunTestOnMainThreadLoop() #7 0x000000ddaf26 content::ShellBrowserMainParts::PreMainMessageLoopRun() #8 0x7f680b753151 content::BrowserMainLoop::PreMainMessageLoopRun() #9 0x7f680bc50a35 content::StartupTaskRunner::RunAllTasksNow() #10 0x7f680b751a5f content::BrowserMainLoop::CreateStartupTasks() #11 0x7f680b755ef5 content::BrowserMainRunnerImpl::Initialize() #12 0x7f680b755e57 content::BrowserMainRunnerImpl::Initialize() #13 0x000000dda70d ShellBrowserMain() #14 0x000000d9015d content::ShellMainDelegate::RunProcess() #15 0x7f680c238e1e content::RunBrowserProcessMain() #16 0x7f680c239e81 content::ContentMainRunnerImpl::Run() #17 0x7f6809113877 service_manager::Main() #18 0x7f680c237ed4 content::ContentMain() #19 0x000000d422e6 content::BrowserTestBase::SetUp() #20 0x000000d37474 content::ContentBrowserTest::SetUp() ../../base/test/metrics/histogram_tester.cc:202: Failure Expected equality of these values: expected_count Which is: 1 actual_count Which is: 0
,
Jul 12
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/70c988359c0c29cd2fd3f84842bd10f762b65b07 commit 70c988359c0c29cd2fd3f84842bd10f762b65b07 Author: Kenichi Ishibashi <bashi@chromium.org> Date: Thu Jul 12 23:53:09 2018 S13nServiceWorker: Skip CrossSiteDocumentBlockingServiceWorkerTest.NoNetwork When S13nServiceWorker is enabled requests/responses directly go from page to service worker and we don't go through the XSDB check. As [1] noted, we wouldn't need to check responses because checking responses would be redundant. This CL make rossSiteDocumentBlockingServiceWorkerTest.NoNetwork to be skipped as the test wouldn't be needed when S13nServiceWorker is enabled. [1] https://chromium-review.googlesource.com/c/chromium/src/+/879348/12/content/browser/loader/cross_site_document_resource_handler.cc#587 Bug: 861644 Change-Id: Id64063995f341733fc8f39c3620c083ab07d0c94 Reviewed-on: https://chromium-review.googlesource.com/1134636 Commit-Queue: Kenichi Ishibashi <bashi@chromium.org> Reviewed-by: Charlie Reis <creis@chromium.org> Reviewed-by: Matt Falkenhagen <falken@chromium.org> Cr-Commit-Position: refs/heads/master@{#574776} [modify] https://crrev.com/70c988359c0c29cd2fd3f84842bd10f762b65b07/content/browser/loader/cross_site_document_blocking_browsertest.cc
,
Jul 12
,
Oct 16
Re-opening as following tests are currently failing: RequestDataBrowserTest.LinkRelPrefetchReferrerPolicy RequestDataBrowserTest.BasicCrossSite RequestDataBrowserTest.LinkRelPrefetch RequestDataBrowserTest.Basic It seems that prefetch isn't working. I added a debug LOG(ERROR) in RequestDataBrowserTest.Basic to show RequestData. I can't find '/prefetch' request when S13nSW is on. w/ S13nSW: http://127.0.0.1:38623/page_with_subresources.html http://127.0.0.1:38623/style http://127.0.0.1:38623/image-inline http://127.0.0.1:38623/script http://127.0.0.1:38623/ping http://127.0.0.1:38623/beacon http://127.0.0.1:38623/image-script w/o S13nSW: http://127.0.0.1:37133/page_with_subresources.html http://127.0.0.1:37133/style http://127.0.0.1:37133/image-inline http://127.0.0.1:37133/script http://127.0.0.1:37133/prefetch http://127.0.0.1:37133/ping http://127.0.0.1:37133/beacon http://127.0.0.1:37133/image-script falken@, kinuko@: Do you know any recent change in loading code path that might cause this?
,
Oct 16
+horo. Maybe the prefetch stuff for signed exchange?
,
Oct 17
The PrefetchURLLoader looks used correctly. But the browser test uses URLLoaderInterceptor and it doesn't see the prefetch request. In NetworkService, the PrefetchLoader comes before the Interceptor which sees the request. In non-NetworkService, the PrefetchLoader comes after the Interceptor which sees the request. But in ServiceWorkerServicfiication, the Interceptor doesn't see the request at all. non-NetworkService, non-ServiceWorkerServification: [250852:250864:1017/093016.842526:ERROR:url_loader_interceptor.cc(409)] bool content::URLLoaderInterceptor::Intercept(content::URLLoaderInterceptor::RequestParams *): Intercepting: http://127.0.0.1:44817/link_rel_prefetch.html [250852:250864:1017/093017.271001:ERROR:url_loader_interceptor.cc(87)] virtual void content::URLLoaderInterceptor::Interceptor::CreateLoaderAndStart(network::mojom::URLLoaderRequest, int32_t, int32_t, uint32_t, const network::ResourceRequest &, network::mojom::URLLoaderClientPtr, const net::MutableNe tworkTrafficAnnotationTag &): http://127.0.0.1:44817/image.jpg [250852:250864:1017/093017.271065:ERROR:url_loader_interceptor.cc(409)] bool content::URLLoaderInterceptor::Intercept(content::URLLoaderInterceptor::RequestParams *): Intercepting: http://127.0.0.1:44817/image.jpg [250852:250864:1017/093017.271111:ERROR:prefetch_url_loader_service.cc(80)] PrefetchLoader::CreateLoaderAndStart [250852:250864:1017/093017.271198:ERROR:prefetch_url_loader.cc(72)] PrefetchURLLoader: http://127.0.0.1:44817/image.jpg [250852:250864:1017/093017.274791:ERROR:prefetch_url_loader.cc(137)] virtual void content::PrefetchURLLoader::OnReceiveResponse(const network::ResourceResponseHead &): recv [250852:250864:1017/093017.277242:ERROR:prefetch_url_loader.cc(182)] virtual void content::PrefetchURLLoader::OnStartLoadingResponseBody(mojo::ScopedDataPipeConsumerHandle): start loading [250852:250864:1017/093017.277307:ERROR:prefetch_url_loader.cc(192)] virtual void content::PrefetchURLLoader::OnComplete(const network::URLLoaderCompletionStatus &): complete non-NetworkService, ServiceWorkerServicification: [250936:250945:1017/093032.243251:ERROR:url_loader_interceptor.cc(409)] bool content::URLLoaderInterceptor::Intercept(content::URLLoaderInterceptor::RequestParams *): Intercepting: http://127.0.0.1:43351/link_rel_prefetch.html [250936:250945:1017/093032.670447:ERROR:prefetch_url_loader_service.cc(80)] PrefetchLoader::CreateLoaderAndStart [250936:250945:1017/093032.670597:ERROR:prefetch_url_loader.cc(72)] PrefetchURLLoader: http://127.0.0.1:43351/image.jpg [250936:250945:1017/093032.674199:ERROR:prefetch_url_loader.cc(137)] virtual void content::PrefetchURLLoader::OnReceiveResponse(const network::ResourceResponseHead &): recv [250936:250945:1017/093032.675026:ERROR:prefetch_url_loader.cc(182)] virtual void content::PrefetchURLLoader::OnStartLoadingResponseBody(mojo::ScopedDataPipeConsumerHandle): start loading [250936:250945:1017/093032.675093:ERROR:prefetch_url_loader.cc(192)] virtual void content::PrefetchURLLoader::OnComplete(const network::URLLoaderCompletionStatus &): complete NetworkService: [251324:251331:1017/093216.647018:ERROR:url_loader_interceptor.cc(87)] virtual void content::URLLoaderInterceptor::Interceptor::CreateLoaderAndStart(network::mojom::URLLoaderRequest, int32_t, int32_t, uint32_t, const network::ResourceRequest &, network::mojom::URLLoaderClientPtr, const net::MutableNetworkTrafficAnnotationTag &): http://127.0.0.1:45495/link_rel_prefetch.html [251324:251331:1017/093216.647089:ERROR:url_loader_interceptor.cc(409)] bool content::URLLoaderInterceptor::Intercept(content::URLLoaderInterceptor::RequestParams *): Intercepting: http://127.0.0.1:45495/link_rel_prefetch.html [251324:251331:1017/093216.769266:ERROR:prefetch_url_loader_service.cc(80)] PrefetchLoader::CreateLoaderAndStart [251324:251331:1017/093216.769386:ERROR:prefetch_url_loader.cc(72)] PrefetchURLLoader: http://127.0.0.1:45495/image.jpg [251324:251331:1017/093216.769592:ERROR:url_loader_interceptor.cc(87)] virtual void content::URLLoaderInterceptor::Interceptor::CreateLoaderAndStart(network::mojom::URLLoaderRequest, int32_t, int32_t, uint32_t, const network::ResourceRequest &, network::mojom::URLLoaderClientPtr, const net::MutableNetworkTrafficAnnotationTag &): http://127.0.0.1:45495/image.jpg [251324:251331:1017/093216.769623:ERROR:url_loader_interceptor.cc(409)] bool content::URLLoaderInterceptor::Intercept(content::URLLoaderInterceptor::RequestParams *): Intercepting: http://127.0.0.1:45495/image.jpg [251324:251331:1017/093216.771415:ERROR:prefetch_url_loader.cc(137)] virtual void content::PrefetchURLLoader::OnReceiveResponse(const network::ResourceResponseHead &): recv [251324:251331:1017/093216.771838:ERROR:prefetch_url_loader.cc(182)] virtual void content::PrefetchURLLoader::OnStartLoadingResponseBody(mojo::ScopedDataPipeConsumerHandle): start loading [251324:251331:1017/093216.772183:ERROR:prefetch_url_loader.cc(192)] virtual void content::PrefetchURLLoader::OnComplete(const network::URLLoaderCompletionStatus &): complete
,
Oct 17
I can make these tests pass with this change in URLLoaderInterceptor::URLLoaderInterceptor.
if (base::FeatureList::IsEnabled(network::features::kNetworkService)) {
becomes
if (base::FeatureList::IsEnabled(
blink::features::kServiceWorkerServicification) ||
base::FeatureList::IsEnabled(network::features::kNetworkService)) {
Not sure if it would break other tests.
,
Oct 17
Is there any way to know when this test was broken?
,
Oct 17
The tests pass after reverting https://chromium-review.googlesource.com/c/1272419 (and readding #include "third_party/blink/public/common/features.h" to signed_exchange_request_handler_browsertest.cc).
,
Oct 17
I see. Yes, we need to make URLLoaderInterceptor call RenderFrameHostImpl::SetNetworkFactoryForTesting() to intercept the prefetch request. And also, I think we should have service_worker_servicification_content_browsertests in /testing/buildbot/chromium.linux.json to run content_browsertests with ServiceWorkerServicification in try bots...
,
Oct 17
Unfortunately just doing the change in c#7 breaks at least HeadlessProtocolBrowserTest.VirtualTimeErrorLoop when ServiceWorkerServicification is on. Regarding trybots, that was considered but I don't know if we would have gotten approval to slow down the CQ. In practice we didn't get failures until now and we are enabling by default once this is fixed.
,
Oct 17
Ah, regarding VirtualTimeErrorLoop, maybe the change is fine. The failure was too much output due to my logging.
,
Oct 17
I take that back. The change causes flaky errors and errors on ASAN.
,
Oct 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/54c7f4ca0d60dba1b9bb7eb8f1368ef3b1d2e49f commit 54c7f4ca0d60dba1b9bb7eb8f1368ef3b1d2e49f Author: Matt Falkenhagen <falken@chromium.org> Date: Wed Oct 17 06:33:28 2018 Teach URLLoaderInterceptor about ServiceWorkerServicification flag. It needs to SetNetworkFactoryForTesting when S13nSW is on, so it can intercept prefetch requests which go to a special factory since r598636. This fixes the following browser tests when S13nSW is enabled: RequestDataBrowserTest.LinkRelPrefetchReferrerPolicy RequestDataBrowserTest.BasicCrossSite RequestDataBrowserTest.LinkRelPrefetch RequestDataBrowserTest.Basic BUG= 861644 Change-Id: Id4982b70d6633ecf2bd44c4afb8f5649cf801f21 Reviewed-on: https://chromium-review.googlesource.com/c/1285789 Commit-Queue: Matt Falkenhagen <falken@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Cr-Commit-Position: refs/heads/master@{#600302} [modify] https://crrev.com/54c7f4ca0d60dba1b9bb7eb8f1368ef3b1d2e49f/content/public/test/url_loader_interceptor.cc
,
Oct 17
Thanks for the fix! |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by creis@chromium.org
, Jul 12Components: Internals>Sandbox>SiteIsolation
Owner: bashi@chromium.org
Status: Started (was: Available)