New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 853251 link

Starred by 7 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 20
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug

Blocking:
issue 844931



Sign in to add a comment

NetworkService: Hook up domain reliability.

Project Member Reported by mmenke@chromium.org, Jun 15 2018

Issue description

Domain reliability is currently plugged in through ProfileIOData to the URLREquestContext.  We'll need to hook it up some other way when the network service is enabled, since ProfileIOData does nothing meaningful in that case, and is going away eventually.

This presumably does not block enabled the network service on Canary.
 
Agreed re not blocking network service canary
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 19 2018

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

commit cb02821074ba14de3d6a503ef204b694b25017a4
Author: Matt Menke <mmenke@chromium.org>
Date: Tue Jun 19 15:40:30 2018

Annotate many of the browser_tests disabled under the NetworkService.

Also remove a couple that either no longer exist, have been disabled
generally due to flakiness, or are now passing.

BUG= 844950 ,  844951 ,  844952 ,  853251 ,  844928 ,
BUG= 843205 ,  844949 ,  844925 ,  844939 , 821021,
BUG=853798,  844973 ,  844927 ,  844926 ,  844950 

Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I094a012fe2076c7badf86a094140c7d74db183be
Reviewed-on: https://chromium-review.googlesource.com/1104802
Commit-Queue: Matt Menke <mmenke@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568464}
[modify] https://crrev.com/cb02821074ba14de3d6a503ef204b694b25017a4/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

Comment 3 by dxie@chromium.org, Jun 19 2018

Labels: Hotlist-KnownIssue
Cc: juliatut...@chromium.org chongz@chromium.org
 Issue 840386  has been merged into this issue.
