Bug 49066

Summary:

Convert to and from DOMTimeStamp with converter functions

Product:

WebKit

Reporter:

John Knottenbelt <jknotten>

Component:

WebCore Misc.

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, andreip, commit-queue, dbates, eric, sam, steveblock

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

OS X 10.5

Bug Depends on:

Bug Blocks:

48518

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none

John Knottenbelt

Reported 2010-11-05 04:50:23 PDT

At present, DOMTimeStamp is defined in WebCore/dom/Event.h, with a FIXME to move somewhere else. It should be moved into its own header and provide conversion functions to convert to and from seconds as provided by WTF::currentTime(). We should change existing conversions (multiply / divide by 1000) to use the conversion functions.

Attachments
Patch (7.91 KB, patch) 2010-11-05 04:53 PDT, John Knottenbelt	no flags	Details Formatted Diff Diff
Patch (17.35 KB, patch) 2010-11-05 09:23 PDT, John Knottenbelt	no flags	Details Formatted Diff Diff
Patch (17.42 KB, patch) 2010-11-05 09:28 PDT, John Knottenbelt	no flags	Details Formatted Diff Diff
Patch (17.50 KB, patch) 2010-11-05 10:38 PDT, John Knottenbelt	no flags	Details Formatted Diff Diff
Patch (17.54 KB, patch) 2010-11-08 07:35 PST, John Knottenbelt	no flags	Details Formatted Diff Diff
Patch (13.24 KB, patch) 2010-11-08 08:34 PST, John Knottenbelt	no flags	Details Formatted Diff Diff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.

John Knottenbelt

Comment 1 2010-11-05 04:53:54 PDT

Created attachment 73055 [details] Patch

Steve Block

Comment 2 2010-11-05 06:56:21 PDT

Comment on attachment 73055 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=73055&action=review LGTM, but would be good to get an OK from somebody else too. > WebCore/ChangeLog:6 > + https://bugs.webkit.org/show_bug.cgi?id=49066 Add a line about LayoutTests - just say this is a refactoring only.

Andrei Popescu

Comment 3 2010-11-05 07:29:23 PDT

You forgot to update the build files for Visual Studio, XCode, etc.

John Knottenbelt

Comment 4 2010-11-05 09:23:08 PDT

Created attachment 73071 [details] Patch

John Knottenbelt

Comment 5 2010-11-05 09:28:20 PDT

Created attachment 73072 [details] Patch

John Knottenbelt

Comment 6 2010-11-05 09:29:01 PDT

Thanks, change log modified and build files attached.

Adam Barth

Comment 7 2010-11-05 10:26:53 PDT

Comment on attachment 73072 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=73072&action=review Thanks! Much easier to understand. :) > WebCore/platform/android/GeolocationServiceAndroid.cpp:175 > - DOMTimeStamp currentTimeMillis = WTF::currentTime() * 1000.0; > - DOMTimeStamp maximumAgeMillis = 10 * 60 * 1000; // 10 minutes > + DOMTimeStamp currentTimeMillis = convertSecondsToDOMTimeStamp(WTF::currentTime()); > + DOMTimeStamp maximumAgeMillis = convertSecondsToDOMTimeStamp(10 * 60); // 10 minutes Should we rename these variables? currentTimeStamp, etc?

John Knottenbelt

Comment 8 2010-11-05 10:38:55 PDT

Created attachment 73079 [details] Patch

John Knottenbelt

Comment 9 2010-11-05 10:41:12 PDT

Comment on attachment 73079 [details] Patch Renamed the variables in GeolocationServiceAndroid.cpp as per Adam's suggestion.

WebKit Commit Bot

Comment 10 2010-11-05 11:03:10 PDT

Comment on attachment 73079 [details] Patch Rejecting patch 73079 from commit-queue. Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=eseidel-cq-sl', 'build', '--no-clean', '--no-update', '--build-style=both']" exit_code: 2 Last 500 characters of output: 4/WebTextCompletionController.o /Projects/CommitQueue/WebKit/mac/WebView/WebTextCompletionController.mm normal x86_64 objective-c++ com.apple.compilers.gcc.4_2 CompileC /Projects/CommitQueue/WebKitBuild/WebKit.build/Debug/WebKit.build/Objects-normal/x86_64/WebView.o /Projects/CommitQueue/WebKit/mac/WebView/WebView.mm normal x86_64 objective-c++ com.apple.compilers.gcc.4_2 (36 failures) Use of uninitialized value $_[0] in join or string at /System/Library/Perl/5.10.0/File/Spec/Unix.pm line 81. Full output: http://queues.webkit.org/results/5247018

Eric Seidel (no email)

Comment 11 2010-11-05 11:07:30 PDT

The perl-error is unrelated (I think). But CCing Dan as this is not the first time I've seen this. It is something to do with File->catdir iirc.

John Knottenbelt

Comment 12 2010-11-08 07:28:35 PST

Looking at the build log, the it failed because DOMTimeStamp.h wasn't included in the Framework's private headers. Will include and try again.

John Knottenbelt

Comment 13 2010-11-08 07:35:54 PST

Created attachment 73238 [details] Patch

John Knottenbelt

Comment 14 2010-11-08 07:58:58 PST

Comment on attachment 73238 [details] Patch In the WebCore Framework, I have now set the header role of DOMTimestamp.h to private. This allowed WebKit to build.

John Knottenbelt

Comment 15 2010-11-08 08:34:04 PST

Created attachment 73242 [details] Patch

WebKit Commit Bot

Comment 16 2010-11-08 20:15:09 PST

Comment on attachment 73242 [details] Patch Clearing flags on attachment: 73242 Committed r71602: <http://trac.webkit.org/changeset/71602>

WebKit Commit Bot

Comment 17 2010-11-08 20:15:16 PST

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.