Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1455513002:
[XHR] Decrease readyStateChange event dispatching during loading. (Closed)
Description[XHR] Decrease readyStateChange event dispatching during loading.
XMLHttpRequest dispatches readyStateChange while loading despite
readyState does not change, in order to provide progress information.
This CL decreases the number of events during loading to increase performance.
This change removes a wrong comment from
XMLHttpRequest::dispatchReadyStateChangeEvent (In fact I added the
comment at [1] but I don't remember the intention).
1: https://chromium.googlesource.com/chromium/src/+/9222a901a9ece4c0724622b5caea40bf2d879ce0
BUG=336066
Committed: https://crrev.com/edd3817ef4a1c3a41610855bfc2703b55359d444
Cr-Commit-Position: refs/heads/master@{#361995}
Patch Set 1 #Patch Set 2 : #Patch Set 3 : #Patch Set 4 : rebase #
Total comments: 2
Patch Set 5 : #
Messages
Total messages: 21 (10 generated)
Description was changed from ========== [XHR] Decrease the number of readyStateChange dispatch during loading. XMLHttpRequest dispatched readyStateChange while loading despite in spite of readyState does not change, in order to provide progress information. This CL decreases the number of events during loading to increase performance. BUG=336066 ========== to ========== [XHR] Decrease readyStateChange dispatching during loading. XMLHttpRequest dispatched readyStateChange while loading despite in spite of readyState does not change, in order to provide progress information. This CL decreases the number of events during loading to increase performance. BUG=336066 ==========
Description was changed from ========== [XHR] Decrease readyStateChange dispatching during loading. XMLHttpRequest dispatched readyStateChange while loading despite in spite of readyState does not change, in order to provide progress information. This CL decreases the number of events during loading to increase performance. BUG=336066 ========== to ========== [XHR] Decrease readyStateChange event dispatching during loading. XMLHttpRequest dispatched readyStateChange while loading despite in spite of readyState does not change, in order to provide progress information. This CL decreases the number of events during loading to increase performance. BUG=336066 ==========
Description was changed from ========== [XHR] Decrease readyStateChange event dispatching during loading. XMLHttpRequest dispatched readyStateChange while loading despite in spite of readyState does not change, in order to provide progress information. This CL decreases the number of events during loading to increase performance. BUG=336066 ========== to ========== [XHR] Decrease readyStateChange event dispatching during loading. XMLHttpRequest dispatches readyStateChange while loading despite readyState does not change, in order to provide progress information. This CL decreases the number of events during loading to increase performance. BUG=336066 ==========
yhirano@chromium.org changed reviewers: + tyoshino@chromium.org
https://codereview.chromium.org/1455513002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequestProgressEventThrottle.cpp (right): https://codereview.chromium.org/1455513002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequestProgressEventThrottle.cpp:124: { are we sure that we can omit "if (!executionContext())" that we have in XMLHttpRequest::dispatchReadyStateChangeEvent() now
Description was changed from ========== [XHR] Decrease readyStateChange event dispatching during loading. XMLHttpRequest dispatches readyStateChange while loading despite readyState does not change, in order to provide progress information. This CL decreases the number of events during loading to increase performance. BUG=336066 ========== to ========== [XHR] Decrease readyStateChange event dispatching during loading. XMLHttpRequest dispatches readyStateChange while loading despite readyState does not change, in order to provide progress information. This CL decreases the number of events during loading to increase performance. This change removes a wrong comment from XMLHttpRequest::dispatchReadyStateChangeEvent (In fact I added the comment at [1] but I don't remember the intention). 1: https://chromium.googlesource.com/chromium/src/+/9222a901a9ece4c0724622b5caea... BUG=336066 ==========
Description was changed from ========== [XHR] Decrease readyStateChange event dispatching during loading. XMLHttpRequest dispatches readyStateChange while loading despite readyState does not change, in order to provide progress information. This CL decreases the number of events during loading to increase performance. This change removes a wrong comment from XMLHttpRequest::dispatchReadyStateChangeEvent (In fact I added the comment at [1] but I don't remember the intention). 1: https://chromium.googlesource.com/chromium/src/+/9222a901a9ece4c0724622b5caea... BUG=336066 ========== to ========== [XHR] Decrease readyStateChange event dispatching during loading. XMLHttpRequest dispatches readyStateChange while loading despite readyState does not change, in order to provide progress information. This CL decreases the number of events during loading to increase performance. This change removes a wrong comment from XMLHttpRequest::dispatchReadyStateChangeEvent (In fact I added the comment at [1] but I don't remember the intention). 1: https://chromium.googlesource.com/chromium/src/+/9222a901a9ece4c0724622b5caea... BUG=336066 ==========
https://codereview.chromium.org/1455513002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequestProgressEventThrottle.cpp (right): https://codereview.chromium.org/1455513002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequestProgressEventThrottle.cpp:124: { On 2015/11/26 07:04:13, tyoshino wrote: > are we sure that we can omit "if (!executionContext())" that we have in > XMLHttpRequest::dispatchReadyStateChangeEvent() now > I'm not sure what the block is for. I added a |stop| call in XMLHttpRequest::stop in order to stop the timer when the execution context is stopped.
lgtm
The CQ bit was checked by yhirano@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1455513002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1455513002/80001
Message was sent while issue was closed.
Description was changed from ========== [XHR] Decrease readyStateChange event dispatching during loading. XMLHttpRequest dispatches readyStateChange while loading despite readyState does not change, in order to provide progress information. This CL decreases the number of events during loading to increase performance. This change removes a wrong comment from XMLHttpRequest::dispatchReadyStateChangeEvent (In fact I added the comment at [1] but I don't remember the intention). 1: https://chromium.googlesource.com/chromium/src/+/9222a901a9ece4c0724622b5caea... BUG=336066 ========== to ========== [XHR] Decrease readyStateChange event dispatching during loading. XMLHttpRequest dispatches readyStateChange while loading despite readyState does not change, in order to provide progress information. This CL decreases the number of events during loading to increase performance. This change removes a wrong comment from XMLHttpRequest::dispatchReadyStateChangeEvent (In fact I added the comment at [1] but I don't remember the intention). 1: https://chromium.googlesource.com/chromium/src/+/9222a901a9ece4c0724622b5caea... BUG=336066 ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== [XHR] Decrease readyStateChange event dispatching during loading. XMLHttpRequest dispatches readyStateChange while loading despite readyState does not change, in order to provide progress information. This CL decreases the number of events during loading to increase performance. This change removes a wrong comment from XMLHttpRequest::dispatchReadyStateChangeEvent (In fact I added the comment at [1] but I don't remember the intention). 1: https://chromium.googlesource.com/chromium/src/+/9222a901a9ece4c0724622b5caea... BUG=336066 ========== to ========== [XHR] Decrease readyStateChange event dispatching during loading. XMLHttpRequest dispatches readyStateChange while loading despite readyState does not change, in order to provide progress information. This CL decreases the number of events during loading to increase performance. This change removes a wrong comment from XMLHttpRequest::dispatchReadyStateChangeEvent (In fact I added the comment at [1] but I don't remember the intention). 1: https://chromium.googlesource.com/chromium/src/+/9222a901a9ece4c0724622b5caea... BUG=336066 Committed: https://crrev.com/edd3817ef4a1c3a41610855bfc2703b55359d444 Cr-Commit-Position: refs/heads/master@{#361995} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/edd3817ef4a1c3a41610855bfc2703b55359d444 Cr-Commit-Position: refs/heads/master@{#361995}
Message was sent while issue was closed.
sigbjornf@opera.com changed reviewers: + sigbjornf@opera.com
Message was sent while issue was closed.
http/tests/xmlhttprequest/interactive-state.html seems to have become flaky following this -- http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.8/builds/... http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=htt...
Message was sent while issue was closed.
On 2015/11/30 10:27:52, sof wrote: > http/tests/xmlhttprequest/interactive-state.html seems to have become flaky > following this -- > > > http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.8/builds/... > > http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=htt... I've observed this in submitting another CL on XHR. I'm investigating it.
Message was sent while issue was closed.
On 2015/11/30 11:32:29, tyoshino wrote: > On 2015/11/30 10:27:52, sof wrote: > > http/tests/xmlhttprequest/interactive-state.html seems to have become flaky > > following this -- > > > > > > > http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.8/builds/... > > > > > http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=htt... > > I've observed this in submitting another CL on XHR. I'm investigating it. Done https://codereview.chromium.org/1484743002/
Message was sent while issue was closed.
On 2015/11/30 12:14:22, tyoshino wrote: > On 2015/11/30 11:32:29, tyoshino wrote: > > On 2015/11/30 10:27:52, sof wrote: > > > http/tests/xmlhttprequest/interactive-state.html seems to have become flaky > > > following this -- > > > > > > > > > > > > http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.8/builds/... > > > > > > > > > http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=htt... > > > > I've observed this in submitting another CL on XHR. I'm investigating it. > > Done > https://codereview.chromium.org/1484743002/ Landed. Confirmed that they are green again. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
