Bug 229838

Summary: WebKitTestRunner does not correctly close all auxiliary WebViews between tests
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: Tools / TestsAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, ap, darin, Hironori.Fujii, jbedard, sam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=229932
Bug Depends on:    
Bug Blocks: 229766, 229873    
Attachments:
Description Flags
WIP patch
none
WIP patch
ews-feeder: commit-queue-
WIP patch
ews-feeder: commit-queue-
WIP patch
none
WIP patch
none
Patch
none
Patch ews-feeder: commit-queue-

Description Chris Dumez 2021-09-02 16:05:51 PDT 
WebKitTestRunner does not close auxiliary WebViews between tests, letting them accumulate and wasting memory.
Comment 1 Chris Dumez 2021-09-02 16:09:33 PDT 
Created attachment 437210 [details]
WIP patch
Comment 2 Chris Dumez 2021-09-02 16:18:39 PDT 
Created attachment 437211 [details]
WIP patch
Comment 3 Chris Dumez 2021-09-02 16:25:42 PDT 
Created attachment 437214 [details]
WIP patch
Comment 4 Chris Dumez 2021-09-02 16:32:16 PDT 
Created attachment 437215 [details]
WIP patch
Comment 5 Chris Dumez 2021-09-02 16:36:11 PDT 
Created attachment 437217 [details]
WIP patch
Comment 6 Chris Dumez 2021-09-02 18:30:01 PDT 
Created attachment 437228 [details]
Patch
Comment 7 Alex Christensen 2021-09-02 20:43:01 PDT 
Comment on attachment 437228 [details]
Patch

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

> Tools/DumpRenderTree/mac/DumpRenderTree.mm:1992
> +        NSArray* array = [DumpRenderTreeWindow openWindows];
> +        unsigned count = [array count];
> +        for (unsigned i = 0; i < count; i++) {
> +            NSWindow *window = [array objectAtIndex:i];

for (NSWindow *window in DumpRenderTreeWindow.openWindows)
A lot of code below could also use dot syntax.

> Tools/DumpRenderTree/win/DumpRenderTree.cpp:1333
>          Vector<HWND> windows = openWindows();
>          unsigned size = windows.size();
>          for (unsigned i = 0; i < size; i++) {

for (HWND window : openWindows())
Also, there's now an unnecessary scope.
Comment 8 Chris Dumez 2021-09-02 21:03:40 PDT 
Comment on attachment 437228 [details]
Patch

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

>> Tools/DumpRenderTree/win/DumpRenderTree.cpp:1333
>>          for (unsigned i = 0; i < size; i++) {
> 
> for (HWND window : openWindows())
> Also, there's now an unnecessary scope.

The reason I kept the scope is that there is a goto above :(
Comment 9 Chris Dumez 2021-09-03 07:17:50 PDT 
Created attachment 437268 [details]
Patch
Comment 10 Chris Dumez 2021-09-03 07:19:53 PDT 
(In reply to Alex Christensen from comment #7)
> Comment on attachment 437228 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=437228&action=review
> 
> > Tools/DumpRenderTree/mac/DumpRenderTree.mm:1992
> > +        NSArray* array = [DumpRenderTreeWindow openWindows];
> > +        unsigned count = [array count];
> > +        for (unsigned i = 0; i < count; i++) {
> > +            NSWindow *window = [array objectAtIndex:i];
> 
> for (NSWindow *window in DumpRenderTreeWindow.openWindows)
> A lot of code below could also use dot syntax.
> 
> > Tools/DumpRenderTree/win/DumpRenderTree.cpp:1333
> >          Vector<HWND> windows = openWindows();
> >          unsigned size = windows.size();
> >          for (unsigned i = 0; i < size; i++) {
> 
> for (HWND window : openWindows())
> Also, there's now an unnecessary scope.

The scope is indeed now necessary with your proposed change.
Comment 11 EWS 2021-09-03 07:47:03 PDT 
Committed r281990 (241297@main): <https://commits.webkit.org/241297@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 437268 [details].
Comment 12 Radar WebKit Bug Importer 2021-09-03 07:48:15 PDT 
<rdar://problem/82722937>
Comment 13 Chris Dumez 2021-09-03 08:25:41 PDT 
Follow-up iOS build fix in  <https://commits.webkit.org/r281994>.
Comment 14 Fujii Hironori 2021-09-07 19:42:15 PDT 
Comment on attachment 437268 [details]
Patch

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

> Tools/DumpRenderTree/win/DumpRenderTree.cpp:-1331
> -        Vector<HWND> windows = openWindows();

This change caused a bug.  
Filed: Bug 229932 – [Win][DumpRenderTree] ASSERTION FAILED: openWindows().size() == 1 in runTest