Cc: -juliatut...@chromium.org
Is there a schedule for when Network Service is going to Beta / Stable?  I mentioned in #c1 that I don't think DR is a blocker for canarying the network service, but we will need to have it in place when the Network Service is in stable.  We're still using DR to monitor traffic to Google domains, and want to run DR and NEL in parallel to verify NEL's coverage is equivalent to DR before we remove DR.
Also, are there example CLs or docs showing how to move other components off of ProfileIOData?  If there's an easy pattern to follow I'm happy to do some of the work (modulo being OOO for bits of August).
Unfortunately, there's no playbook - lots of stuff hooked into ProfileIOData in very different ways.  Some stuff could be moved completely into the network service.  Other stuff could be moved to completely be in chrome/ or content/, and sit on top of the network service APIs.  Other stuff just needs access to a new mojo API to query / modify data.  Yet other stuff has to straddle the line, which also involves introducing new mojo APIs, but requests significant logic both within services/network and outside it in order to work.
Oh, and there's no fixed timeling for getting the network service in beta or stable - basically, once everything works.  We'll have a better idea of when that will be once we get it on Canary, and it's good enough to stay on Canary.
Blocking: 844931
I just took a quick look at this. The component itself looks like it depends on content (so can't be in network service) and is also tied closely to net/ (i.e. needs to be in network service).

It sounds like NEL will replace it. When is NEL going to stable?
NEL is going to stable experiment in M69 (crbug.com/799253).  Assuming that doesn't encounter any regressions that we didn't catch in the Beta experiment (which is currently going), we'd go to stable a few weeks after that.

That said, we want to keep DR running for ~a quarter in parallel with NEL, so that we can verify that it has the same coverage before switching our internal monitoring pipelines over to the new data feed.
Labels: -Hotlist-KnownIssue
Labels: OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows
Status: Available (was: Untriaged)
Labels: ReleaseBlock-Stable
Project Member

Comment 17 by sheriffbot@chromium.org, Sep 7

Cc: dxie@google.com
This issue is marked as a release blocker with no milestone associated. Please add an appropriate milestone.

All release blocking issues should have milestones associated to it, so that the issue can tracked and the fixes can be pushed promptly.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -ReleaseBlock-Stable
Labels: -Pri-3 Proj-Servicification-Stable Hotlist-KnownIssue Pri-1
Status: WontFix (was: Available)
Per comment 12, since this is going to be removed soonish let's not spend effort on making it work with NS.
Owner: jam@chromium.org
Status: Started (was: WontFix)
per https://bugs.chromium.org/p/chromium/issues/detail?id=904860#c5 looks like this isn't removed yet. I'll make it work with network service so we don't have dependencies on its removal.
Here's my plan: the content/ dependencies on this code are pretty minimal, and they seem easier to remove than the (naturally) tight net/ coupling. So I'll remove the content/ dependencies and then instantiate it in the network process if the network service is running.

the content deps are:
-networkconnectiontracker: I'll revert 4307dceb95b23b5075ca78b14790f8d039363f1d: https://chromium-review.googlesource.com/c/chromium/src/+/1334231
-remove content::IsOriginSecure since it's not necessary: https://chromium-review.googlesource.com/c/chromium/src/+/1334277
-the remaining content dependency is content::GetPermissionControllerw hich is checked before upload. That API was already asynchronous anyways, so I'll make it use network::mojom::NetworkContextClient to ask the browser. This is just like NetworkContextClient::OnCanSendReportingReports.
Cc: sburnett@chromium.org
Project Member

Comment 24 by bugdroid1@chromium.org, Nov 13

Project Member

Comment 25 by bugdroid1@chromium.org, Nov 13

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

commit c764f07b5b599650c5ea491df5b897e4241fa20f
Author: John Abd-El-Malek <jam@chromium.org>
Date: Tue Nov 13 22:46:52 2018

Remove content::IsOriginSecure call in Domain Reliablity, since GURL::SchemeIsCryptographic should be enough.

Domain reliablity only cares about http(s) traffic, so the extra functionality in content::IsOriginSecure (e.g. file/filesystem, UI, local, about, data schemes) aren't needed.

Bug:  853251 
Change-Id: Ic43672a378722bb1c9c41a85ab78d8d7e466818c
Reviewed-on: https://chromium-review.googlesource.com/c/1334277
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Misha Efimov <mef@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607782}
[modify] https://crrev.com/c764f07b5b599650c5ea491df5b897e4241fa20f/components/domain_reliability/DEPS
[modify] https://crrev.com/c764f07b5b599650c5ea491df5b897e4241fa20f/components/domain_reliability/header.cc

Project Member

Comment 26 by bugdroid1@chromium.org, Nov 14

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

commit bd611e3e7daa6e4048f775a4db95f1045a800548
Author: John Abd-El-Malek <jam@chromium.org>
Date: Wed Nov 14 23:46:11 2018

Remove remaining components/domain_reliability dependency on content/ and components/keyed_service.

This way it can run in the network process, similar to Network Error Logging.

Bug:  853251 
Change-Id: I403a5dcc3cae12b592a444c7e23f71c552b58e1f
Reviewed-on: https://chromium-review.googlesource.com/c/1334848
Reviewed-by: Misha Efimov <mef@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608175}
[modify] https://crrev.com/bd611e3e7daa6e4048f775a4db95f1045a800548/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
[modify] https://crrev.com/bd611e3e7daa6e4048f775a4db95f1045a800548/chrome/browser/domain_reliability/service_factory.cc
[modify] https://crrev.com/bd611e3e7daa6e4048f775a4db95f1045a800548/chrome/browser/profiles/profile_impl.cc
[modify] https://crrev.com/bd611e3e7daa6e4048f775a4db95f1045a800548/chrome/browser/profiles/profile_impl_io_data.cc
[modify] https://crrev.com/bd611e3e7daa6e4048f775a4db95f1045a800548/components/domain_reliability/BUILD.gn
[modify] https://crrev.com/bd611e3e7daa6e4048f775a4db95f1045a800548/components/domain_reliability/DEPS
[modify] https://crrev.com/bd611e3e7daa6e4048f775a4db95f1045a800548/components/domain_reliability/monitor.cc
[modify] https://crrev.com/bd611e3e7daa6e4048f775a4db95f1045a800548/components/domain_reliability/monitor.h
[modify] https://crrev.com/bd611e3e7daa6e4048f775a4db95f1045a800548/components/domain_reliability/monitor_unittest.cc
[modify] https://crrev.com/bd611e3e7daa6e4048f775a4db95f1045a800548/components/domain_reliability/service.cc
[modify] https://crrev.com/bd611e3e7daa6e4048f775a4db95f1045a800548/components/domain_reliability/service.h
[modify] https://crrev.com/bd611e3e7daa6e4048f775a4db95f1045a800548/components/domain_reliability/service_unittest.cc

