RESOLVED FIXED Bug 91311
[Chromium] Move GamepadController into TestRunner.a
https://bugs.webkit.org/show_bug.cgi?id=91311
Summary [Chromium] Move GamepadController into TestRunner.a
Adam Barth
Reported 2012-07-13 22:13:50 PDT
[Chromium] Move GamepadController into TestRunner.a
Attachments
Patch (106.47 KB, patch)
2012-07-13 22:16 PDT, Adam Barth
no flags
Patch for landing (105.77 KB, patch)
2012-07-16 15:57 PDT, Adam Barth
no flags
Adam Barth
Comment 1 2012-07-13 22:16:19 PDT
jochen
Comment 2 2012-07-14 13:39:08 PDT
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
Comment 3 2012-07-14 15:45:23 PDT
(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.
jochen
Comment 4 2012-07-15 04:15:30 PDT
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
Comment 5 2012-07-15 09:54:28 PDT
(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.
jochen
Comment 6 2012-07-15 10:39:14 PDT
sure, no need to block this patch, it looks good
Tony Chang
Comment 7 2012-07-16 10:04:59 PDT
Comment on attachment 152407 [details] Patch Please land via the cq or get an EWS run that applies.
Adam Barth
Comment 8 2012-07-16 15:57:52 PDT
Created attachment 152632 [details] Patch for landing
WebKit Review Bot
Comment 9 2012-07-16 17:43:37 PDT
Comment on attachment 152632 [details] Patch for landing Clearing flags on attachment: 152632 Committed r122783: <http://trac.webkit.org/changeset/122783>
WebKit Review Bot
Comment 10 2012-07-16 17:43:42 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.