Bug 81419 – WebKitTestRunner needs to protect the user's pasteboard contents while running

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

[Brady Eidson]

WebKitTestRunner needs to protect the user's pasteboard contents while running

DumpRenderTree has code to do this.  It would be extremely polite if WKTR did it also.

[Brady Eidson]

<rdar://problem/11066794>

[Alexey Proskuryakov]

Created attachment 209674 [details]
proposed fix

This makes it nearly impossible to run the whole WK2 tests suite locally, and almost certainly introduces a lot of flakiness on bots (as tests interfere with each other). Crazy.

[Darin Adler]

Comment on attachment 209674 [details]
proposed fix

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

> Tools/WebKitTestRunner/mac/PoseAsClass.mm:34
> +    unsigned int imposterMethodCount;

s/unsigned int/unsigned/

> Tools/WebKitTestRunner/mac/WebKitTestRunnerPasteboard.h:4
> + * Copyright (C) 2005, 2006, 2007, 2013 Apple, Inc.  All rights reserved.
> + * Copyright (C) 2007 Graham Dennis ([e-mail])
> + * Copyright (C) 2007 Eric Seidel <[e-mail]>

Just Apple, these other people didn’t write this header.

> Tools/WebKitTestRunner/mac/WebKitTestRunnerPasteboard.mm:4
> + * Copyright (C) 2005, 2006, 2007, 2013 Apple, Inc.  All rights reserved.
> + * Copyright (C) 2007 Graham Dennis ([e-mail])
> + * Copyright (C) 2007 Eric Seidel <[e-mail]>

What did these people write of this file?

> Tools/WebKitTestRunner/mac/WebKitTestRunnerPasteboard.mm:84
> ++ (id)alloc
> +{
> +    return NSAllocateObject(self, 0, 0);
> +}

Why is this needed?

> Tools/WebKitTestRunner/mac/WebKitTestRunnerPasteboard.mm:92
> +    typesArray = [[NSMutableArray alloc] init];
> +    typesSet = [[NSMutableSet alloc] init];
> +    dataByType = [[NSMutableDictionary alloc] init];
> +    pasteboardName = [name copy];
> +    return self;

Why no [super init] here?

[Alexey Proskuryakov]

> > Tools/WebKitTestRunner/mac/WebKitTestRunnerPasteboard.mm:4
> > + * Copyright (C) 2005, 2006, 2007, 2013 Apple, Inc.  All rights reserved.
> > + * Copyright (C) 2007 Graham Dennis ([e-mail])
> > + * Copyright (C) 2007 Eric Seidel <[e-mail]>
> 
> What did these people write of this file?

Graham Dennis wrote code for exposing declareTypes to JavaScript through WebScriptObject. I will remove the code, and his copyright.

Eric extracted DumpRenderTreePasteboard code to a separate file. Comparing the code, it doesn't seem like any substantial changes were made in the process, I'll remove his copyright, too.

> > Tools/WebKitTestRunner/mac/WebKitTestRunnerPasteboard.mm:84
> > ++ (id)alloc
> > +{
> > +    return NSAllocateObject(self, 0, 0);
> > +}
> 
> Why is this needed?

I do not know. In r17480, you wrote "Added, so we don't have to call NSAllocateObject explicitly elsewhere."

> Why no [super init] here?

Hmm. I guess I'll post a separate patch to add it to both DRT and WTR at once.

[Alexey Proskuryakov]

Committed <http://trac.webkit.org/r154640>.

[Alexey Proskuryakov]

> > ++ (id)alloc
> > +{
> > +    return NSAllocateObject(self, 0, 0);
> > +}
> 
> Why is this needed?

Turns out that NSPasteboard overrides +alloc and calls doesNotRecognizeSelector: there, essentially raising an exception.

I'm not sure if calling -init on such a class makes sense.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 120329

[Alexey Proskuryakov]

Re-landed with small fixes in <http://trac.webkit.org/r154652>:

- Added [super init] to match DRT code, which was just recently fixed (this doesn't observably help anything though).
- Added an override for _updateTypeCacheIfNeeded, which crashes during dragging (this doesn't seem to affect WK1 because dragging if fake, apparently).