Bug 74522

Summary: [Qt] DeviceOrientationClientQt should initialize m_controller to zero.
Product: WebKit Reporter: Alexander Færøy <ahf>
Component: WebKit QtAssignee: Alexander Færøy <ahf>
Status: RESOLVED FIXED    
Severity: Normal CC: kenneth, tonikitoo, vestbo, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Alexander Færøy 2011-12-14 11:00:52 PST
We currently forget to set m_controller to zero upon construction. We should also forward any calls to setController() to our provider.

Patch coming.
Comment 1 Alexander Færøy 2011-12-14 11:03:27 PST
Created attachment 119253 [details]
Patch
Comment 2 Tor Arne Vestbø 2011-12-14 11:06:26 PST
Comment on attachment 119253 [details]
Patch

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

> Source/WebKit/qt/WebCoreSupport/DeviceOrientationProviderQt.cpp:33
> +    m_controller = 0;

initializer
Comment 3 Alexander Færøy 2011-12-14 11:12:30 PST
Created attachment 119257 [details]
Patch
Comment 4 Alexander Færøy 2011-12-14 11:13:44 PST
Adding Tor Arne.
Comment 5 Antonio Gomes 2011-12-14 12:14:41 PST
Comment on attachment 119257 [details]
Patch

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

> Source/WebKit/qt/WebCoreSupport/DeviceOrientationClientQt.cpp:43
> +    ASSERT(controller);
>      m_controller = controller;
> +    m_provider->setController(controller);

you seem to be doing more than what to changelog'ed.

> Source/WebKit/qt/WebCoreSupport/DeviceOrientationProviderQt.cpp:32
> +    : m_lastOrientation(DeviceOrientation::create())
> +    , m_controller(0)
> +    , m_sensor()

ditto
Comment 6 Antonio Gomes 2011-12-14 12:15:49 PST
(In reply to comment #5)
> (From update of attachment 119257 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=119257&action=review
> 
> > Source/WebKit/qt/WebCoreSupport/DeviceOrientationClientQt.cpp:43
> > +    ASSERT(controller);
> >      m_controller = controller;
> > +    m_provider->setController(controller);
> 
> you seem to be doing more than what to changelog'ed.

s/what to/what you/g
Comment 7 Kenneth Rohde Christiansen 2011-12-14 12:39:08 PST
Comment on attachment 119257 [details]
Patch

Write proper changelogs Alex! :-)
Comment 8 Alexander Færøy 2011-12-14 12:46:14 PST
(In reply to comment #7)
> (From update of attachment 119257 [details])
> Write proper changelogs Alex! :-)

It is because you are not around to pet me!

Jokes aside, new patch coming up in one moment. It is being split up such that the setController() changes are being moved to a separate patch.
Comment 9 Kenneth Rohde Christiansen 2011-12-14 12:51:18 PST
Comment on attachment 119257 [details]
Patch

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

> Source/WebKit/qt/WebCoreSupport/DeviceOrientationProviderQt.cpp:31
> +    , m_controller(0)

why isn't it a smart controller? you seem to use it by two here in this patch? should it be reffed?
Comment 10 Kenneth Rohde Christiansen 2011-12-14 12:52:33 PST
(In reply to comment #9)
> (From update of attachment 119257 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=119257&action=review
> 
> > Source/WebKit/qt/WebCoreSupport/DeviceOrientationProviderQt.cpp:31
> > +    , m_controller(0)
> 
> why isn't it a smart controller? you seem to use it by two here in this patch? should it be reffed?

smart pointer :-)
Comment 11 Alexander Færøy 2011-12-14 12:55:34 PST
Created attachment 119278 [details]
Patch
Comment 12 WebKit Review Bot 2011-12-14 14:10:04 PST
Comment on attachment 119278 [details]
Patch

Clearing flags on attachment: 119278

Committed r102822: <http://trac.webkit.org/changeset/102822>
Comment 13 WebKit Review Bot 2011-12-14 14:10:08 PST
All reviewed patches have been landed.  Closing bug.