New issue
Advanced search Search tips

Issue 856651 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

WebSocket bufferedAmount can be deducted after an onmessage callback

Project Member Reported by ricea@chromium.org, Jun 26 2018

Issue description

The test https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/external/wpt/websockets/interfaces/WebSocket/bufferedAmount/bufferedAmount-getting.html fails about one time in 200. It appears that the onmessage callback can run before bufferedAmount is actually deducted. Probably the timer buffered_amount_consume_timer_ is serviced after mojo events.
 

Comment 1 by ricea@chromium.org, Jun 28 2018

Probably a simple fix would be to call ReflectBufferedAmountConsumption before firing the onmessage event.
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 2

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

commit cd9e2f9f54ada77635d77821427bc8f2c4eee051
Author: Adam Rice <ricea@chromium.org>
Date: Thu Aug 02 03:59:07 2018

[WebSocket] Don't use timers to post tasks

Replace the use of zero-length timers in blink::DOMWebSocket with direct
use of the PostTask API. This slightly improves clarity and efficiency.

BUG= 856651 

Change-Id: Ib6003fb9861a59513477eafcaa07e02eacef1271
Reviewed-on: https://chromium-review.googlesource.com/1151045
Commit-Queue: Adam Rice <ricea@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580053}
[modify] https://crrev.com/cd9e2f9f54ada77635d77821427bc8f2c4eee051/third_party/blink/renderer/modules/websockets/dom_websocket.cc
[modify] https://crrev.com/cd9e2f9f54ada77635d77821427bc8f2c4eee051/third_party/blink/renderer/modules/websockets/dom_websocket.h

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 2

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

commit 6f7b2afb2209712e3867f2f56d451a8db556f39a
Author: Adam Rice <ricea@chromium.org>
Date: Thu Aug 02 05:27:18 2018

Update buffered amount when async callbacks are called

If an asynchronous callback is called, it means we must have returned to
the start of the event loop. Ensure that any consumed bufferedAmount is
reflected in that case. Do not reflect bufferedAmount if the EventQueue
is paused, as that means that we may be in a nested event loop.

Add a unit test for this case. Also add a unit test for normal
bufferedAmount behaviour, as there wasn't one.

Add a web platform test for what happens if a sync XHR is performed
between calling send() and looking at bufferedAmount.

BUG= 856651 

Change-Id: Iafa2d619a1eb5284b64500ac03d336fb6380193b
Reviewed-on: https://chromium-review.googlesource.com/1151086
Commit-Queue: Adam Rice <ricea@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580078}
[add] https://crrev.com/6f7b2afb2209712e3867f2f56d451a8db556f39a/third_party/WebKit/LayoutTests/external/wpt/websockets/bufferedAmount-unchanged-by-sync-xhr.any.js
[modify] https://crrev.com/6f7b2afb2209712e3867f2f56d451a8db556f39a/third_party/blink/renderer/modules/websockets/dom_websocket.cc
[modify] https://crrev.com/6f7b2afb2209712e3867f2f56d451a8db556f39a/third_party/blink/renderer/modules/websockets/dom_websocket.h
[modify] https://crrev.com/6f7b2afb2209712e3867f2f56d451a8db556f39a/third_party/blink/renderer/modules/websockets/dom_websocket_test.cc

Status: Fixed (was: Assigned)

Sign in to add a comment