[Webkit-unassigned] [Bug 43258] Implement chromium WebDeviceOrientationClient wrapper and have WebViewImpl get it from WebViewClient.
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Aug 6 08:11:32 PDT 2010
https://bugs.webkit.org/show_bug.cgi?id=43258
Jeremy Orlow <jorlow at chromium.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #63717|review? |review-
Flag| |
--- Comment #18 from Jeremy Orlow <jorlow at chromium.org> 2010-08-06 08:11:32 PST ---
(From update of attachment 63717)
WebKit/chromium/src/WebViewImpl.cpp:290
+ m_deviceOrientationClientProxy.set(new DeviceOrientationClientProxy(client ? client->deviceOrientationClient() : 0));
See if you can clean up this logic a bit. It's kinda weird how we have this matrix of possibilities between whether the flag is on and whether there's a client.
WebKit/chromium/src/WebDeviceOrientationController.cpp:31
+ #include "PassRefPtr.h"
<wtf/PassRefPtr.h>
WebKit/chromium/src/WebDeviceOrientationController.cpp:36
+ void WebDeviceOrientationController::didChangeDeviceOrientation(const WebDeviceOrientation& o)
use a better variable name
WebKit/chromium/src/WebDeviceOrientationClientMock.cpp:68
+ delete m_clientMock;
and set it to 0...and first do an if to see if it's already 0
and maybe you should just make a class much like PrivatePtr...make that PrivateRefPtr and then add PrivateOwnPtr? Just a thought.
WebKit/chromium/src/WebDeviceOrientationClientMock.cpp:63
+ m_clientMock = new WebCore::DeviceOrientationClientMock();
Hmmm...i'm tempted to have this call reset() but I guess the constructor doesn't set it to 0 before calling it...so that's not possible. I guess leave it as is.
WebKit/chromium/src/WebDeviceOrientationClientMock.cpp:53
+ return lastOrientation ? new WebDeviceOrientation(lastOrientation) : 0;
Hmmm....this should probably just pass back the object, not a pointer to it. But the problem is that there might not be a last orientation. Hmmmm....maybe you shoudl add an invalid/null state to the WebDO class? This is what we've done for serializedScriptValue and a couple others where a null refPtr has semantic meaning. Not sure about this, but I think it's better than passing around pointers when we don't absolutely have to.
WebKit/chromium/src/WebDeviceOrientation.cpp:49
+ m_canProvideGamma = o->canProvideGamma();
what about the actual gamma value?
WebKit/chromium/src/WebDeviceOrientation.cpp:39
+ m_canProvideGamma(o->canProvideGamma())
ditto
WebKit/chromium/src/DeviceOrientationClientProxy.cpp:32
+ #include "RefPtr.h"
ditto ditto ditto
WebKit/chromium/src/DeviceOrientationClientProxy.cpp:44
+ if (!m_client)
add fixme about getting rid of these null checks once it's enabled by default
WebKit/chromium/src/DeviceOrientationClientProxy.cpp:68
+ OwnPtr<WebDeviceOrientation> lastWebOrientation(m_client->lastOrientation());
Hmmm...this probably needs to be a refPtr in WebCore.
WebKit/chromium/src/DeviceOrientationClientProxy.cpp:72
+ m_lastOrientation = *lastWebOrientation;
Why are we caching it here? I guess because it was an own ptr? Well, we should def make it ref.
WebKit/chromium/public/WebDeviceOrientation.h:39
+ : m_canProvideAlpha(canProvideAlpha),
too many spaces in
WebKit/chromium/public/WebDeviceOrientation.h:46
+ }
You probably should add methods to this for getting the values.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list