Bug 53592

Any security aspects here? Seems like I remember there was something similar that was pulled from console last year, due to security concerns. I'd like to see this implemented though.

After discussions, we made memory info available only by explicit user request (preference in Safari, cmdline flag in Chrome). This resolves security concerns, I believe.

Created attachment 80919 [details] patch @JSC developers: The MAX_NUM_BLOCKS const (ex. maxNumBlocks) appears to be ridiculously big -- being multiplied by BLOCK_SIZE it exceeds ULONG_MAX. Does it need to be adjusted somehow? On a 64-bit platform MAX_NUM_BLOCKS value is 2^59 - 1 (as sizeof(PageAllocationAligned) = 16), and BLOCK_SIZE is 2^18.

Attachment 80919 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1 Source/JavaScriptCore/runtime/MarkedSpace.cpp:43: MAX_NUM_BLOCKS is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Total errors found: 1 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 80921 [details] fixed style Renamed the constant back, as the new name introduces style violation.

Attachment 80919 [details] did not build on win: Build output: http://queues.webkit.org/results/7683985

...and VC++ confirms that maxNumBlocks is uselessly big. Geoffrey, can you comment on this, please?

Attachment 80921 [details] did not build on win: Build output: http://queues.webkit.org/results/7689214

What does "limit" mean in this context? JavaScriptCore doesn't place a hard limit on the size of the heap -- it will use whatever the OS will give it.

But there can be other restrictions that prevent VM from allocating new space above a certain limit, e.g. control structures size limits. Why this maxNumBlocks constant exist at all?

> Why this maxNumBlocks constant exist at all? It seems to exist solely to check for overflow in multiplication. Whatever you're trying to do, this probably isn't the best way to do it. What does "limit" mean in this context? What are you trying to do?

I introduced this change because in V8 there is actually a hard limit on the VM heap size, and its value differs on 32-bit and 64-bit versions, so I didn't want to hardcode it. But knowing the limit appears to be useful, because when web app memory consumption grows up to the limit, renderer crashes. If JSC really doesn't have a limit, I can put something like undefined in this field. I wanted to make sure that both engines are aligned.

undefined sounds good to me.

Created attachment 81043 [details] return 'undefined' limit value for JSC

Attachment 81043 [details] did not build on win: Build output: http://queues.webkit.org/results/7692258

Attachment 81043 [details] did not build on qt: Build output: http://queues.webkit.org/results/7687838

Created attachment 81049 [details] fixed custom getter signature

Manually committed http://trac.webkit.org/changeset/77495 2011-02-03 Mikhail Naganov <mnaganov@chromium.org> Reviewed by Pavel Feldman. Web Inspector: Add reporting of JS heap size limit to 'console.memory'. https://bugs.webkit.org/show_bug.cgi?id=53592 In JSC there is no limit, thus 'undefined' value is returned. For V8, the limit reported by the VM is returned. * Android.jscbindings.mk: * CMakeLists.txt: * GNUmakefile.am: * WebCore.gypi: * WebCore.pro: * WebCore.vcproj/WebCore.vcproj: * WebCore.xcodeproj/project.pbxproj: * bindings/js/JSBindingsAllInOne.cpp: * bindings/js/JSMemoryInfoCustom.cpp: Added. * bindings/js/ScriptGCEvent.cpp: (WebCore::ScriptGCEvent::getHeapSize): * bindings/js/ScriptGCEvent.h: * bindings/v8/ScriptGCEvent.cpp: (WebCore::ScriptGCEvent::getHeapSize): * bindings/v8/ScriptGCEvent.h: * inspector/InspectorTimelineAgent.cpp: (WebCore::InspectorTimelineAgent::setHeapSizeStatistic): * page/MemoryInfo.cpp: (WebCore::MemoryInfo::MemoryInfo): * page/MemoryInfo.h: (WebCore::MemoryInfo::jsHeapSizeLimit): * page/MemoryInfo.idl:

> Manually committed http://trac.webkit.org/changeset/77495 This broke a test on Snow Leopard: http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(Tests)/r77508%20(24728)/fast/dom/Window/window-properties-pretty-diff.html

http://trac.webkit.org/changeset/77495 might have broken GTK Linux 32-bit Debug