Bug 76913 – Web Inspector: should be possible to open function declaration from script popover

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

[Yury Semikhatsky]

Original Chromium bug report: http://code.google.com/p/chromium/issues/detail?id=100157

[Yury Semikhatsky]

Created attachment 123732 [details]
Patch

[Yury Semikhatsky]

(In reply to comment #1)
> Created an attachment (id=123732) [details]
> Patch

This patch depends on changes in V8 API.

[WebKit Review Bot]

Comment on attachment 123732 [details]
Patch

Attachment 123732 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11334398

[Pavel Feldman]

Is there a screenshot?

[Yury Semikhatsky]

Created attachment 123914 [details]
Screenshot with the patch applied

[Yury Semikhatsky]

(In reply to comment #4)
> Is there a screenshot?

Yes, see attachment.

[Pavel Feldman]

Comment on attachment 123732 [details]
Patch

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

> Source/WebCore/inspector/InjectedScriptHost.idl:48
> +        [Custom] DOMString functionInferredName(in DOMObject object);

I would combine these into a single getter. I.e. extended location would contain function name. As a result, the protocol would have

Runtime.getFunctionInfo returning location, name, displayName and inferredName as separate optional properties.

> Source/WebCore/inspector/InjectedScriptSource.js:502
> +            var name = object.name;

You should use object.displayName || object.name.

[Yury Semikhatsky]

Created attachment 123956 [details]
Patch

[Yury Semikhatsky]

(In reply to comment #7)
> (From update of attachment 123732 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=123732&action=review
> 
> > Source/WebCore/inspector/InjectedScriptHost.idl:48
> > +        [Custom] DOMString functionInferredName(in DOMObject object);
> 
> I would combine these into a single getter. I.e. extended location would contain function name. As a result, the protocol would have
Done.

> 
> Runtime.getFunctionInfo returning location, name, displayName and inferredName as separate optional properties.
> 
> > Source/WebCore/inspector/InjectedScriptSource.js:502
> > +            var name = object.name;
> 
> You should use object.displayName || object.name.
Done.

[WebKit Review Bot]

Attachment 123956 [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/InjectedScript.h:81:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Total errors found: 2 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Comment on attachment 123956 [details]
Patch

Attachment 123956 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11242849

[Yury Semikhatsky]

Created attachment 123968 [details]
Patch

[Yury Semikhatsky]

(In reply to comment #12)
> Created an attachment (id=123968) [details]
> Patch

Fixed Win build, Source/JavaScriptCore/JavaScriptCore.exp still needs to be changed for the patch to compile on Mac.

[WebKit Review Bot]

Attachment 123968 [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/InjectedScript.h:81:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Total errors found: 2 in 22 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Comment on attachment 123968 [details]
Patch

Attachment 123968 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11242878

[Yury Semikhatsky]

Created attachment 124096 [details]
Patch for landing

Fixed Mac compilation.

[WebKit Review Bot]

Attachment 124096 [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/JavaScriptCore/ChangeLog:11:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Source/WebCore/inspector/InjectedScript.cpp:91:  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: 3 in 23 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Comment on attachment 124096 [details]
Patch for landing

Attachment 124096 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11163302

[Yury Semikhatsky]

Committed r106101: <http://trac.webkit.org/changeset/106101>