Pass browser_tests and content_browsertests when NetS13nSW is on. |
||||
Issue descriptionIn order to enable NetS13nSW by default, we'll need to make all browser tests pass. We should prepare a CL that enables by default and see how many tests fail on the CQ and fix all the failures so we can eventually enable by default when ready.
,
Jul 23
Ah, I got confused and thought we only did this for ServiceWorker* tests. Thanks, I didn't realize you already did this for everything. Would you be able to run it on the CQ to see if everything is really green everywhere?
,
Jul 23
Sure. Assigning to myself for now.
,
Jul 23
yhirano: FYI, we expect to already pass almost all tests with --enable-features="ServiceWorkerServicification" (unless there are failures we wouldn't see locally like Android-only failures etc). We are still working on content_unittests at issue 860361 .
,
Jul 23
Ran on trybots: https://chromium-review.googlesource.com/c/chromium/src/+/1146529 Looks like we have some failures: content_browsertests PreviewsStateBrowserTest.ShouldEnableLoFiModeOn PreviewsStateBrowserTest.ShouldEnableLoFiModeNavigateBackThenForward PreviewsStateBrowserTest.ShouldEnableLoFiModeReload browser_tests on ChromeOS and Linux ResourceLoadingHintsBrowserTest.ResourceLoadingHintsHttpsWhitelistedRedirectToHttps ResourceLoadingHintsBrowserTest.ResourceLoadingHintsHttpsWhitelisted PreviewsNoScriptBrowserTest.NoScriptPreviewsEnabled PreviewsNoScriptBrowserTest.NoScriptPreviewsEnabledHttpRedirectToHttps
,
Jul 23
And some layout tests not covered by the virtual test suite? I'm not sure if it's out-of-scope of this issue though.
,
Jul 24
It looks like the finch config only runs browser_tests with the flag. At https://chromium-review.googlesource.com/c/chromium/src/+/1147891, the failures are: content_browsertests on Android (flake?): MSE_ExternalClearKey/EncryptedMediaTest.FrameSizeChangeVideo/0 browser_tests on others: PreviewsNoScriptBrowserTest.NoScriptPreviewsEnabled PreviewsNoScriptBrowserTest.NoScriptPreviewsEnabledHttpRedirectToHttps
,
Jul 24
Actually not sure about the first sentence of c#8.
,
Jul 24
Regarding PreviewsNoScriptBrowserTests: It looks like Previews use NavigationData to store their data. Uploaded a fix. https://chromium-review.googlesource.com/c/chromium/src/+/1148094
,
Jul 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a71756341b5f561768144f0ba51b1567783884d5 commit a71756341b5f561768144f0ba51b1567783884d5 Author: Kenichi Ishibashi <bashi@chromium.org> Date: Mon Jul 30 02:08:35 2018 S13nServiceWorker: Populate NavigationData when NetworkService is disabled When S13nServiceWorker is enabled but NetworkService is disabled, NavigationURLLoaderImpl actually uses URLRequest when a navigation is not intercepted. In that case we have a NavigationData and we can populate it to NavigationHandle. Since some features like Previews use NavigationData to store their data, it would be better to populate it if possible. This fixes following browser_tests: - PreviewsNoScriptBrowserTest.NoScriptPreviewsEnabled - PreviewsNoScriptBrowserTest.NoScriptPreviewsEnabledHttpRedirectToHttps Bug: 866367 Change-Id: Idc85d272772be81dddde9e9e0bdf76f4fadfef8a Reviewed-on: https://chromium-review.googlesource.com/1148094 Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Reviewed-by: Matt Falkenhagen <falken@chromium.org> Reviewed-by: Makoto Shimazu <shimazu@chromium.org> Commit-Queue: Kenichi Ishibashi <bashi@chromium.org> Cr-Commit-Position: refs/heads/master@{#578963} [modify] https://crrev.com/a71756341b5f561768144f0ba51b1567783884d5/content/browser/loader/navigation_url_loader_impl.cc
,
Jul 31
All browser_tests and content_browsertests pass on my Linux machine. (Actually some tests are failing but these failures happen w/o S13nSW too). Filed tracking bug for layout test failures ( issue 869302 ). I'll re-open this if I find other failures on bots etc. |
||||
►
Sign in to add a comment |
||||
Comment 1 by bashi@chromium.org
, Jul 23