New issue
Advanced search Search tips

Issue 807954 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

DCHECK in external/wpt/service-workers/service-worker/fetch-event-respond-with-readable-stream-chunk.https.html

Project Member Reported by raphael....@intel.com, Feb 1 2018

Issue description

This test, added in https://github.com/w3c/web-platform-tests/pull/9327 and coming from WebKit, is hitting a DCHECK in the bots and blocking WPT imports.

From https://ci.chromium.org/buildbot/tryserver.chromium.linux/linux_chromium_rel_ng/637449:

crash log for renderer (pid <unknown>):
STDOUT: #CRASHED - renderer
STDERR: 
STDERR: DevTools listening on ws://127.0.0.1:46351/devtools/browser/76367735-4c5a-4556-aa38-2ce7b4f504cc
STDERR: Fontconfig warning: "/etc/fonts/fonts.conf", line 146: blank doesn't take any effect anymore. please remove it from your fonts.conf
STDERR: [1:17:0201/020555.851651:FATAL:FetchDataLoader.cpp(521)] Check failed: available > 0UL (0 vs. 0)
STDERR: #0 0x00000314f35c base::debug::StackTrace::StackTrace()
STDERR: #1 0x00000316dd4c logging::LogMessage::~LogMessage()
STDERR: #2 0x00000533b2f0 blink::(anonymous namespace)::FetchDataLoaderAsDataPipe::OnStateChange()
STDERR: #3 0x0000053518a6 blink::ReadableStreamBytesConsumer::OnRead()
STDERR: #4 0x000005351d81 blink::ReadableStreamBytesConsumer::OnFulfilled::Call()
STDERR: #5 0x000004a11a58 blink::ScriptFunction::CallCallback()
STDERR: #6 0x000001ceb2de v8::internal::FunctionCallbackArguments::Call()
STDERR: #7 0x000001dcb980 v8::internal::(anonymous namespace)::HandleApiCallHelper<>()
STDERR: #8 0x000001dc9e49 v8::internal::Builtin_Impl_HandleApiCall()
STDERR: #9 0x000001dc988d v8::internal::Builtin_HandleApiCall()
STDERR: #10 0x0ddaac084404 <unknown>
STDERR: 

I'm going to skip them to allow WPT imports to proceed, but I'm filing a separate bug so that it's easier to track than another entry in the catch-all bug we have for DCHECKs in the bots.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 1 2018

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

commit 7724273cae9b4ebbe85dcc4ad457c540bf9a3930
Author: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Date: Thu Feb 01 11:16:40 2018

Mark upcoming service-workers WPT test as crashing

external/wpt/service-workers/service-worker/fetch-event-respond-with-readable-stream-chunk.https.html
is currently blocking imports because it hits a DCHECK in the CQ bots
running layout tests.

While here, move the
external/wpt/custom-elements/upgrading/Node-cloneNode.html entry together
with the other DCHECK ones.

TBR=falken,mek

Bug:  807954 
Change-Id: I3e4174f74d7c336bb5aecbe30e968c378f9e1be4
No-Try: True
Reviewed-on: https://chromium-review.googlesource.com/897622
Reviewed-by: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Commit-Queue: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Cr-Commit-Position: refs/heads/master@{#533631}
[modify] https://crrev.com/7724273cae9b4ebbe85dcc4ad457c540bf9a3930/third_party/WebKit/LayoutTests/TestExpectations

Project Member

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

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

commit ae1e8023bd48be9fd2c9546b7140e7e535c64884
Author: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Date: Thu Feb 01 14:06:24 2018

Skip the outofblink-cors version of fetch-event-respond-with-readable-stream-chunk.https.html

This should have been part of https://chromium-review.googlesource.com/897622

TBR=falken

Bug:  807954 
Change-Id: I9099d325ca42a8e38862c652abf6db275339442b
No-Try: True
Reviewed-on: https://chromium-review.googlesource.com/897802
Reviewed-by: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Commit-Queue: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Cr-Commit-Position: refs/heads/master@{#533653}
[modify] https://crrev.com/ae1e8023bd48be9fd2c9546b7140e7e535c64884/third_party/WebKit/LayoutTests/TestExpectations

Labels: -Pri-3 Pri-1
Components: Blink>Network>StreamsAPI
This looks readable streams related, but I haven't looked deeply. Adding the component.
Cc: toyoshim@chromium.org
Cc: -toyoshim@chromium.org
Ah, sorry, I misunderstood. Never mind.

Comment 7 by bashi@chromium.org, Feb 20 2018

Owner: yhirano@chromium.org
Status: Assigned (was: Available)
FetchDataLoader isn't consistent with handling |available| returned by ReadableStreamBytesConsumer::BeginRead().

- In FetchDataLoaderAsDataPipe::OnStateChange() it assumes |available| > 0.
- In FetchDataLoaderAsArrayBuffer::OnStateChange() it assumes |available| >= 0.

Which assumption is correct? If >= 0 makes sense, I can create a fix. Assigning to yhirano@ to get feedback. Feel free to assign back to me.
Owner: bashi@chromium.org
Project Member

Comment 9 by bugdroid1@chromium.org, Jun 15 2018

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

commit 48d1e0391a7d175d828ec26a56c5ec3473ea70aa
Author: Kenichi Ishibashi <bashi@chromium.org>
Date: Fri Jun 15 00:50:57 2018

Allow empty read in FetchDataLoaderAsDataPipe

ByteConsumer::BeginRead() may return Result::kOK with no available
read data. FetchDataLoaderAsDataPipe should handle zero read as other
ByteConsumer::Client do.

Bug:  807954 
Change-Id: I47186ccc01d10c8ff18bdf749376598ee75342a7
Reviewed-on: https://chromium-review.googlesource.com/1096515
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567503}
[modify] https://crrev.com/48d1e0391a7d175d828ec26a56c5ec3473ea70aa/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/48d1e0391a7d175d828ec26a56c5ec3473ea70aa/third_party/blink/renderer/core/fetch/fetch_data_loader.cc

Comment 10 by bashi@chromium.org, Jun 28 2018

Status: Fixed (was: Assigned)

Sign in to add a comment