Web Inspector: function remote objetct should provide access to script id and the function position in the script.
Created attachment 114495 [details] Patch
Attachment 114495 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/insp..." exit_code: 1 Source/WebCore/inspector/InjectedScript.cpp:91: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorDebuggerAgent.cpp:357: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorDebuggerAgent.h:87: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InjectedScript.h:81: The parameter name "errorString" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/inspector/InjectedScript.h:81: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Total errors found: 5 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #1) > Created an attachment (id=114495) [details] > Patch Note that this patch depends on this V8 change which has not been pushed to trunk yet: http://code.google.com/p/v8/source/detail?r=9935
Created attachment 114496 [details] Patch
Attachment 114496 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/insp..." exit_code: 1 Source/WebCore/inspector/InjectedScript.cpp:91: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorDebuggerAgent.cpp:357: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorDebuggerAgent.h:87: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InjectedScript.h:81: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Total errors found: 4 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 114496 [details] Patch Attachment 114496 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10400334
Comment on attachment 114496 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=114496&action=review r- for failing JSC bots. > LayoutTests/inspector/debugger/function-location-expected.txt:9 > +columnNumber: -1 You should not provide columnNumber in case it is not known.
Created attachment 114518 [details] Patch
Attachment 114518 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/insp..." exit_code: 1 Source/WebCore/inspector/InjectedScript.cpp:91: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorDebuggerAgent.cpp:357: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorDebuggerAgent.h:87: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InjectedScript.h:81: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Total errors found: 4 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 114640 [details] Patch
Attachment 114640 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/insp..." exit_code: 1 Source/WebCore/inspector/InjectedScript.cpp:91: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorDebuggerAgent.cpp:357: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorDebuggerAgent.h:87: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InjectedScript.h:81: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Total errors found: 4 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 114664 [details] Patch
Attachment 114664 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/insp..." exit_code: 1 Source/WebCore/inspector/InjectedScript.cpp:91: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorDebuggerAgent.cpp:357: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorDebuggerAgent.h:87: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InjectedScript.h:81: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Total errors found: 4 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 114667 [details] Patch
Attachment 114667 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/insp..." exit_code: 1 Source/WebCore/inspector/InjectedScript.cpp:91: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorDebuggerAgent.cpp:357: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorDebuggerAgent.h:87: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InjectedScript.h:81: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Total errors found: 4 in 17 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 114664 [details] Patch Inspector part looks good, please check with JSC folks whether exposing SourceCode on JSFunction is Ok.
Created attachment 114671 [details] Patch
Attachment 114671 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/insp..." exit_code: 1 Source/WebCore/inspector/InjectedScript.cpp:91: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorDebuggerAgent.cpp:357: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorDebuggerAgent.h:87: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InjectedScript.h:81: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Total errors found: 4 in 17 files If any of these errors are false positives, please file a bug against check-webkit-style.
Committed r99950: <http://trac.webkit.org/changeset/99950>
Landed this patch instead of another one by mistake which is why the bug was closed, reopening.
Created attachment 114686 [details] Patch
Attachment 114686 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/insp..." exit_code: 1 Source/WebCore/inspector/InjectedScript.cpp:91: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorDebuggerAgent.cpp:357: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorDebuggerAgent.h:87: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InjectedScript.h:81: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Total errors found: 4 in 19 files If any of these errors are false positives, please file a bug against check-webkit-style.
Alexei or Geoffrey could you look at the JSC-related changes, in particular 1. If you are fine with adding JSFunction::sourceCode getter which is used to avoid direct access to runtime/Executable.h that is not visible from WebCore. 2. In JSInjectedScriptHost::functionLocation new JSObject is created and kept as a raw pointer. I've seen such code in other bindings but I'm not sure if I have to create a handle to the object to protect it from being GC'ed or can assume that GC won't run while I'm in that method.
> 1. If you are fine with adding JSFunction::sourceCode getter which is used to avoid direct access to runtime/Executable.h that is not visible from WebCore. Seems fine. > 2. In JSInjectedScriptHost::functionLocation new JSObject is created and kept as a raw pointer. I've seen such code in other bindings but I'm not sure if I have to create a handle to the object to protect it from being GC'ed or can assume that GC won't run while I'm in that method. This code is also fine. For future reference, the reason this is OK is not that GC won't run. (You're allocating a string, so GC might run.) Rather, it's OK because the collector scans the C++ stack, and you have a reference to your new object on the C++ stack.
(In reply to comment #24) > > 2. In JSInjectedScriptHost::functionLocation new JSObject is created and kept as a raw pointer. I've seen such code in other bindings but I'm not sure if I have to create a handle to the object to protect it from being GC'ed or can assume that GC won't run while I'm in that method. > > This code is also fine. For future reference, the reason this is OK is not that GC won't run. (You're allocating a string, so GC might run.) Rather, it's OK because the collector scans the C++ stack, and you have a reference to your new object on the C++ stack. That is what I suspected, thanks for the explanation!
Comment on attachment 114686 [details] Patch Attachment 114686 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10448611
Committed r100113: <http://trac.webkit.org/changeset/100113>