Summary: | [Qt] Mock DeviceOrientation client for DRT | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Diego Gonzalez <diegohcg> | ||||||||||
Component: | WebKit Qt | Assignee: | Diego Gonzalez <diegohcg> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | cshu, hausmann, kenneth, kling, laszlo.gombos, ossy, qi.2.zhang, tonikitoo, webkit-ews, webkit.review.bot | ||||||||||
Priority: | P2 | Keywords: | Qt | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 47050 | ||||||||||||
Attachments: |
|
Description
Diego Gonzalez
2010-10-11 07:53:34 PDT
Created attachment 70436 [details]
Patch
For it's works with DRT is necessary to build if --device-orientation and assure if Qt Mobility with sensors lib is installed in the bots. Ossy, could you check if this patch build in the bots? Comment on attachment 70436 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=70436&action=review > WebKit/qt/WebCoreSupport/DeviceMotionClientQt.cpp:-25 > - Touching an unrelated file just to remove a newline seems a little unnecessary. ;-) > WebKit/qt/WebCoreSupport/DumpRenderTreeSupportQt.cpp:662 > + delete DeviceOrientationClientMockQt::client(); This doesn't look right- after this, client() will still return the old (now deleted) pointer. Attachment 70436 [details] did not build on qt: Build output: http://queues.webkit.org/results/4290028 (In reply to comment #3) > (From update of attachment 70436 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=70436&action=review > > > WebKit/qt/WebCoreSupport/DeviceMotionClientQt.cpp:-25 > > - > > Touching an unrelated file just to remove a newline seems a little unnecessary. ;-) > > > WebKit/qt/WebCoreSupport/DumpRenderTreeSupportQt.cpp:662 > > + delete DeviceOrientationClientMockQt::client(); > > This doesn't look right- after this, client() will still return the old (now deleted) pointer. It's only used by DRT, for this I called to remove it at DRT destructor. Created attachment 70439 [details]
Patch v2
Make it build with on Qt
Created attachment 75492 [details]
Patch v3
Comment on attachment 75492 [details] Patch v3 View in context: https://bugs.webkit.org/attachment.cgi?id=75492&action=review > WebKit/qt/WebCoreSupport/DeviceOrientationClientMockQt.cpp:48 > + delete m_clientMock; cant this be an OwnPtr? If not this is ok > WebKit/qt/WebCoreSupport/DeviceOrientationClientMockQt.h:46 > + static bool drtActive; static bool mockIsActive would be more descriptive. (In reply to comment #8) > (From update of attachment 75492 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=75492&action=review > > > WebKit/qt/WebCoreSupport/DeviceOrientationClientMockQt.cpp:48 > > + delete m_clientMock; > > cant this be an OwnPtr? If not this is ok > > > WebKit/qt/WebCoreSupport/DeviceOrientationClientMockQt.h:46 > > + static bool drtActive; > > static bool mockIsActive would be more descriptive. Thanks Kenneth. To build the orientation stuff we should user --device-orientation flags. Ossy: is possible to get the bots building with this flag? Created attachment 75818 [details]
followup patch for Patch v3
I prefer making build system better to passing --device-orientation to build-webkit manually.
Shouldn't we enable it by default? (In reply to comment #11) > Shouldn't we enable it by default? It would break the build if QtMobility isn't installed somewhere. Attachment 75818 [details] did not pass style-queue:
Failed to run "[u'git', u'reset', u'--hard', u'refs/remotes/trunk']" exit_code: 128
error: Could not write new index file.
fatal: Could not reset index file to revision 'refs/remotes/trunk'.
If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #12) > (In reply to comment #11) > > Shouldn't we enable it by default? > > It would break the build if QtMobility isn't installed somewhere. Does the bots have QtMobility installed? (In reply to comment #14) > Does the bots have QtMobility installed? Of course. ;) Attachment 75818 [details] did not pass style-queue:
Failed to run "[u'git', u'reset', u'--hard', u'refs/remotes/trunk']" exit_code: 128
error: Could not write new index file.
fatal: Could not reset index file to revision 'refs/remotes/trunk'.
If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #15) > (In reply to comment #14) > > Does the bots have QtMobility installed? > Of course. ;) Cool, in this case I think your patch is a good way :) Attachment 75818 [details] did not pass style-queue:
Failed to run "[u'git', u'reset', u'--hard', u'refs/remotes/trunk']" exit_code: 128
error: Could not write new index file.
fatal: Could not reset index file to revision 'refs/remotes/trunk'.
If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 75818 [details] did not pass style-queue:
Failed to run "[u'git', u'reset', u'--hard', u'refs/remotes/trunk']" exit_code: 128
error: Could not write new index file.
fatal: Could not reset index file to revision 'refs/remotes/trunk'.
If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 75818 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/update-webkit']" exit_code: 2
Updating OpenSource
Incomplete data: Delta source ended unexpectedly at /usr/lib/git-core/git-svn line 5061
Died at WebKitTools/Scripts/update-webkit line 132.
If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #10) > Created an attachment (id=75818) [details] > followup patch for https://bugs.webkit.org/attachment.cgi?id=75492 > > I prefer making build system better to passing --device-orientation to build-webkit manually. Could anyone review this Ossy's patch? I cannot commit my patch without this patch? :) Comment on attachment 75818 [details] followup patch for Patch v3 It is obsolote because of https://bugs.webkit.org/show_bug.cgi?id=50781 I'm going to update it. Patch landed at r73862. Closing bug. |