Bug 76913 - Web Inspector: should be possible to open function declaration from script popover
Summary: Web Inspector: should be possible to open function declaration from script po...
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:
Blocks:
 
Reported: 2012-01-24 07:21 PST by Yury Semikhatsky
Modified: 2012-01-27 01:53 PST (History)
14 users (show)

See Also:


Attachments
Patch (13.85 KB, patch)
2012-01-24 07:51 PST, Yury Semikhatsky
no flags Details | Formatted Diff | Diff
Screenshot with the patch applied (210.55 KB, image/png)
2012-01-25 03:35 PST, Yury Semikhatsky
no flags Details
Patch (37.23 KB, patch)
2012-01-25 09:25 PST, Yury Semikhatsky
no flags Details | Formatted Diff | Diff
Patch (38.09 KB, patch)
2012-01-25 10:16 PST, Yury Semikhatsky
pfeldman: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Patch for landing (37.07 KB, patch)
2012-01-26 03:40 PST, Yury Semikhatsky
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yury Semikhatsky 2012-01-24 07:21:34 PST
Original Chromium bug report: http://code.google.com/p/chromium/issues/detail?id=100157
Comment 1 Yury Semikhatsky 2012-01-24 07:51:24 PST
Created attachment 123732 [details]
Patch
Comment 2 Yury Semikhatsky 2012-01-24 07:52:19 PST
(In reply to comment #1)
> Created an attachment (id=123732) [details]
> Patch

This patch depends on changes in V8 API.
Comment 3 WebKit Review Bot 2012-01-24 08:18:26 PST
Comment on attachment 123732 [details]
Patch

Attachment 123732 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11334398
Comment 4 Pavel Feldman 2012-01-25 01:32:39 PST
Is there a screenshot?
Comment 5 Yury Semikhatsky 2012-01-25 03:35:29 PST
Created attachment 123914 [details]
Screenshot with the patch applied
Comment 6 Yury Semikhatsky 2012-01-25 03:35:53 PST
(In reply to comment #4)
> Is there a screenshot?

Yes, see attachment.
Comment 7 Pavel Feldman 2012-01-25 04:08:25 PST
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.
Comment 8 Yury Semikhatsky 2012-01-25 09:25:38 PST
Created attachment 123956 [details]
Patch
Comment 9 Yury Semikhatsky 2012-01-25 09:26:12 PST
(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.
Comment 10 WebKit Review Bot 2012-01-25 09:28:00 PST
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.
Comment 11 WebKit Review Bot 2012-01-25 09:52:22 PST
Comment on attachment 123956 [details]
Patch

Attachment 123956 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11242849
Comment 12 Yury Semikhatsky 2012-01-25 10:16:17 PST
Created attachment 123968 [details]
Patch
Comment 13 Yury Semikhatsky 2012-01-25 10:17:18 PST
(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.
Comment 14 WebKit Review Bot 2012-01-25 10:19:58 PST
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.
Comment 15 WebKit Review Bot 2012-01-25 11:00:57 PST
Comment on attachment 123968 [details]
Patch

Attachment 123968 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11242878
Comment 16 Yury Semikhatsky 2012-01-26 03:40:11 PST
Created attachment 124096 [details]
Patch for landing

Fixed Mac compilation.
Comment 17 WebKit Review Bot 2012-01-26 03:41:43 PST
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.
Comment 18 WebKit Review Bot 2012-01-26 04:01:29 PST
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
Comment 19 Yury Semikhatsky 2012-01-27 01:53:53 PST
Committed r106101: <http://trac.webkit.org/changeset/106101>