Bug 147348

Summary: Web Inspector: Type profiler return types aren't showing up
Product: WebKit Reporter: Saam Barati <saam>
Component: JavaScriptCoreAssignee: Saam Barati <saam>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
patch
joepeck: review+, joepeck: commit-queue-
patch none

Description Saam Barati 2015-07-27 19:09:07 PDT
The JSC patch that changed text offsets to start from the open paren of the parameter list 
instead of the open brace of the function body broke the type profiler's protocol.
Comment 1 Saam Barati 2015-07-27 19:16:16 PDT
Created attachment 257626 [details]
patch
Comment 2 Joseph Pecoraro 2015-07-28 12:30:16 PDT
Comment on attachment 257626 [details]
patch

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

> Source/WebInspectorUI/UserInterface/Models/ScriptSyntaxTree.js:179
> -                    divot: node.body.range[0]
> +                    divot: node.isGetterOrSetter ? node.getterOrSetterRange[0] : node.range[0] // "f" in function, "s" in set, "g" in get, first letter in any method name for classes.

Is this compatible with iOS 9 (a soon to be considered Legacy backend that wouldn't have had your recent backend changes).

If not, maybe we should make this a function so that it would be easier to read. Something like:

    function functionReturnDivot(node)
    {
        // COMPATIBILITY (iOS 9): Legacy Backends something something something.
        // FIXME: Need a better way to determine backend versions. Using DOM.pseudoElement because that was added after iOS 9.
        if (!DOMAgent.hasEvent("pseudoElementAdded"))
            return node.body.range[0];

        // "f" in function, "s" in set, "g" in get, first letter in any method name for classes.
        return node.isGetterOrSetter ? node.getterOrSetterRange[0] : node.range[0];
    }

Would be good to have a frontend test for this so it won't regress again in the future. But I won't make you write one now.
Comment 3 Saam Barati 2015-08-17 13:40:29 PDT
Created attachment 259178 [details]
patch
Comment 4 Matt Baker 2015-08-17 14:33:17 PDT
Comment on attachment 259178 [details]
patch

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

> Source/WebInspectorUI/ChangeLog:8
> +        Bug #145995 changed the starting offset of a function to 

Nit: We typically use the form <https://webkit.org/b/xxxxxx> when referencing bugs in the WebInspector change log.
Comment 5 BJ Burg 2015-08-17 14:38:27 PDT
Comment on attachment 259178 [details]
patch

r=me
Comment 6 WebKit Commit Bot 2015-08-17 15:57:31 PDT
Comment on attachment 259178 [details]
patch

Clearing flags on attachment: 259178

Committed r188549: <http://trac.webkit.org/changeset/188549>
Comment 7 WebKit Commit Bot 2015-08-17 15:57:35 PDT
All reviewed patches have been landed.  Closing bug.