Bug 229838 – WebKitTestRunner does not correctly close all auxiliary WebViews between tests

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Chris Dumez]

WebKitTestRunner does not close auxiliary WebViews between tests, letting them accumulate and wasting memory.

[Chris Dumez]

Created attachment 437210 [details]
WIP patch

[Chris Dumez]

Created attachment 437211 [details]
WIP patch

[Chris Dumez]

Created attachment 437214 [details]
WIP patch

[Chris Dumez]

Created attachment 437215 [details]
WIP patch

[Chris Dumez]

Created attachment 437217 [details]
WIP patch

[Chris Dumez]

Created attachment 437228 [details]
Patch

[Alex Christensen]

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.

[Chris Dumez]

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 :(

[Chris Dumez]

Created attachment 437268 [details]
Patch

[Chris Dumez]

(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.

[EWS]

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].

[Radar WebKit Bug Importer]

<rdar://problem/82722937>

[Chris Dumez]

Follow-up iOS build fix in  <https://commits.webkit.org/r281994>.

[Fujii Hironori]

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