Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2496933002:
XMLHttpRequest: implement "send() flag" tracking and updating per spec. (Closed)
DescriptionXMLHttpRequest: implement "send() flag" tracking and updating per spec.
The implementation has until now tracked/approximated the spec's
"send() flag"[1] by checking if the XMLHttpRequest object had an active
loader. That object does not have lifetime equal to what the spec
requires for the "send() flag", nor is the loader set for sync XHR
send()s.
There's no good reason to hold out on tracking this flag per spec,
so introduce it here.
[1] - https://xhr.spec.whatwg.org/#send-flag
R=yhirano
BUG=649516
Committed: https://crrev.com/78363083b0281dbc05ba3ce3c73eec1bf9dcc9b1
Cr-Commit-Position: refs/heads/master@{#432148}
Patch Set 1 #
Total comments: 2
Patch Set 2 : add recursive send() test #
Total comments: 2
Patch Set 3 : add missing field initialization #
Messages
Total messages: 29 (18 generated)
The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
sigbjornf@opera.com changed reviewers: + tyoshino@chromium.org, yhirano@chromium.org
hi, please take a look. I think it's time to just do this.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Thank you for working on this issue. Can you add a recursive send test case? https://codereview.chromium.org/2496933002/diff/1/third_party/WebKit/LayoutTe... File third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/withCredentials-after-send.html (right): https://codereview.chromium.org/2496933002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/withCredentials-after-send.html:23: testAsync.done(); Not needed?
The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== XMLHttpRequest: implement "send() flag" tracking and updating per spec. The implementation has until now tracked/approximated the spec's "send() flag"[1] by checking if the XMLHttpRequest object had an active loader. That object does not have lifetime equal to what the spec requires for the "send() flag", nor is the loader set for sync XHR send()s. There's no good reason to hold out on tracking this flag per spec, so introduce it here. [1] - https://xhr.spec.whatwg.org/#send-flag R= BUG= ========== to ========== XMLHttpRequest: implement "send() flag" tracking and updating per spec. The implementation has until now tracked/approximated the spec's "send() flag"[1] by checking if the XMLHttpRequest object had an active loader. That object does not have lifetime equal to what the spec requires for the "send() flag", nor is the loader set for sync XHR send()s. There's no good reason to hold out on tracking this flag per spec, so introduce it here. [1] - https://xhr.spec.whatwg.org/#send-flag R= BUG=649516 ==========
On 2016/11/14 06:45:22, yhirano wrote: > Thank you for working on this issue. > > Can you add a recursive send test case? > Added; when send() flag tracking is in place, issue 642242 should also be (readily) addressable. But before that time such abort() usage will continue to assert.
https://codereview.chromium.org/2496933002/diff/1/third_party/WebKit/LayoutTe... File third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/withCredentials-after-send.html (right): https://codereview.chromium.org/2496933002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/withCredentials-after-send.html:23: testAsync.done(); On 2016/11/14 06:45:22, yhirano wrote: > Not needed? Yes, unintentionally crept in.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2496933002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp (right): https://codereview.chromium.org/2496933002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp:249: m_responseTextOverflow(false) {} Please initialize the flag.
The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2496933002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp (right): https://codereview.chromium.org/2496933002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp:249: m_responseTextOverflow(false) {} On 2016/11/15 02:26:08, yhirano wrote: > Please initialize the flag. oops, thanks - done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
Description was changed from ========== XMLHttpRequest: implement "send() flag" tracking and updating per spec. The implementation has until now tracked/approximated the spec's "send() flag"[1] by checking if the XMLHttpRequest object had an active loader. That object does not have lifetime equal to what the spec requires for the "send() flag", nor is the loader set for sync XHR send()s. There's no good reason to hold out on tracking this flag per spec, so introduce it here. [1] - https://xhr.spec.whatwg.org/#send-flag R= BUG=649516 ========== to ========== XMLHttpRequest: implement "send() flag" tracking and updating per spec. The implementation has until now tracked/approximated the spec's "send() flag"[1] by checking if the XMLHttpRequest object had an active loader. That object does not have lifetime equal to what the spec requires for the "send() flag", nor is the loader set for sync XHR send()s. There's no good reason to hold out on tracking this flag per spec, so introduce it here. [1] - https://xhr.spec.whatwg.org/#send-flag R=yhirano BUG=649516 ==========
The CQ bit was checked by sigbjornf@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm
Message was sent while issue was closed.
Description was changed from ========== XMLHttpRequest: implement "send() flag" tracking and updating per spec. The implementation has until now tracked/approximated the spec's "send() flag"[1] by checking if the XMLHttpRequest object had an active loader. That object does not have lifetime equal to what the spec requires for the "send() flag", nor is the loader set for sync XHR send()s. There's no good reason to hold out on tracking this flag per spec, so introduce it here. [1] - https://xhr.spec.whatwg.org/#send-flag R=yhirano BUG=649516 ========== to ========== XMLHttpRequest: implement "send() flag" tracking and updating per spec. The implementation has until now tracked/approximated the spec's "send() flag"[1] by checking if the XMLHttpRequest object had an active loader. That object does not have lifetime equal to what the spec requires for the "send() flag", nor is the loader set for sync XHR send()s. There's no good reason to hold out on tracking this flag per spec, so introduce it here. [1] - https://xhr.spec.whatwg.org/#send-flag R=yhirano BUG=649516 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== XMLHttpRequest: implement "send() flag" tracking and updating per spec. The implementation has until now tracked/approximated the spec's "send() flag"[1] by checking if the XMLHttpRequest object had an active loader. That object does not have lifetime equal to what the spec requires for the "send() flag", nor is the loader set for sync XHR send()s. There's no good reason to hold out on tracking this flag per spec, so introduce it here. [1] - https://xhr.spec.whatwg.org/#send-flag R=yhirano BUG=649516 ========== to ========== XMLHttpRequest: implement "send() flag" tracking and updating per spec. The implementation has until now tracked/approximated the spec's "send() flag"[1] by checking if the XMLHttpRequest object had an active loader. That object does not have lifetime equal to what the spec requires for the "send() flag", nor is the loader set for sync XHR send()s. There's no good reason to hold out on tracking this flag per spec, so introduce it here. [1] - https://xhr.spec.whatwg.org/#send-flag R=yhirano BUG=649516 Committed: https://crrev.com/78363083b0281dbc05ba3ce3c73eec1bf9dcc9b1 Cr-Commit-Position: refs/heads/master@{#432148} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/78363083b0281dbc05ba3ce3c73eec1bf9dcc9b1 Cr-Commit-Position: refs/heads/master@{#432148} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
