Bug 81419 - WebKitTestRunner needs to protect the user's pasteboard contents while running
Summary: WebKitTestRunner needs to protect the user's pasteboard contents while running
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All Unspecified
: P2 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords:
Depends on: 120329
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-16 15:23 PDT by Brady Eidson
Modified: 2013-08-26 16:39 PDT (History)
4 users (show)

See Also:


Attachments
proposed fix (20.11 KB, patch)
2013-08-26 13:52 PDT, Alexey Proskuryakov
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 2012-03-16 15:23:05 PDT
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.
Comment 1 Brady Eidson 2012-03-16 15:24:20 PDT
<rdar://problem/11066794>
Comment 2 Alexey Proskuryakov 2013-08-26 13:52:24 PDT
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.
Comment 3 Darin Adler 2013-08-26 14:07:38 PDT
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 (graham.dennis@gmail.com)
> + * Copyright (C) 2007 Eric Seidel <eric@webkit.org>

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 (graham.dennis@gmail.com)
> + * Copyright (C) 2007 Eric Seidel <eric@webkit.org>

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?
Comment 4 Alexey Proskuryakov 2013-08-26 14:23:00 PDT
> > Tools/WebKitTestRunner/mac/WebKitTestRunnerPasteboard.mm:4
> > + * Copyright (C) 2005, 2006, 2007, 2013 Apple, Inc.  All rights reserved.
> > + * Copyright (C) 2007 Graham Dennis (graham.dennis@gmail.com)
> > + * Copyright (C) 2007 Eric Seidel <eric@webkit.org>
> 
> 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.
Comment 5 Alexey Proskuryakov 2013-08-26 14:34:02 PDT
Committed <http://trac.webkit.org/r154640>.
Comment 6 Alexey Proskuryakov 2013-08-26 14:45:54 PDT
> > ++ (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.
Comment 7 WebKit Commit Bot 2013-08-26 16:03:28 PDT
Re-opened since this is blocked by bug 120329
Comment 8 Alexey Proskuryakov 2013-08-26 16:39:23 PDT
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).