Bug 51100

Summary: [QT] QtWebkit geolocation's position.timestamp is not in miliseconds
Product: WebKit Reporter: Mahesh Kulkarni <maheshk>
Component: WebCore Misc.Assignee: Mahesh Kulkarni <maheshk>
Status: RESOLVED FIXED    
Severity: Normal CC: ademar, bulach, commit-queue, hausmann, jarred, laszlo.gombos, mitz, steveblock
Priority: P2 Keywords: Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
patch
kling: review-
patch none

Description Mahesh Kulkarni 2010-12-15 02:46:09 PST
According to HTML5 GeoLocation standards, (Geo)Position interface’s timestamp should be of DOMTimestamp type and DOMTimestamp represents a number of milliseconds. 

Ref: http://dev.w3.org/geo/api/spec-source.html#position_interface 
       http://www.w3.org/TR/DOM-Level-3-Core/core.html#Core-DOMTimeStamp 

However in QTWebKit code while creating Geoposition object, timestamp was in ‘seconds ‘ (using QDateTime::toTime_t() function  which returns the datetime as the number of seconds that have passed since 1970-01-01T00:00:00 UTC ) instead of ‘miliseconds’ (QDateTime:: toMSecsSinceEpoch() which returns the datetime as the number of milliseconds that have passed since 1970-01-01T00:00:00.000). So patched GeolocationServiceQt::positionUpdated() function to create Geoposition object using milliseconds.
Comment 1 Jarred Nicholls 2010-12-15 06:22:40 PST
diff --git a/WebCore/platform/qt/GeolocationServiceQt.cpp b/WebCore/platform/qt/GeolocationServiceQt.cpp
index 3562eb9..2cccee6 100644
--- a/WebCore/platform/qt/GeolocationServiceQt.cpp
+++ b/WebCore/platform/qt/GeolocationServiceQt.cpp
@@ -83,7 +83,7 @@ void GeolocationServiceQt::positionUpdated(const QGeoPositionInfo &geoPosition)
     RefPtr<Coordinates> coordinates = Coordinates::create(latitude, longitude, providesAltitude, altitude,
                                                           accuracy, providesAltitudeAccuracy, altitudeAccuracy,
                                                           providesHeading, heading, providesSpeed, speed);
-    m_lastPosition = Geoposition::create(coordinates.release(), geoPosition.timestamp().toTime_t());
+    m_lastPosition = Geoposition::create(coordinates.release(), geoPosition.timestamp().toMSecsSinceEpoch());
     positionChanged();
 }
 
LayoutTests/fast/dom/Geolocation/timestamp.html appears to pass.  Nice catch.
Comment 2 Mahesh Kulkarni 2010-12-20 00:21:21 PST
Created attachment 76979 [details]
patch

Proposing patch, now qtwebkit milisec timestamp from GeoPosition. Please review
Comment 3 Andreas Kling 2010-12-20 00:45:28 PST
Comment on attachment 76979 [details]
patch

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

> WebCore/platform/qt/GeolocationServiceQt.cpp:86
> -    m_lastPosition = Geoposition::create(coordinates.release(), geoPosition.timestamp().toTime_t());
> +    m_lastPosition = Geoposition::create(coordinates.release(), geoPosition.timestamp().toMSecsSinceEpoch());

toMSecsSinceEpoch() is new in Qt 4.7. Since we still support building QtWebKit against Qt 4.6, this needs a QT_VERSION check and something for 4.6.
Comment 4 Mahesh Kulkarni 2010-12-20 05:40:28 PST
Created attachment 76991 [details]
patch

Thanks Kling for review. 

Incorporated review comments as per comment 3.
Comment 5 Andreas Kling 2010-12-20 06:27:58 PST
Comment on attachment 76991 [details]
patch

LGTM.
Comment 6 WebKit Commit Bot 2010-12-20 07:34:03 PST
Comment on attachment 76991 [details]
patch

Rejecting attachment 76991 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-3', 'apply-attachment', '--non-interactive', 76991]" exit_code: 2
Last 500 characters of output:
/ChangeLog
Failed to merge in the changes.
Patch failed at 0001 2010-12-20  Yury Semikhatsky  <yurys@chromium.org>

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at Tools/Scripts/update-webkit line 132.

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 2

Full output: http://queues.webkit.org/results/7269066
Comment 7 WebKit Commit Bot 2010-12-20 08:23:51 PST
The commit-queue encountered the following flaky tests while processing attachment 76991 [details]:

inspector/elements-panel-rewrite-href.html bug 51337 (authors: apavlov@chromium.org, pfeldman@chromium.org, and yurys@chromium.org)
The commit-queue is continuing to process your patch.
Comment 8 Eric Seidel (no email) 2010-12-20 23:05:59 PST
Comment on attachment 76991 [details]
patch

That cq bug has finally been fixed.  Sorry for the trouble.
Comment 9 WebKit Commit Bot 2010-12-20 23:49:04 PST
The commit-queue encountered the following flaky tests while processing attachment 76991 [details]:

inspector/styles-disable-then-enable.html bug 51384 (author: pfeldman@chromium.org)
The commit-queue is continuing to process your patch.
Comment 10 WebKit Commit Bot 2010-12-20 23:50:28 PST
Comment on attachment 76991 [details]
patch

Clearing flags on attachment: 76991

Committed r74404: <http://trac.webkit.org/changeset/74404>
Comment 11 WebKit Commit Bot 2010-12-20 23:50:34 PST
All reviewed patches have been landed.  Closing bug.
Comment 12 Ademar Reis 2011-01-03 05:32:15 PST
Revision r74404 cherry-picked into qtwebkit-2.2 with commit be21b13 <http://gitorious.org/webkit/qtwebkit/commit/be21b13>