Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1718463002:
Gracefully handle nested eventSender.beginDragWithFiles() attempts. (Closed)
|
Created:
4 years, 10 months ago by sof Modified:
4 years, 10 months ago CC:
dcheng, blink-reviews, chromium-reviews, jochen+watch_chromium.org, mlamouri+watch-test-runner_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionGracefully handle nested eventSender.beginDragWithFiles() attempts.
Programmatic drag operations in test code may attempt to initiate nested
file drag operations, something eventSender nor anyone else is prepared
for. Throw an error and cancel the current drag operation to discourage
(fuzzer?) code from attempting this.
R=dcheng,mkwst
BUG=479216
Committed: https://crrev.com/23324ffc7cb20cc4e4615946cc44e9c2a282da13
Cr-Commit-Position: refs/heads/master@{#376733}
Patch Set 1 #
Total comments: 4
Patch Set 2 : no need for test to be async #
Messages
Total messages: 25 (9 generated)
sigbjornf@opera.com changed reviewers: + dcheng@chromium.org, rbyers@chromium.org
please take a look. This one is just generating too much fuzzer noise & overhead.
https://codereview.chromium.org/1718463002/diff/1/third_party/WebKit/LayoutTe... File third_party/WebKit/LayoutTests/fast/events/drag-nested-eventSender-use.html (right): https://codereview.chromium.org/1718463002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/fast/events/drag-nested-eventSender-use.html:8: window.jsTestIsAsync = true; Does the test need to be async?
https://codereview.chromium.org/1718463002/diff/1/third_party/WebKit/LayoutTe... File third_party/WebKit/LayoutTests/fast/events/drag-nested-eventSender-use.html (right): https://codereview.chromium.org/1718463002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/fast/events/drag-nested-eventSender-use.html:8: window.jsTestIsAsync = true; On 2016/02/19 18:16:27, dcheng wrote: > Does the test need to be async? Most tests in this directory are keyed off onload, so I assume drag testing was. I can try to change..does it matter much?
https://codereview.chromium.org/1718463002/diff/1/third_party/WebKit/LayoutTe... File third_party/WebKit/LayoutTests/fast/events/drag-nested-eventSender-use.html (right): https://codereview.chromium.org/1718463002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/fast/events/drag-nested-eventSender-use.html:8: window.jsTestIsAsync = true; On 2016/02/19 at 18:37:48, sof wrote: > On 2016/02/19 18:16:27, dcheng wrote: > > Does the test need to be async? > > Most tests in this directory are keyed off onload, so I assume drag testing was. I can try to change..does it matter much? If the test runs synchronously in onload, making it async shouldn't be necessary. It's not strictly required, but it's nice to have a slightly simpler test.
https://codereview.chromium.org/1718463002/diff/1/third_party/WebKit/LayoutTe... File third_party/WebKit/LayoutTests/fast/events/drag-nested-eventSender-use.html (right): https://codereview.chromium.org/1718463002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/fast/events/drag-nested-eventSender-use.html:8: window.jsTestIsAsync = true; On 2016/02/19 18:39:21, dcheng wrote: > On 2016/02/19 at 18:37:48, sof wrote: > > On 2016/02/19 18:16:27, dcheng wrote: > > > Does the test need to be async? > > > > Most tests in this directory are keyed off onload, so I assume drag testing > was. I can try to change..does it matter much? > > If the test runs synchronously in onload, making it async shouldn't be > necessary. > > It's not strictly required, but it's nice to have a slightly simpler test. De-async'ified the test; works fine.
lgtm
Description was changed from ========== Gracefully handle nested eventSender.beginDragWithFiles() attempts. Programmatic drag operations in test code may attempt to initiate nested file drag operations, something eventSender nor anyone else is prepared for. Throw an error and cancel the current drag operation to discourage (fuzzer?) code from attempting this. R= BUG=479216 ========== to ========== Gracefully handle nested eventSender.beginDragWithFiles() attempts. Programmatic drag operations in test code may attempt to initiate nested file drag operations, something eventSender nor anyone else is prepared for. Throw an error and cancel the current drag operation to discourage (fuzzer?) code from attempting this. R=dcheng BUG=479216 ==========
The CQ bit was checked by sigbjornf@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1718463002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1718463002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
On 2016/02/19 21:23:34, commit-bot: I haz the power wrote: > Try jobs failed on following builders: > chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, > http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...) That's a surprise; rbyers@ - owner's review when you have the time? tia.
sigbjornf@opera.com changed reviewers: + mkwst@chromium.org
mkwst@: (same timezone) owner approval? tia
On 2016/02/22 at 06:39:03, sigbjornf wrote: > mkwst@: (same timezone) owner approval? tia This is such a nice timezone, isn't it? Much better than wherever those other OWNERS find themselves. LGTM.
On 2016/02/22 10:08:18, Mike West wrote: > On 2016/02/22 at 06:39:03, sigbjornf wrote: > > mkwst@: (same timezone) owner approval? tia > > This is such a nice timezone, isn't it? Much better than wherever those other > OWNERS find themselves. > > LGTM. I think you're right, the last bit convinced me. thanks :)
Description was changed from ========== Gracefully handle nested eventSender.beginDragWithFiles() attempts. Programmatic drag operations in test code may attempt to initiate nested file drag operations, something eventSender nor anyone else is prepared for. Throw an error and cancel the current drag operation to discourage (fuzzer?) code from attempting this. R=dcheng BUG=479216 ========== to ========== Gracefully handle nested eventSender.beginDragWithFiles() attempts. Programmatic drag operations in test code may attempt to initiate nested file drag operations, something eventSender nor anyone else is prepared for. Throw an error and cancel the current drag operation to discourage (fuzzer?) code from attempting this. R=dcheng,mkwst BUG=479216 ==========
The CQ bit was checked by sigbjornf@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1718463002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1718463002/20001
Also LGTM (personally I find UTC-5 to be nicely between europe and California <grin>).
Message was sent while issue was closed.
Description was changed from ========== Gracefully handle nested eventSender.beginDragWithFiles() attempts. Programmatic drag operations in test code may attempt to initiate nested file drag operations, something eventSender nor anyone else is prepared for. Throw an error and cancel the current drag operation to discourage (fuzzer?) code from attempting this. R=dcheng,mkwst BUG=479216 ========== to ========== Gracefully handle nested eventSender.beginDragWithFiles() attempts. Programmatic drag operations in test code may attempt to initiate nested file drag operations, something eventSender nor anyone else is prepared for. Throw an error and cancel the current drag operation to discourage (fuzzer?) code from attempting this. R=dcheng,mkwst BUG=479216 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Gracefully handle nested eventSender.beginDragWithFiles() attempts. Programmatic drag operations in test code may attempt to initiate nested file drag operations, something eventSender nor anyone else is prepared for. Throw an error and cancel the current drag operation to discourage (fuzzer?) code from attempting this. R=dcheng,mkwst BUG=479216 ========== to ========== Gracefully handle nested eventSender.beginDragWithFiles() attempts. Programmatic drag operations in test code may attempt to initiate nested file drag operations, something eventSender nor anyone else is prepared for. Throw an error and cancel the current drag operation to discourage (fuzzer?) code from attempting this. R=dcheng,mkwst BUG=479216 Committed: https://crrev.com/23324ffc7cb20cc4e4615946cc44e9c2a282da13 Cr-Commit-Position: refs/heads/master@{#376733} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/23324ffc7cb20cc4e4615946cc44e9c2a282da13 Cr-Commit-Position: refs/heads/master@{#376733} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
