WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
45460
Hook up LayoutTestController.setMockDeviceOrientation() in Chromium DumpRenderTree.
https://bugs.webkit.org/show_bug.cgi?id=45460
Summary
Hook up LayoutTestController.setMockDeviceOrientation() in Chromium DumpRende...
Hans Wennborg
Reported
2010-09-09 08:37:45 PDT
Hook up LayoutTestController.setMockDeviceOrientation() in Chromium DumpRenderTree.
Attachments
Patch
(7.32 KB, patch)
2010-09-09 08:50 PDT
,
Hans Wennborg
no flags
Details
Formatted Diff
Diff
Patch
(7.16 KB, patch)
2010-09-09 09:24 PDT
,
Hans Wennborg
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Hans Wennborg
Comment 1
2010-09-09 08:50:09 PDT
Created
attachment 67039
[details]
Patch
Hans Wennborg
Comment 2
2010-09-09 08:51:41 PDT
Satish: feel free to take a look as this follows your speech input code very closely.
Satish Sampath
Comment 3
2010-09-09 09:01:24 PDT
Looks good to me. It would be useful if this patch also contained a layout test using the newly added layoutTestController method to validate that it works as intended.
Jeremy Orlow
Comment 4
2010-09-09 09:04:46 PDT
Comment on
attachment 67039
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=67039&action=prettypatch
> WebKitTools/DumpRenderTree/chromium/LayoutTestController.cpp:1397 > + if (arguments.size() < 6 || !arguments[0].isBool() || !arguments[1].isNumber()
not sure this needs to be multiple lines
> WebKitTools/DumpRenderTree/chromium/LayoutTestController.cpp:1399 > + || !arguments[4].isBool() || !arguments[5].isNumber())
use {}'s when the if or the subsequent part is more than one line
> WebKitTools/DumpRenderTree/chromium/LayoutTestController.cpp:1402 > + WebKit::WebDeviceOrientation orientation(arguments[0].toBoolean(), arguments[1].toDouble(),
or this
> WebKitTools/DumpRenderTree/chromium/LayoutTestController.cpp:1406 > + ASSERT(m_deviceOrientationClientMock.get());
dont need .get() I'm pretty sure r=me
Hans Wennborg
Comment 5
2010-09-09 09:24:12 PDT
(In reply to
comment #3
)
> Looks good to me. > > It would be useful if this patch also contained a layout test using the newly added layoutTestController method to validate that it works as intended.
There are tests in LayoutTests/fast/dom/DeviceOrientation already, but they are currently skipped for Chromium. My plan is to enable them when both this patch and the corresponding patch to test_shell in the Chromium repository has landed. (In reply to
comment #4
)
> (From update of
attachment 67039
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=67039&action=prettypatch
> > > WebKitTools/DumpRenderTree/chromium/LayoutTestController.cpp:1397 > > + if (arguments.size() < 6 || !arguments[0].isBool() || !arguments[1].isNumber() > not sure this needs to be multiple lines
Making it one line.
> > WebKitTools/DumpRenderTree/chromium/LayoutTestController.cpp:1399 > > + || !arguments[4].isBool() || !arguments[5].isNumber()) > use {}'s when the if or the subsequent part is more than one line
Obsolete since it's now one line.
> > WebKitTools/DumpRenderTree/chromium/LayoutTestController.cpp:1402 > > + WebKit::WebDeviceOrientation orientation(arguments[0].toBoolean(), arguments[1].toDouble(), > or this
Making it one line.
> > WebKitTools/DumpRenderTree/chromium/LayoutTestController.cpp:1406 > > + ASSERT(m_deviceOrientationClientMock.get()); > dont need .get() I'm pretty sure
You're right. Fixed. Uploading new patch with the fixes mentioned above.
Hans Wennborg
Comment 6
2010-09-09 09:24:34 PDT
Created
attachment 67043
[details]
Patch
WebKit Commit Bot
Comment 7
2010-09-10 00:17:16 PDT
Comment on
attachment 67043
[details]
Patch Clearing flags on attachment: 67043 Committed
r67177
: <
http://trac.webkit.org/changeset/67177
>
WebKit Commit Bot
Comment 8
2010-09-10 00:17:21 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.
Top of Page
Format For Printing
XML
Clone This Bug