Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(95)

Issue 1053013006: Revert of Use enum BinaryType for WebSocket.binaryType, instead of DOMString (Closed)

Created:
5 years, 8 months ago by jsbell
Modified:
5 years, 8 months ago
CC:
blink-reviews, yhirano+watch_chromium.org, tyoshino+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Revert of Use enum BinaryType for WebSocket.binaryType, instead of DOMString (patchset #7 id:120001 of https://codereview.chromium.org/871013007/) Reason for revert: Failing in gtests: https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.8/builds/35898/steps/webkit_unit_tests/logs/DOMWebSocketDeathTest.binaryType [WARNING] ../../testing/gtest/src/gtest-death-test.cc:825:: Death tests use fork(), which is unsafe particularly in a threaded context. For this test, Google Test detected 5 threads. ../../third_party/WebKit/Source/modules/websockets/DOMWebSocketTest.cpp:687: Failure Death test: m_websocket->setBinaryType("hoge") Result: failed to die. Error msg: [ DEATH ] [ FAILED ] DOMWebSocketDeathTest.binaryType (5 ms) Original issue's description: > This addresses a FIXME in WebSocket.idl > > Also removing logging for wrong value as now we are logging in binding code: > https://codereview.chromium.org/955413002/. > > Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=193201 TBR=habib.virji@samsung.com,nbarth@chromium.org,yutak@chromium.org,tyoshino@chromium.org,haraken@chromium.org,bashi@chromium.org,paritosh.in@samsung.com NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=193206

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+14 lines, -18 lines) Patch
M LayoutTests/http/tests/websocket/binary-type-expected.txt View 1 chunk +3 lines, -3 lines 0 comments Download
M Source/modules/websockets/DOMWebSocket.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/websockets/DOMWebSocketTest.cpp View 1 chunk +8 lines, -13 lines 0 comments Download
M Source/modules/websockets/WebSocket.idl View 2 chunks +2 lines, -1 line 0 comments Download

Messages

Total messages: 6 (0 generated)
jsbell
Created Revert of Use enum BinaryType for WebSocket.binaryType, instead of DOMString
5 years, 8 months ago (2015-04-06 19:10:36 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1053013006/1
5 years, 8 months ago (2015-04-06 19:10:52 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1) as https://src.chromium.org/viewvc/blink?view=rev&revision=193206
5 years, 8 months ago (2015-04-06 19:11:31 UTC) #3
Nils Barth (inactive)
LGTM. Looks like we can't use death tests?
5 years, 8 months ago (2015-04-06 19:14:06 UTC) #4
jsbell
On 2015/04/06 19:14:06, Nils Barth (inactive) wrote: > LGTM. > Looks like we can't use ...
5 years, 8 months ago (2015-04-06 19:15:56 UTC) #5
haraken
5 years, 8 months ago (2015-04-06 23:21:02 UTC) #6
Message was sent while issue was closed.
LGTM

Powered by Google App Engine
This is Rietveld 408576698