Bug 31416 - Fail early in parseDateFromNullTerminatedCharacters
Summary: Fail early in parseDateFromNullTerminatedCharacters
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-11-12 10:03 PST by Steve VanDeBogart
Modified: 2009-11-12 21:08 PST (History)
2 users (show)

See Also:


Attachments
Only calculate time offsets for valid dates. (1.27 KB, patch)
2009-11-12 10:05 PST, Steve VanDeBogart
levin: review-
Details | Formatted Diff | Diff
Other case and early exit (1.60 KB, patch)
2009-11-12 11:06 PST, Steve VanDeBogart
vandebo: review-
Details | Formatted Diff | Diff
Fix spacing (1.61 KB, patch)
2009-11-12 11:12 PST, Steve VanDeBogart
no flags Details | Formatted Diff | Diff

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