Bug 31416

Summary: Fail early in parseDateFromNullTerminatedCharacters
Product: WebKit Reporter: Steve VanDeBogart <vandebo>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ggaren
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
Only calculate time offsets for valid dates.
levin: review-
Other case and early exit
vandebo: review-
Fix spacing none

Description Steve VanDeBogart 2009-11-12 10:03:46 PST
When parseDateFromNullTerminatedCharacters() is unable to parse the passed string, it still calculates the timezone and DST offset to apply to the result.  This causes an unnecessary IPC in chromium.  The attached patch changes the code to only calculate the offsets if parsing was successful.
Comment 1 Steve VanDeBogart 2009-11-12 10:05:20 PST
Created attachment 43071 [details]
Only calculate time offsets for valid dates.
Comment 2 David Levin 2009-11-12 10:34:09 PST
Comment on attachment 43071 [details]
Only calculate time offsets for valid dates.

Thanks for fixing this.


> Index: JavaScriptCore/ChangeLog
> +2009-11-11  Steve VanDeBogart  <vandebo@chromium.org>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Calculate the time offset only if we were able to parse
> +        the date string. This saves an IPC in Chromium for
> +        invalid date strings.

Please add a bug link.


> Index: JavaScriptCore/wtf/DateMath.cpp
> @@ -825,7 +825,7 @@ double parseDateFromNullTerminatedCharac
>      int offset;
>      double ms = parseDateFromNullTerminatedCharacters(dateString, haveTZ, offset);
>      // fall back to local timezone

Just do 
if (isnan(ms))
    return NaN;

> -    if (!haveTZ) {
> +    if (!haveTZ && ms != NaN) {

Also there is another instance to fix in "double parseDateFromNullTerminatedCharacters(ExecState* exec, const char* dateString)"
Comment 3 Steve VanDeBogart 2009-11-12 11:06:17 PST
Created attachment 43079 [details]
Other case and early exit
Comment 4 Steve VanDeBogart 2009-11-12 11:11:19 PST
Comment on attachment 43079 [details]
Other case and early exit

spacing
Comment 5 Steve VanDeBogart 2009-11-12 11:12:06 PST
Created attachment 43081 [details]
Fix spacing
Comment 6 Adam Barth 2009-11-12 20:49:50 PST
Comment on attachment 43081 [details]
Fix spacing

Thanks!
Comment 7 WebKit Commit Bot 2009-11-12 21:08:07 PST
Comment on attachment 43081 [details]
Fix spacing

Clearing flags on attachment: 43081

Committed r50927: <http://trac.webkit.org/changeset/50927>
Comment 8 WebKit Commit Bot 2009-11-12 21:08:12 PST
All reviewed patches have been landed.  Closing bug.