[Webkit-unassigned] [Bug 48506] Move DeviceOrientationClientMock from LayoutTestController to WebViewHost

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=48506





--- Comment #2 from Hans Wennborg <hans at chromium.org>  2010-10-28 06:24:57 PST ---
(From update of attachment 72178)
View in context: https://bugs.webkit.org/attachment.cgi?id=72178&action=review

This looks fine (assuming the tests still pass of course :), I only have a couple of nits (also I'm not a real reviewer):

> WebCore/ChangeLog:5
> +        Add an assert to check that the DeviceOrientationClient is 

DeviceOrientationClient -> DeviceOrientationClientMock?

> WebKitTools/ChangeLog:6
> +        WebViewHost.

Maybe add a comment that this is to make sure that each Page gets its own client?

> WebKitTools/DumpRenderTree/chromium/LayoutTestController.cpp:1562
> +WebKit::WebDeviceOrientationClientMock* LayoutTestController::deviceOrientationClientMock()

There's a using directive for the WebKit namespace, so WebKit:: shouldn't be necessary.

> WebKitTools/DumpRenderTree/chromium/WebViewHost.cpp:1110
> +    PassOwnPtr<WebDeviceOrientationClientMock> deviceOrientationClientMock = m_deviceOrientationClientMock.release();

Should this be a PassOwnPtr? I'm always confused by these smart pointers, but since release() returns a raw pointer, isn't that what we should use? Steve would know this better than me, I think..

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.