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

Issue 778814 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Bug
Proj-Servicification

Blocking:
issue 769401



Sign in to add a comment

null NavigationData is given to NavigationURLLoaderDelegate::OnResponseStarted() as parameter

Project Member Reported by juncai@chromium.org, Oct 26 2017

Issue description

When 
ChromeResourceDispatcherHostDelegateBrowserTest.NavigationDataProcessed browser test runs with "--enable-features=NetworkService" flag, it crashes at: DidFinishNavigationObserver::DidFinishNavigation()
because |data| is nullptr, since null NavigationData is given to NavigationURLLoaderDelegate::OnResponseStarted() as a parameter:
https://cs.chromium.org/chromium/src/content/browser/loader/navigation_url_loader_network_service.cc?l=618

 

Comment 1 by mmenke@chromium.org, Oct 26 2017

Blocking: 769401

Comment 2 by laforge@google.com, Nov 7 2017

Components: -Internals>Network>Service Internals>Services>Network
Apologies, applied the wrong component in bulk.

Comment 3 by juncai@chromium.org, Mar 16 2018

Cc: -juncai@chromium.org
Owner: juncai@chromium.org
Status: Assigned (was: Available)

Comment 4 by juncai@chromium.org, Mar 16 2018

Status: Started (was: Assigned)

Comment 5 by juncai@chromium.org, Mar 16 2018

This issue is likely related to:
https://bugs.chromium.org/p/chromium/issues/detail?id=705744

Comment 6 by juncai@chromium.org, Mar 16 2018

Cc: juncai@chromium.org
Owner: ----
Status: Available (was: Started)

Comment 7 by jam@chromium.org, May 17 2018

Owner: jam@chromium.org
Status: Assigned (was: Available)
I looked into this a bit and it seems to be caused by the fact that we don't set NavigationData in NavigationURLLoaderImpl::URLLoaderRequestController::OnReceiveResponse if the network service is running (https://cs.chromium.org/chromium/src/content/browser/loader/navigation_url_loader_impl.cc?sq=package:chromium&dr&g=0&l=897).

I'm not sure what to do about this though. Do we need to set NavigationData? If so, is it ok to get it from "ResourceDispatcherHostImpl::Get()->delegate()->GetNavigationData()", or does that not make sense with the network service? If not, do we update the test assertion?

Comment 9 by dxie@google.com, Jun 7 2018

Labels: Proj-Servicification-Canary

Comment 10 by jam@chromium.org, Jun 8 2018

With the network service, RDH shouldn't be exercised (although it still is sometimes). So NavigationData doesn't make sense to have, and most tests that exercise part of RDH won't make sense with the network service.
Project Member

Comment 11 by bugdroid1@chromium.org, Jun 26 2018

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

commit 8202f4a3f317e80baf30659dc7445011a7515bff
Author: John Abd-El-Malek <jam@chromium.org>
Date: Tue Jun 26 19:12:18 2018

Don't run ChromeResourceDispatcherHostDelegateBrowserTest.NavigationDataProcessed with network service.

The code path it's testing isn't used with network service.

Bug:  778814 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: Ib06d13e3c62beb59094cdb691dfb5f55ade38e04
Reviewed-on: https://chromium-review.googlesource.com/1115355
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570483}
[modify] https://crrev.com/8202f4a3f317e80baf30659dc7445011a7515bff/chrome/browser/loader/chrome_resource_dispatcher_host_delegate_browsertest.cc
[modify] https://crrev.com/8202f4a3f317e80baf30659dc7445011a7515bff/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

Comment 12 by jam@chromium.org, Jun 27 2018

Status: Fixed (was: Assigned)

Sign in to add a comment