Bug 47105 - [Qt] Hook up accelerometer data via Qt DeviceMotion
Summary: [Qt] Hook up accelerometer data via Qt DeviceMotion
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Diego Gonzalez
URL:
Keywords: Qt
Depends on:
Blocks: 47050
  Show dependency tree / graph
 
Reported: 2010-10-04 12:43 PDT by Diego Gonzalez
Modified: 2010-10-07 09:27 PDT (History)
4 users (show)

See Also:

Attachments
Patch (12.50 KB, patch)
2010-10-05 10:46 PDT, Diego Gonzalez 		no flags Details | Formatted Diff | Diff
Patch v2 (12.41 KB, patch)
2010-10-06 11:57 PDT, Diego Gonzalez 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Diego Gonzalez 2010-10-04 12:43:48 PDT 
Get accelerometer necessary data via Qt mobility library using a provider class.
Comment 1 Diego Gonzalez 2010-10-05 10:46:31 PDT 
Created attachment 69808 [details]
Patch
Comment 2 Kenneth Rohde Christiansen 2010-10-05 11:36:49 PDT 
Comment on attachment 69808 [details]
Patch

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

Are we not supposed to pass more tests?

> WebKit/qt/WebCoreSupport/DeviceMotionProviderQt.cpp:30
> +    m_motion = DeviceMotionData::create();

Who is freeing this one?
Comment 3 Simon Hausmann 2010-10-06 01:24:05 PDT 
(In reply to comment #2)
> > WebKit/qt/WebCoreSupport/DeviceMotionProviderQt.cpp:30
> > +    m_motion = DeviceMotionData::create();
> 
> Who is freeing this one?

It looks like RefPtr is going to :)

RefPtr<DeviceMotionData> m_motion;
Comment 4 Diego Gonzalez 2010-10-06 05:41:04 PDT 
(In reply to comment #2)
> (From update of attachment 69808 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=69808&action=review
> 
> Are we not supposed to pass more tests?

Needs some implementation in DRT, I will working on it in other patch

> > WebKit/qt/WebCoreSupport/DeviceMotionProviderQt.cpp:30
> > +    m_motion = DeviceMotionData::create();
> 
> Who is freeing this one?
Comment 5 Diego Gonzalez 2010-10-06 05:45:18 PDT 
I did some test with OwnPtr for m_motion, it seems leaking and crashing, because the client needs to have access to the last motion state. So if think RefPtr is the best choice, as it frees automatically.
Comment 6 Diego Gonzalez 2010-10-06 11:57:41 PDT 
Created attachment 69974 [details]
Patch v2
Comment 7 Andreas Kling 2010-10-06 22:35:06 PDT 
Comment on attachment 69974 [details]
Patch v2

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

r=me, but please fix the below:

> WebKit/qt/WebCoreSupport/DeviceMotionProviderQt.h:44
> +    DeviceMotionData* currentDeviceMotion() { return m_motion.get(); }

This method should be const.
Comment 8 Diego Gonzalez 2010-10-07 08:53:22 PDT 
(In reply to comment #7)
> (From update of attachment 69974 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=69974&action=review
> 
> r=me, but please fix the below:
> 
> > WebKit/qt/WebCoreSupport/DeviceMotionProviderQt.h:44
> > +    DeviceMotionData* currentDeviceMotion() { return m_motion.get(); }
> 
> This method should be const.

Thanks Kling :)
Comment 9 Diego Gonzalez 2010-10-07 09:26:21 PDT 
Comment on attachment 69974 [details]
Patch v2

Clearing flags. Patch landed at r69313