Bug 203882

Summary: WindowEventLoop should be shared among similar origin documents
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: DOMAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, dbates, esprehn+autocc, ews-watchlist, kangil.han, koivisto, simon.fraser, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=204510
Bug Depends on:    
Bug Blocks: 202843    
Attachments:
Description Flags
WIP
none
Patch wenson_hsieh: review+

Description Ryosuke Niwa 2019-11-06 01:02:53 PST 
Share WindowEventLoop amongst similar-origin window events as HTML5 spec says:
https://html.spec.whatwg.org/multipage/webappapis.html#window-event-loop
Comment 1 Ryosuke Niwa 2019-11-06 01:07:16 PST 
Created attachment 382902 [details]
WIP
Comment 2 Radar WebKit Bug Importer 2019-11-06 16:46:38 PST 
<rdar://problem/56965038>
Comment 3 Ryosuke Niwa 2019-11-06 16:50:25 PST 
Created attachment 382985 [details]
Patch
Comment 4 Ryosuke Niwa 2019-11-07 12:07:03 PST 
Ping reviewers
Comment 5 Wenson Hsieh 2019-11-07 12:17:41 PST 
Comment on attachment 382985 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=382985&action=review

> Source/WebCore/dom/WindowEventLoop.cpp:58
> +    ASSERT(windowEventLoopMap().contains(m_domain));

Do we need both this debug assert and the release assert below?

> LayoutTests/http/tests/eventloop/queue-task-across-cross-site-frames.html:22
> +    setTimeout(() => finishJSTest(), 3000);

I don’t think you meant to include this.
Comment 6 Ryosuke Niwa 2019-11-07 16:30:23 PST 
(In reply to Wenson Hsieh from comment #5)
> Comment on attachment 382985 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=382985&action=review
> 
> > Source/WebCore/dom/WindowEventLoop.cpp:58
> > +    ASSERT(windowEventLoopMap().contains(m_domain));
> 
> Do we need both this debug assert and the release assert below?
> 
> > LayoutTests/http/tests/eventloop/queue-task-across-cross-site-frames.html:22
> > +    setTimeout(() => finishJSTest(), 3000);
> 
> I don’t think you meant to include this.

Oops these are added for debugging. Removed both.
Comment 7 Ryosuke Niwa 2019-11-07 17:13:21 PST 
Committed r252221: <https://trac.webkit.org/changeset/252221>