Project Member

Comment 27 by bugdroid1@chromium.org, Nov 15

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

commit 0dde16793a37c31a7093378258a2abfac3bb49f1
Author: Kentaro Hara <haraken@chromium.org>
Date: Thu Nov 15 05:15:31 2018

Revert "Remove remaining components/domain_reliability dependency on content/ and components/keyed_service."

This reverts commit bd611e3e7daa6e4048f775a4db95f1045a800548.

Reason for revert: It looks like this CL broke CFI bots:

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20CFI/11301

Original change's description:
> Remove remaining components/domain_reliability dependency on content/ and components/keyed_service.
> 
> This way it can run in the network process, similar to Network Error Logging.
> 
> Bug:  853251 
> Change-Id: I403a5dcc3cae12b592a444c7e23f71c552b58e1f
> Reviewed-on: https://chromium-review.googlesource.com/c/1334848
> Reviewed-by: Misha Efimov <mef@chromium.org>
> Commit-Queue: John Abd-El-Malek <jam@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#608175}

TBR=jam@chromium.org,mef@chromium.org

Change-Id: I57829e413ad57ec667882069f77bf2d97faec4a3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  853251 
Reviewed-on: https://chromium-review.googlesource.com/c/1337215
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608268}
[modify] https://crrev.com/0dde16793a37c31a7093378258a2abfac3bb49f1/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
[modify] https://crrev.com/0dde16793a37c31a7093378258a2abfac3bb49f1/chrome/browser/domain_reliability/service_factory.cc
[modify] https://crrev.com/0dde16793a37c31a7093378258a2abfac3bb49f1/chrome/browser/profiles/profile_impl.cc
[modify] https://crrev.com/0dde16793a37c31a7093378258a2abfac3bb49f1/chrome/browser/profiles/profile_impl_io_data.cc
[modify] https://crrev.com/0dde16793a37c31a7093378258a2abfac3bb49f1/components/domain_reliability/BUILD.gn
[modify] https://crrev.com/0dde16793a37c31a7093378258a2abfac3bb49f1/components/domain_reliability/DEPS
[modify] https://crrev.com/0dde16793a37c31a7093378258a2abfac3bb49f1/components/domain_reliability/monitor.cc
[modify] https://crrev.com/0dde16793a37c31a7093378258a2abfac3bb49f1/components/domain_reliability/monitor.h
[modify] https://crrev.com/0dde16793a37c31a7093378258a2abfac3bb49f1/components/domain_reliability/monitor_unittest.cc
[modify] https://crrev.com/0dde16793a37c31a7093378258a2abfac3bb49f1/components/domain_reliability/service.cc
[modify] https://crrev.com/0dde16793a37c31a7093378258a2abfac3bb49f1/components/domain_reliability/service.h
[modify] https://crrev.com/0dde16793a37c31a7093378258a2abfac3bb49f1/components/domain_reliability/service_unittest.cc

Project Member

Comment 28 by bugdroid1@chromium.org, Nov 15

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

commit 506d1d88dd858024e14a5b9565a8940df67c92df
Author: John Abd-El-Malek <jam@chromium.org>
Date: Thu Nov 15 06:51:33 2018

Reland "Remove remaining components/domain_reliability dependency on content/ and components/keyed_service."

