New issue
Advanced search Search tips

Issue 754827 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 3
Type: Bug
Proj-Servicification

Blocking:
issue 598073



Sign in to add a comment

Synchronous XHR broken by Network Service

Project Member Reported by reillyg@chromium.org, Aug 11 2017

Issue description

Switching synchronous XMLHttpRequests to using the asynchronous loading path has broken some tests when run with the Network Service.

 

Comment 1 by jam@chromium.org, Aug 11 2017

It never worked in the first place, so your patch didn't break anything!

Comment 2 by jam@chromium.org, Aug 11 2017

Blocking: 598073
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 11 2017

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

commit 1157300ceae86a9b5a2e670cae237de800c10c01
Author: Reilly Grant <reillyg@chromium.org>
Date: Fri Aug 11 22:43:12 2017

Disable sync XHR tests broken with Network Service

This patch disables tests which were broken by r493606 which switched
synchronous XMLHttpRequest to using the asynchronous loading path
instead of its own special IPC message.

NOTRY=true

Bug:  754827 
Change-Id: Iee5ab66efee9b05a14ab40cb660f9eeb27eba4e8
Reviewed-on: https://chromium-review.googlesource.com/612211
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493873}
[modify] https://crrev.com/1157300ceae86a9b5a2e670cae237de800c10c01/testing/buildbot/filters/mojo.fyi.network_content_browsertests.filter
[modify] https://crrev.com/1157300ceae86a9b5a2e670cae237de800c10c01/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService

Comment 4 by ricea@chromium.org, Aug 14 2017

Components: Blink>Network>XHR
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 26 2017

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

commit d35e92ac3a05c531a92b35374208f3fd25fbcd00
Author: Reilly Grant <reillyg@chromium.org>
Date: Sat Aug 26 00:43:13 2017

Triage content_browsertest failures in sync XHRs with network service

ResourceDispatcherHostBrowserTest.SyncXMLHttpRequest_Cancelled is
expected to fail because it relies on behavior of
ResourceDispatcherHostImpl which will be removed by the Network Service
but will be similiar to a process crash.

BrowserSideNavigationBrowserDisableWebSecurityTest.ValidateBaseUrlForDataUrl
fails because file:// URLs are currently handled by the network service
and the cross-origin request is not blocked which is tracked by issue
759230.

ResourceDispatcherHostBrowserTest.SyncXMLHttpRequest now passes.

TBR=jam@chromium.org

Bug:  754827 , 759230 
Change-Id: I856542f6f2ef4d23173f0c74ff76e290cbe0d905
Reviewed-on: https://chromium-review.googlesource.com/636467
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497614}
[modify] https://crrev.com/d35e92ac3a05c531a92b35374208f3fd25fbcd00/testing/buildbot/filters/mojo.fyi.network_content_browsertests.filter

Project Member

Comment 6 by bugdroid1@chromium.org, Sep 1 2017

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

commit 520f84b567ee6165f06d60a0b2c7e8926baf7e42
Author: Reilly Grant <reillyg@chromium.org>
Date: Fri Sep 01 20:47:52 2017

Support URLLoaderThrottles with synchronous XHR

This change restores support for synchronous XMLHttpRequest with the
network service enabled by adding the concept of detaching a
URLLoaderThrottle from the thread on which it was created and later
attaching it to the thread on which the request is being processed.

To make this work for the Safe Browsing throttle a Clone message was
added to the SafeBrowsing interface, allowing a new pipe, bound to the
thread on which the request is being process, to be created.

Bug:  754827 
Change-Id: Ifd212061e95a7f49d3e6238ccc6182231692989c
Reviewed-on: https://chromium-review.googlesource.com/631040
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Reviewed-by: Yuzhu Shen <yzshen@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499286}
[modify] https://crrev.com/520f84b567ee6165f06d60a0b2c7e8926baf7e42/components/safe_browsing/browser/mojo_safe_browsing_impl.cc
[modify] https://crrev.com/520f84b567ee6165f06d60a0b2c7e8926baf7e42/components/safe_browsing/browser/mojo_safe_browsing_impl.h
[modify] https://crrev.com/520f84b567ee6165f06d60a0b2c7e8926baf7e42/components/safe_browsing/common/safe_browsing.mojom
[modify] https://crrev.com/520f84b567ee6165f06d60a0b2c7e8926baf7e42/components/safe_browsing/renderer/renderer_url_loader_throttle.cc
[modify] https://crrev.com/520f84b567ee6165f06d60a0b2c7e8926baf7e42/components/safe_browsing/renderer/renderer_url_loader_throttle.h
[modify] https://crrev.com/520f84b567ee6165f06d60a0b2c7e8926baf7e42/components/safe_browsing/renderer/websocket_sb_handshake_throttle_unittest.cc
[modify] https://crrev.com/520f84b567ee6165f06d60a0b2c7e8926baf7e42/content/child/resource_dispatcher.cc
[modify] https://crrev.com/520f84b567ee6165f06d60a0b2c7e8926baf7e42/content/child/sync_load_context.cc
[modify] https://crrev.com/520f84b567ee6165f06d60a0b2c7e8926baf7e42/content/child/sync_load_context.h
[modify] https://crrev.com/520f84b567ee6165f06d60a0b2c7e8926baf7e42/content/public/common/BUILD.gn
[add] https://crrev.com/520f84b567ee6165f06d60a0b2c7e8926baf7e42/content/public/common/url_loader_throttle.cc
[modify] https://crrev.com/520f84b567ee6165f06d60a0b2c7e8926baf7e42/content/public/common/url_loader_throttle.h

Status: Fixed (was: Assigned)
This work is complete.

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

Components: Internals>Network>Service

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

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

Sign in to add a comment