Bug 48058

Summary: Web Inspector: decouple ScriptArguments from ScriptCallStack
Product: WebKit Reporter: Yury Semikhatsky <yurys>
Component: Web Inspector (Deprecated)Assignee: Yury Semikhatsky <yurys>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, apavlov, bweinstein, eric, joepeck, keishi, loislo, pfeldman, pmuellr, rik, timothy, webkit-ews, webkit.review.bot, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 48122    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Manually uploaded patch
none
Patch to land
none
Patch to land
none
patch
none
Patch none

Yury Semikhatsky
Reported 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.
Attachments
Patch (58.76 KB, patch)
2010-10-21 03:43 PDT, Yury Semikhatsky
no flags
Manually uploaded patch (111.72 KB, patch)
2010-10-21 03:57 PDT, Yury Semikhatsky
no flags
Patch to land (119.71 KB, patch)
2010-10-21 05:11 PDT, Yury Semikhatsky
no flags
Patch to land (122.58 KB, patch)
2010-10-21 05:28 PDT, Yury Semikhatsky
no flags
patch (127.68 KB, patch)
2010-10-21 11:23 PDT, Yury Semikhatsky
no flags
Patch (129.09 KB, patch)
2010-10-22 08:20 PDT, Yury Semikhatsky
no flags
Yury Semikhatsky
Comment 1 2010-10-21 03:43:03 PDT
Yury Semikhatsky
Comment 2 2010-10-21 03:57:35 PDT
Created attachment 71419 [details] Manually uploaded patch
Early Warning System Bot
Comment 3 2010-10-21 04:05:04 PDT
Pavel Feldman
Comment 4 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()"
Early Warning System Bot
Comment 5 2010-10-21 04:56:27 PDT
Yury Semikhatsky
Comment 6 2010-10-21 05:11:34 PDT
Created attachment 71424 [details] Patch to land Added necessary changes to other projects.
Yury Semikhatsky
Comment 7 2010-10-21 05:28:04 PDT
Created attachment 71425 [details] Patch to land Updated V8TestObj.cpp and JSTestObj.cpp
Yury Semikhatsky
Comment 8 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
Yury Semikhatsky
Comment 9 2010-10-21 07:00:47 PDT
The patch was reverted in r70235.
Eric Seidel (no email)
Comment 10 2010-10-21 10:31:30 PDT
Yury Semikhatsky
Comment 11 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.
WebKit Review Bot
Comment 12 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
Yury Semikhatsky
Comment 13 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.
Eric Seidel (no email)
Comment 14 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.
Yury Semikhatsky
Comment 15 2010-11-12 07:21:37 PST
Commited r71515.
Note You need to log in before you can comment on or make changes to this bug.