This is a reland of bd611e3e7daa6e4048f775a4db95f1045a800548

Original change's description:
> Remove remaining components/domain_reliability dependency on content/ and components/keyed_service.
>
> This way it can run in the network process, similar to Network Error Logging.
>
> Bug:  853251 
> Change-Id: I403a5dcc3cae12b592a444c7e23f71c552b58e1f
> Reviewed-on: https://chromium-review.googlesource.com/c/1334848
> Reviewed-by: Misha Efimov <mef@chromium.org>
> Commit-Queue: John Abd-El-Malek <jam@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#608175}

TBR=mef@chromium.org

Bug:  853251 
Change-Id: Iee089ae0cd422e242df53604da6dcb8ef25b74cb
Reviewed-on: https://chromium-review.googlesource.com/c/1337373
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608281}
[modify] https://crrev.com/506d1d88dd858024e14a5b9565a8940df67c92df/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
[modify] https://crrev.com/506d1d88dd858024e14a5b9565a8940df67c92df/chrome/browser/domain_reliability/service_factory.cc
[modify] https://crrev.com/506d1d88dd858024e14a5b9565a8940df67c92df/chrome/browser/domain_reliability/service_factory.h
[modify] https://crrev.com/506d1d88dd858024e14a5b9565a8940df67c92df/chrome/browser/profiles/profile_impl.cc
[modify] https://crrev.com/506d1d88dd858024e14a5b9565a8940df67c92df/chrome/browser/profiles/profile_impl_io_data.cc
[modify] https://crrev.com/506d1d88dd858024e14a5b9565a8940df67c92df/components/domain_reliability/BUILD.gn
[modify] https://crrev.com/506d1d88dd858024e14a5b9565a8940df67c92df/components/domain_reliability/DEPS
[modify] https://crrev.com/506d1d88dd858024e14a5b9565a8940df67c92df/components/domain_reliability/monitor.cc
[modify] https://crrev.com/506d1d88dd858024e14a5b9565a8940df67c92df/components/domain_reliability/monitor.h
[modify] https://crrev.com/506d1d88dd858024e14a5b9565a8940df67c92df/components/domain_reliability/monitor_unittest.cc
[modify] https://crrev.com/506d1d88dd858024e14a5b9565a8940df67c92df/components/domain_reliability/service.cc
[modify] https://crrev.com/506d1d88dd858024e14a5b9565a8940df67c92df/components/domain_reliability/service.h
[modify] https://crrev.com/506d1d88dd858024e14a5b9565a8940df67c92df/components/domain_reliability/service_unittest.cc

Project Member

Comment 29 by bugdroid1@chromium.org, Nov 17

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

commit aec73c860b15a0873431e32913e7e536f929da68
Author: John Abd-El-Malek <jam@chromium.org>
Date: Sat Nov 17 00:07:11 2018

Instantiate Domain Reliability code in NetworkContext so it works with and without the network service.

