Bug 218546

Summary: window.event may get set on wrong global when dispatching an event
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: DOMAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, calvaris, clopez, darin, ews-watchlist, ggaren, sam, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
URL: https://dom.spec.whatwg.org/#concept-event-listener-inner-invoke
Attachments:
Description Flags
WIP Patch
none
Patch
none
Patch
none
Patch
none
Patch
sam: review+, ews-feeder: commit-queue-
Patch none

Chris Dumez
Reported 2020-11-03 16:40:39 PST
window.event may get set on wrong global when dispatching an event.
Attachments
WIP Patch (3.24 KB, patch)
2020-11-03 16:41 PST, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (10.47 KB, patch)
2020-11-04 07:54 PST, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (10.62 KB, patch)
2020-11-04 08:05 PST, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (18.15 KB, patch)
2020-11-04 11:05 PST, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (105.55 KB, patch)
2020-11-04 11:10 PST, Chris Dumez 		sam: review+
ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (18.13 KB, patch)
2020-11-04 16:11 PST, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.
Chris Dumez
Comment 1 2020-11-03 16:41:03 PST
Created attachment 413122 [details] WIP Patch
Chris Dumez
Comment 2 2020-11-04 07:54:17 PST
Created attachment 413163 [details] Patch
Chris Dumez
Comment 3 2020-11-04 08:05:09 PST
Created attachment 413165 [details] Patch
Geoffrey Garen
Comment 4 2020-11-04 10:31:15 PST
Comment on attachment 413165 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=413165&action=review > Source/WebCore/ChangeLog:9 > + 'current event', stating that global the 'listener callbackâs associated Realmâs global that global the => that global is the > Source/WebCore/bindings/js/JSDOMWindowBase.h:114 > + Event* m_currentEvent { nullptr }; Can we use RefPtr here? > Source/WebCore/bindings/js/JSErrorHandler.cpp:83 > + Event* savedEvent = nullptr; Can we use RefPtr here? > Source/WebCore/bindings/js/JSErrorHandler.cpp:86 > + jsFunctionWindow->currentEvent(); I think you meant to assign to savedEvent here? Seems like a bug. Might need a new test. > Source/WebCore/bindings/js/JSEventListener.cpp:166 > + Event* savedEvent = nullptr; Can we use RefPtr here?
Chris Dumez
Comment 5 2020-11-04 10:54:00 PST
Comment on attachment 413165 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=413165&action=review >> Source/WebCore/bindings/js/JSDOMWindowBase.h:114 >> + Event* m_currentEvent { nullptr }; > > Can we use RefPtr here? I am merely moving the code so this is not new. I think we can make it a RefPtr though, even if not strictly needed for safety.
Chris Dumez
Comment 6 2020-11-04 11:05:22 PST
Created attachment 413182 [details] Patch
Chris Dumez
Comment 7 2020-11-04 11:10:54 PST
Created attachment 413184 [details] Patch
Chris Dumez
Comment 8 2020-11-04 16:11:35 PST
Created attachment 413221 [details] Patch
EWS
Comment 9 2020-11-04 18:23:36 PST
Committed r269414: <https://trac.webkit.org/changeset/269414> All reviewed patches have been landed. Closing bug and clearing flags on attachment 413221 [details].
Radar WebKit Bug Importer
Comment 10 2020-11-04 18:24:21 PST
<rdar://problem/71060677>
Note You need to log in before you can comment on or make changes to this bug.