New issue
Advanced search Search tips

Issue 805951 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug
Proj-Servicification



Sign in to add a comment

Fix DnsProbeBrowserTest.OtherErrorsWithCorrection* with Network Service

Project Member Reported by blundell@chromium.org, Jan 25 2018

Issue description

These tests have a somewhat complex flow:
- They perform a failed navigation
- The failed navigation results in a navigation to a correction URL via the Link Doctor
- When they see the navigation to the Link Doctor URL, they either perform another failed navigation or navigate to a given filepath (depending on the parameters set by the test)

This flow is set up via BreakableCorrectionInterceptor. I have a hacky patch that fixes DnsProbeBrowserTest.OtherErrorsWithCorrectionSuccess(). It needs lots of work to get it to landable (documented in TODOs in the patch).

The patch is here:

https://chromium-review.googlesource.com/c/chromium/src/+/887065


 
This patch is now much further along and would be ready for review except that I discovered that it broke a couple tests that had been passing:

https://logs.chromium.org/v/?s=chromium%2Fbb%2Ftryserver.chromium.linux%2Flinux_chromium_rel_ng%2F637630%2F%2B%2Frecipes%2Fsteps%2Fnetwork_service_browser_tests__with_patch_%2F0%2Flogs%2FDnsProbeBrowserTest.NoInternetProbeResultWithBrokenCorrections%2F0

Likely there are link doctor requests for which the URLLoaderInterceptor was returning false that it is now returning true. I'll need to dig into this further and likely set some state that determines how the interceptor responds to link doctor requests.
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 8 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/09a4b20be0be25fb8ef32164877335bb30498131

commit 09a4b20be0be25fb8ef32164877335bb30498131
Author: Colin Blundell <blundell@chromium.org>
Date: Thu Feb 08 15:34:03 2018

Fix DNSProbe OtherErrorWithCorrectionsSuccess test with Network Service

This test has a somewhat complex flow:
- It performs a failed navigation
- The failed navigation results in a navigation to a correction URL via
  the Link Doctor
- When it sees the navigation to the Link Doctor URL, it responds with
  the contents of a test file directing the relevant Link Doctor actions
  (//chrome/test/data/mock-link-doctor.json)
- It then verifies that the final URL is as expected given the contents
  of that file

For non-Network Service, this flow is set up via
BreakableCorrectionInterceptor. This CL handles the flow in the Network
Service case by intercepting the Link Doctor request with a
URLLoaderInterceptor and extracting a utility function from
errorpage_browsertest.cc to write the contents of the test file as the
URLLoaderInterceptor's response.

I discovered after making this fix that
the NxdomainProbeResultWithWorkingCorrections test suffers from the same
root issue and hence is also fixed by this change; I've removed both
sets from the set of browser_tests that fail with the Network Service.

Bug:  805951 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Ia72c944079b46ed4aaa3538bde462a3c595f6248
Reviewed-on: https://chromium-review.googlesource.com/887065
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535391}
[modify] https://crrev.com/09a4b20be0be25fb8ef32164877335bb30498131/chrome/browser/net/dns_probe_browsertest.cc
[modify] https://crrev.com/09a4b20be0be25fb8ef32164877335bb30498131/chrome/browser/net/errorpage_browsertest.cc
[modify] https://crrev.com/09a4b20be0be25fb8ef32164877335bb30498131/chrome/browser/net/url_request_mock_util.cc
[modify] https://crrev.com/09a4b20be0be25fb8ef32164877335bb30498131/chrome/browser/net/url_request_mock_util.h
[modify] https://crrev.com/09a4b20be0be25fb8ef32164877335bb30498131/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

Status: Fixed (was: Started)

Sign in to add a comment