Bug 167088 – [Win] Clipboard tests are flaky.

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

[Per Arne Vollan]

Tests involving the clipboard are flaky when running with multiple DRTs, since the clipboard is global. We can fix this by assigning each DRT a separate window station (each window station has its own clipboard).

[Per Arne Vollan]

Created attachment 298949 [details]
Patch

[Brent Fulgham]

Comment on attachment 298949 [details]
Patch

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

What a great idea! I didn't know this was a problem, or that Window Stations even existed. Fantastic! I just had a question about how error state is handled in this patch, and whether we should be calling "CreateDesktop" with bad inputs. If it is safe, I approve the patch. But if you are not sure, I'd prefer you bypass these steps.

> Tools/DumpRenderTree/win/DumpRenderTree.cpp:1485
> +            fprintf(stderr, "DumpRenderTree should be run as Administrator!\n");

This is a shame! Is there no permission that can be granted a test user account to allow creation of unique Window Stations?

> Tools/DumpRenderTree/win/DumpRenderTree.cpp:1488
> +    if (!::SetProcessWindowStation(windowsStation))

Is it okay to pass a nullptr windowStation here (assuming line 1481 is false)? We probably shouldn't terminate if this stuff fails, since we've been living without Window Stations for years. But I wonder if we should bypass the steps in lines 1481-1493 when stuff is failing, rather than passing bad values to these API calls.

[Per Arne Vollan]

(In reply to comment #2)
> Comment on attachment 298949 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=298949&action=review
> 
> What a great idea! I didn't know this was a problem, or that Window Stations
> even existed. Fantastic! I just had a question about how error state is
> handled in this patch, and whether we should be calling "CreateDesktop" with
> bad inputs. If it is safe, I approve the patch. But if you are not sure, I'd
> prefer you bypass these steps.
> 
> > Tools/DumpRenderTree/win/DumpRenderTree.cpp:1485
> > +            fprintf(stderr, "DumpRenderTree should be run as Administrator!\n");
> 
> This is a shame! Is there no permission that can be granted a test user
> account to allow creation of unique Window Stations?
> 
> > Tools/DumpRenderTree/win/DumpRenderTree.cpp:1488
> > +    if (!::SetProcessWindowStation(windowsStation))
> 
> Is it okay to pass a nullptr windowStation here (assuming line 1481 is
> false)? We probably shouldn't terminate if this stuff fails, since we've
> been living without Window Stations for years. But I wonder if we should
> bypass the steps in lines 1481-1493 when stuff is failing, rather than
> passing bad values to these API calls.

Thanks for reviewing! I will update the patch before landing :)

[Per Arne Vollan]

Committed r210850: <https://trac.webkit.org/changeset/210850>