Bug 74522 - [Qt] DeviceOrientationClientQt should initialize m_controller to zero.
Summary: [Qt] DeviceOrientationClientQt should initialize m_controller to zero.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alexander Færøy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-14 11:00 PST by Alexander Færøy
Modified: 2011-12-14 14:10 PST (History)
4 users (show)

See Also:


Attachments
Patch (2.22 KB, patch)
2011-12-14 11:03 PST, Alexander Færøy
no flags Details | Formatted Diff | Diff
Patch (2.34 KB, patch)
2011-12-14 11:12 PST, Alexander Færøy
no flags Details | Formatted Diff | Diff
Patch (1.38 KB, patch)
2011-12-14 12:55 PST, Alexander Færøy
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.