Bug 89661 – Web Inspector: Properly display native memory sizes bigger than 2GB

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

[Alexei Filippov]

Created attachment 148795 [details]
screenshot

Currently when a component size exceeds 2GB it is shown as a negative value.

[Alexei Filippov]

Created attachment 148796 [details]
Patch

[Yury Semikhatsky]

Comment on attachment 148796 [details]
Patch

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

> Source/WebCore/inspector/InspectorMemoryAgent.cpp:340
> +    jsHeapAllocated->setSize(static_cast<double>(totalJSHeapSize));

int to double conversion shouldn't require static_cast, pleas remove it.

[Alexei Filippov]

Comment on attachment 148796 [details]
Patch

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

>> Source/WebCore/inspector/InspectorMemoryAgent.cpp:340
>> +    jsHeapAllocated->setSize(static_cast<double>(totalJSHeapSize));
> 
> int to double conversion shouldn't require static_cast, pleas remove it.

It is size_t which is 64-bits long on x64, thus it may lead to precision loss when converted to double, thus it should require a cast.

[Alexander Pavlov (apavlov)]

Comment on attachment 148796 [details]
Patch

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

>>> Source/WebCore/inspector/InspectorMemoryAgent.cpp:340
>>> +    jsHeapAllocated->setSize(static_cast<double>(totalJSHeapSize));
>> 
>> int to double conversion shouldn't require static_cast, pleas remove it.
> 
> It is size_t which is 64-bits long on x64, thus it may lead to precision loss when converted to double, thus it should require a cast.

So, what will static_cast do in this case?

[Yury Semikhatsky]

(In reply to comment #3)
> (From update of attachment 148796 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=148796&action=review
> > int to double conversion shouldn't require static_cast, pleas remove it.
> 
> It is size_t which is 64-bits long on x64, thus it may lead to precision loss when converted to double, thus it should require a cast.

But C++ compiler won't complain in that case.

[Alexei Filippov]

(In reply to comment #4)
> (From update of attachment 148796 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=148796&action=review
> 
> >>> Source/WebCore/inspector/InspectorMemoryAgent.cpp:340
> >>> +    jsHeapAllocated->setSize(static_cast<double>(totalJSHeapSize));
> >> 
> >> int to double conversion shouldn't require static_cast, pleas remove it.
> > 
> > It is size_t which is 64-bits long on x64, thus it may lead to precision loss when converted to double, thus it should require a cast.
> 
> So, what will static_cast do in this case?

Convert uint64 to double losing precision.
If I remove the cast it will also do the conversion on some platforms while reporting warnings and errors on other platforms & flag settings.

[Alexei Filippov]

(In reply to comment #5)
> (In reply to comment #3)
> > (From update of attachment 148796 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=148796&action=review
> > > int to double conversion shouldn't require static_cast, pleas remove it.
> > 
> > It is size_t which is 64-bits long on x64, thus it may lead to precision loss when converted to double, thus it should require a cast.
> 
> But C++ compiler won't complain in that case.

Ok, if you insist I'll remove the cast.

[Alexei Filippov]

Created attachment 148806 [details]
Patch

[WebKit Review Bot]

Comment on attachment 148806 [details]
Patch

Clearing flags on attachment: 148806

Committed r121002: <http://trac.webkit.org/changeset/121002>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.