Bug 91311 – [Chromium] Move GamepadController into TestRunner.a

• Component: New Bugs
• Status: RESOLVED FIXED

[Adam Barth]

[Chromium] Move GamepadController into TestRunner.a

[Adam Barth]

Created attachment 152407 [details]
Patch

Comment on attachment 152407 [details]
Patch

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

> Tools/DumpRenderTree/chromium/TestRunner/GamepadController.cpp:2
> + * Copyright (C) 2011 Google Inc. All rights reserved.

nit 2012 (and other files as well)

> Tools/DumpRenderTree/chromium/TestRunner/GamepadController.cpp:75
> +    webkit_support::SetGamepadData(internalData);

This won't work with content_shell, so ideally, we would have a delegate for setting the gamepad data.

> Tools/DumpRenderTree/chromium/TestRunner/GamepadController.cpp:180
> +           "GamepadController\n");

i'm not sure whether stdout/stderr is correctly captured from renderers across all platforms. We might want to have some kind of printf on the delegate as well, wdyt?

[Adam Barth]

(In reply to comment #2)
> (From update of attachment 152407 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=152407&action=review
> 
> > Tools/DumpRenderTree/chromium/TestRunner/GamepadController.cpp:2
> > + * Copyright (C) 2011 Google Inc. All rights reserved.
> 
> nit 2012 (and other files as well)

I'm not changing any of these files.  WebKit policy is not to update the copyright.

> > Tools/DumpRenderTree/chromium/TestRunner/GamepadController.cpp:75
> > +    webkit_support::SetGamepadData(internalData);
> 
> This won't work with content_shell, so ideally, we would have a delegate for setting the gamepad data.

That's a problem to solve once we have this code linked into ContentShell.

> > Tools/DumpRenderTree/chromium/TestRunner/GamepadController.cpp:180
> > +           "GamepadController\n");
> 
> i'm not sure whether stdout/stderr is correctly captured from renderers across all platforms. We might want to have some kind of printf on the delegate as well, wdyt?

I don't think we should change any of this yet.  These are problems to solve in the future when we're smarter.

Comment on attachment 152407 [details]
Patch

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

>>> Tools/DumpRenderTree/chromium/TestRunner/GamepadController.cpp:75
>>> +    webkit_support::SetGamepadData(internalData);
>> 
>> This won't work with content_shell, so ideally, we would have a delegate for setting the gamepad data.
> 
> That's a problem to solve once we have this code linked into ContentShell.

Ok. Although I'm still curious to see how you intend to make the delegate interface look and where you hook it up.

Btw, by "won't work" I meant segfault

[Adam Barth]

(In reply to comment #4)
> (From update of attachment 152407 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=152407&action=review
> 
> >>> Tools/DumpRenderTree/chromium/TestRunner/GamepadController.cpp:75
> >>> +    webkit_support::SetGamepadData(internalData);
> >> 
> >> This won't work with content_shell, so ideally, we would have a delegate for setting the gamepad data.
> > 
> > That's a problem to solve once we have this code linked into ContentShell.
> 
> Ok. Although I'm still curious to see how you intend to make the delegate interface look and where you hook it up.

I'm curious too!  We'll see when we get there.  :)

> Btw, by "won't work" I meant segfault

webkit_support does a bunch of work for LayoutTestController and friends.  We should probably look at all that stuff to see what all we need.

I can continue to work on my branch, but this patch is blocking further progress on trunk.

sure, no need to block this patch, it looks good

[Tony Chang]

Comment on attachment 152407 [details]
Patch

Please land via the cq or get an EWS run that applies.

[Adam Barth]

Created attachment 152632 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 152632 [details]
Patch for landing

Clearing flags on attachment: 152632

Committed r122783: <http://trac.webkit.org/changeset/122783>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.