Bug 48058 - Web Inspector: decouple ScriptArguments from ScriptCallStack
Summary: Web Inspector: decouple ScriptArguments from ScriptCallStack
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Yury Semikhatsky
URL:
Keywords:
Depends on: 48122
Blocks:
  Show dependency treegraph
 
Reported: 2010-10-21 03:26 PDT by Yury Semikhatsky
Modified: 2010-11-12 07:21 PST (History)
14 users (show)

See Also:


Attachments
Patch (58.76 KB, patch)
2010-10-21 03:43 PDT, Yury Semikhatsky
no flags Details | Formatted Diff | Diff
Manually uploaded patch (111.72 KB, patch)
2010-10-21 03:57 PDT, Yury Semikhatsky
no flags Details | Formatted Diff | Diff
Patch to land (119.71 KB, patch)
2010-10-21 05:11 PDT, Yury Semikhatsky
no flags Details | Formatted Diff | Diff
Patch to land (122.58 KB, patch)
2010-10-21 05:28 PDT, Yury Semikhatsky
no flags Details | Formatted Diff | Diff
patch (127.68 KB, patch)
2010-10-21 11:23 PDT, Yury Semikhatsky
no flags Details | Formatted Diff | Diff
Patch (129.09 KB, patch)
2010-10-22 08:20 PDT, Yury Semikhatsky
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yury Semikhatsky 2010-10-21 03:26:14 PDT
We need ScriptState* only for script arguments serialization. However, in many cases we need to store stack trace and don't have any script arguments. To support those cases we need to extract script arguments into a separate class ScriptArguments and use it instead of ScriptCallStack for passing the arguments.
Comment 1 Yury Semikhatsky 2010-10-21 03:43:03 PDT
Created attachment 71416 [details]
Patch
Comment 2 Yury Semikhatsky 2010-10-21 03:57:35 PDT
Created attachment 71419 [details]
Manually uploaded patch
Comment 3 Early Warning System Bot 2010-10-21 04:05:04 PDT
Attachment 71416 [details] did not build on qt:
Build output: http://queues.webkit.org/results/4671008
Comment 4 Pavel Feldman 2010-10-21 04:05:48 PDT
Comment on attachment 71419 [details]
Manually uploaded patch

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

> WebCore/bindings/scripts/CodeGeneratorJS.pm:1970
> +                        push(@implContent, "    size_t maxStackSize = imp->shouldCaptureFullStackTrace() ? ScriptCallStack::maxCallStackSizeToCapture : 1;\n");

Nit: I'd use the opposite one "shouldCaptureTopFrameOnly()"
Comment 5 Early Warning System Bot 2010-10-21 04:56:27 PDT
Attachment 71419 [details] did not build on qt:
Build output: http://queues.webkit.org/results/4724001
Comment 6 Yury Semikhatsky 2010-10-21 05:11:34 PDT
Created attachment 71424 [details]
Patch to land

Added necessary changes to other projects.
Comment 7 Yury Semikhatsky 2010-10-21 05:28:04 PDT
Created attachment 71425 [details]
Patch to land

