Summary: | [Chromium/Mac] fast/events/constructors/progress-event-constructor.html is failing | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Fumitoshi Ukai <ukai> | ||||||||
Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | abarth, dominicc, haraken, haraken, heycam, japhet, sam, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 71340 | ||||||||||
Attachments: |
|
Description
Fumitoshi Ukai
2011-09-12 19:30:38 PDT
> -PASS new ProgressEvent('eventType', { loaded: -1 }).loaded is 18446744073709551615
> +FAIL new ProgressEvent('eventType', { loaded: -1 }).loaded should be 18446744073709552000. Was 0.
I am not 100% sure but this failure seems to happen on clang build.
- Linux Chromium (V8 + gcc) => PASS
- Mac Chromium (V8 + gcc) => PASS
- Mac Safari (JSC + gcc) => PASS
- Mac Chromium Canary (V8 + clang) => FAIL
- Win Chromium Canary (V8 + VC) => PASS
I am investigating more in detail...
Created attachment 112459 [details]
Patch
Comment on attachment 112459 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=112459&action=review > Source/WebCore/bindings/v8/OptionsObject.cpp:238 > + // Calculate d % 2^{64}. > + // -2^{64} < fmodValue < 2^{64}. > + double fmodValue = fmod(trunc(d), std::numeric_limits<unsigned long long>::max() + 1.0); > + if (fmodValue >= 0) { > + // 0 <= fmodValue < 2^{64}. > + // 0 <= value < 2^{64}. This cast causes no loss. > + value = static_cast<unsigned long long>(fmodValue); > + } else { > + // -2^{64} < fmodValue < 0. > + // 0 < fmodValueInUnsignedLongLong < 2^{64}. This cast causes no loss. > + unsigned long long fmodValueInUnsignedLongLong = static_cast<unsigned long long>(-fmodValue); > + // -1 < (std::numeric_limits<unsigned long long>::max() - fmodValueInUnsignedLongLong) < 2^{64} - 1. > + // 0 < value < 2^{64}. > + value = std::numeric_limits<unsigned long long>::max() - fmodValueInUnsignedLongLong + 1; > + } This looks cool, but it should be factored into a more general location so that folks can re-use it. If it's V8-specific logic, it should probably go in V8Utilities. If it's more general than V8, it should go in WTF somewhere. Created attachment 112631 [details]
Patch
(In reply to comment #3) > (From update of attachment 112459 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=112459&action=review > > > Source/WebCore/bindings/v8/OptionsObject.cpp:238 > > + // Calculate d % 2^{64}. > > + // -2^{64} < fmodValue < 2^{64}. > > + double fmodValue = fmod(trunc(d), std::numeric_limits<unsigned long long>::max() + 1.0); > > + if (fmodValue >= 0) { > > + // 0 <= fmodValue < 2^{64}. > > + // 0 <= value < 2^{64}. This cast causes no loss. > > + value = static_cast<unsigned long long>(fmodValue); > > + } else { > > + // -2^{64} < fmodValue < 0. > > + // 0 < fmodValueInUnsignedLongLong < 2^{64}. This cast causes no loss. > > + unsigned long long fmodValueInUnsignedLongLong = static_cast<unsigned long long>(-fmodValue); > > + // -1 < (std::numeric_limits<unsigned long long>::max() - fmodValueInUnsignedLongLong) < 2^{64} - 1. > > + // 0 < value < 2^{64}. > > + value = std::numeric_limits<unsigned long long>::max() - fmodValueInUnsignedLongLong + 1; > > + } > > This looks cool, but it should be factored into a more general location so that folks can re-use it. If it's V8-specific logic, it should probably go in V8Utilities. If it's more general than V8, it should go in WTF somewhere. Done. Moved it to wtf/MathExtras.h. Comment on attachment 112631 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=112631&action=review > Source/JavaScriptCore/wtf/MathExtras.h:297 > +inline void doubleToInteger(double d, unsigned long long& value) Why not return value? That can remove all else clauses. Comment on attachment 112631 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=112631&action=review >> Source/JavaScriptCore/wtf/MathExtras.h:297 > > Why not return value? That can remove all else clauses. I am thinking of overloading doubleToInteger() in the future, like doubleToInteger(double d, long long& value). Or should we rename this to doubleToUnsignedLongLong() and just return the value? Comment on attachment 112631 [details]
Patch
OK, I got it.
Created attachment 113439 [details]
patch for commit
Comment on attachment 113439 [details] patch for commit Rejecting attachment 113439 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: n/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc First, rewinding head to replay your work on top of it... Fast-forwarded master to refs/remotes/origin/master. Updating chromium port dependencies using gclient... ________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' ________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' Updating webkit projects from gyp files... Full output: http://queues.webkit.org/results/10190549 Comment on attachment 113439 [details] patch for commit Clearing flags on attachment: 113439 Committed r99161: <http://trac.webkit.org/changeset/99161> |