Bug:  853251 
Change-Id: I34a5d73c7fe2e3500fe0d1b345f10c2433a7e202
Reviewed-on: https://chromium-review.googlesource.com/c/1338575
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Misha Efimov <mef@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609036}
[modify] https://crrev.com/aec73c860b15a0873431e32913e7e536f929da68/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc
[modify] https://crrev.com/aec73c860b15a0873431e32913e7e536f929da68/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h
[modify] https://crrev.com/aec73c860b15a0873431e32913e7e536f929da68/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_factory.cc
[modify] https://crrev.com/aec73c860b15a0873431e32913e7e536f929da68/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
[modify] https://crrev.com/aec73c860b15a0873431e32913e7e536f929da68/chrome/browser/domain_reliability/browsertest.cc
[modify] https://crrev.com/aec73c860b15a0873431e32913e7e536f929da68/chrome/browser/domain_reliability/service_factory.cc
[modify] https://crrev.com/aec73c860b15a0873431e32913e7e536f929da68/chrome/browser/domain_reliability/service_factory.h
[modify] https://crrev.com/aec73c860b15a0873431e32913e7e536f929da68/chrome/browser/net/chrome_network_delegate.cc
[modify] https://crrev.com/aec73c860b15a0873431e32913e7e536f929da68/chrome/browser/net/chrome_network_delegate.h
[modify] https://crrev.com/aec73c860b15a0873431e32913e7e536f929da68/chrome/browser/net/profile_network_context_service.cc
[modify] https://crrev.com/aec73c860b15a0873431e32913e7e536f929da68/chrome/browser/net/profile_network_context_service.h
[modify] https://crrev.com/aec73c860b15a0873431e32913e7e536f929da68/chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc
[modify] https://crrev.com/aec73c860b15a0873431e32913e7e536f929da68/chrome/browser/profiles/profile_impl.cc
[modify] https://crrev.com/aec73c860b15a0873431e32913e7e536f929da68/chrome/browser/profiles/profile_impl.h
[modify] https://crrev.com/aec73c860b15a0873431e32913e7e536f929da68/chrome/browser/profiles/profile_impl_io_data.cc
[modify] https://crrev.com/aec73c860b15a0873431e32913e7e536f929da68/chrome/browser/profiles/profile_impl_io_data.h
[modify] https://crrev.com/aec73c860b15a0873431e32913e7e536f929da68/chrome/browser/profiles/profile_io_data.cc
[modify] https://crrev.com/aec73c860b15a0873431e32913e7e536f929da68/chrome/browser/profiles/profile_io_data.h
[modify] https://crrev.com/aec73c860b15a0873431e32913e7e536f929da68/chrome/browser/ui/webui/domain_reliability_internals_ui.cc
[modify] https://crrev.com/aec73c860b15a0873431e32913e7e536f929da68/chrome/browser/ui/webui/domain_reliability_internals_ui.h
[modify] https://crrev.com/aec73c860b15a0873431e32913e7e536f929da68/components/domain_reliability/BUILD.gn
[modify] https://crrev.com/aec73c860b15a0873431e32913e7e536f929da68/components/domain_reliability/monitor.cc
[modify] https://crrev.com/aec73c860b15a0873431e32913e7e536f929da68/components/domain_reliability/monitor.h
[modify] https://crrev.com/aec73c860b15a0873431e32913e7e536f929da68/components/domain_reliability/monitor_unittest.cc
[delete] https://crrev.com/8244a03c99ff784241adc0ac3a9cbef599900582/components/domain_reliability/service.cc
[delete] https://crrev.com/8244a03c99ff784241adc0ac3a9cbef599900582/components/domain_reliability/service.h
[delete] https://crrev.com/8244a03c99ff784241adc0ac3a9cbef599900582/components/domain_reliability/service_unittest.cc
[modify] https://crrev.com/aec73c860b15a0873431e32913e7e536f929da68/content/browser/storage_partition_impl.cc
[modify] https://crrev.com/aec73c860b15a0873431e32913e7e536f929da68/content/browser/storage_partition_impl.h
[modify] https://crrev.com/aec73c860b15a0873431e32913e7e536f929da68/services/network/BUILD.gn
[modify] https://crrev.com/aec73c860b15a0873431e32913e7e536f929da68/services/network/DEPS
[modify] https://crrev.com/aec73c860b15a0873431e32913e7e536f929da68/services/network/network_context.cc
[modify] https://crrev.com/aec73c860b15a0873431e32913e7e536f929da68/services/network/network_context.h
[modify] https://crrev.com/aec73c860b15a0873431e32913e7e536f929da68/services/network/public/mojom/network_context.mojom
[modify] https://crrev.com/aec73c860b15a0873431e32913e7e536f929da68/services/network/test/test_network_context.h
[modify] https://crrev.com/aec73c860b15a0873431e32913e7e536f929da68/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

Status: Fixed (was: Started)

Sign in to add a comment