Updated V8TestObj.cpp and JSTestObj.cpp
Comment 8 Yury Semikhatsky 2010-10-21 05:35:18 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	C	WebCore/bindings/js/ScriptCallFrame.cpp => WebCore/bindings/js/ScriptCallStackFactory.h
	C	WebCore/bindings/js/ScriptCallFrame.cpp => WebCore/bindings/v8/ScriptCallStackFactory.h
	C	WebCore/bindings/v8/ScriptCallFrame.cpp => WebCore/inspector/ScriptArguments.cpp
	C	WebCore/bindings/js/ScriptCallFrame.h => WebCore/inspector/ScriptArguments.h
	R	WebCore/bindings/js/ScriptCallStack.cpp => WebCore/bindings/js/ScriptCallStackFactory.cpp
	R	WebCore/bindings/v8/ScriptCallStack.cpp => WebCore/bindings/v8/ScriptCallStackFactory.cpp
	R	WebCore/bindings/v8/ScriptCallFrame.cpp => WebCore/inspector/ScriptCallFrame.cpp
	R	WebCore/bindings/js/ScriptCallFrame.h => WebCore/inspector/ScriptCallFrame.h
	R	WebCore/bindings/js/ScriptCallFrame.cpp => WebCore/inspector/ScriptCallStack.cpp
	R	WebCore/bindings/js/ScriptCallStack.h => WebCore/inspector/ScriptCallStack.h
	D	WebCore/bindings/v8/ScriptCallFrame.h
	D	WebCore/bindings/v8/ScriptCallStack.h
	M	WebCore/Android.jscbindings.mk
	M	WebCore/Android.v8bindings.mk
	M	WebCore/CMakeLists.txt
	M	WebCore/ChangeLog
	M	WebCore/GNUmakefile.am
	M	WebCore/WebCore.gypi
	M	WebCore/WebCore.pro
	M	WebCore/WebCore.vcproj/WebCore.vcproj
	M	WebCore/WebCore.xcodeproj/project.pbxproj
	M	WebCore/bindings/js/JSBindingsAllInOne.cpp
	M	WebCore/bindings/js/ScriptState.h
	M	WebCore/bindings/scripts/CodeGeneratorJS.pm
	M	WebCore/bindings/scripts/CodeGeneratorV8.pm
	M	WebCore/bindings/scripts/test/JS/JSTestObj.cpp
	M	WebCore/bindings/scripts/test/V8/V8TestObj.cpp
	M	WebCore/bindings/v8/ScriptController.cpp
	M	WebCore/bindings/v8/ScriptState.h
	M	WebCore/bindings/v8/V8ConsoleMessage.cpp
	M	WebCore/bindings/v8/V8ConsoleMessage.h
	M	WebCore/bindings/v8/custom/V8ConsoleCustom.cpp
	M	WebCore/inspector/ConsoleMessage.cpp
	M	WebCore/inspector/ConsoleMessage.h
	M	WebCore/inspector/InspectorController.cpp
	M	WebCore/inspector/InspectorController.h
	M	WebCore/page/Console.cpp
	M	WebCore/page/Console.h
Committed r70232
Comment 9 Yury Semikhatsky 2010-10-21 07:00:47 PDT
The patch was reverted in r70235.
Comment 10 Eric Seidel (no email) 2010-10-21 10:31:30 PDT
Attachment 71416 [details] did not build on mac:
Build output: http://queues.webkit.org/results/4634013
Comment 11 Yury Semikhatsky 2010-10-21 11:23:37 PDT
Created attachment 71463 [details]
patch

This patch fixes compilation issue on Chromium Linux and profiler layout test failures.
Comment 12 WebKit Review Bot 2010-10-22 05:34:25 PDT
http://trac.webkit.org/changeset/70298 might have broken Leopard Intel Debug (Tests)
The following tests are not passing:
editing/spelling/context-menu-suggestions.html
editing/spelling/spellcheck-attribute.html
editing/spelling/spelling-backspace-between-lines.html
editing/spelling/spelling-contenteditable.html
editing/spelling/spelling-textarea.html
platform/mac/accessibility/attributed-string-includes-misspelled-with-selection.html
platform/mac/accessibility/misspelled-attributed-string.html
Comment 13 Yury Semikhatsky 2010-10-22 08:20:11 PDT
Created attachment 71565 [details]
Patch

This patch should compile on WebKit Win bot after I fixed transitive include issue in ScriptState.h, also fixed console-log-before-inspector-open test failure.
Comment 14 Eric Seidel (no email) 2010-10-22 11:02:33 PDT
Comment on attachment 71419 [details]
Manually uploaded patch

Cleared Pavel Feldman's review+ from obsolete attachment 71419 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment 15 Yury Semikhatsky 2010-11-12 07:21:37 PST
Commited r71515.