Bug 129913

Summary: Web Inspector: show a debugging-oriented dashboard when scripts pause
Product: WebKit Reporter: BJ Burg <bburg>
Component: Web InspectorAssignee: BJ Burg <bburg>
Status: RESOLVED FIXED    
Severity: Normal CC: graouts, joepeck, timothy, webkit-bug-importer
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 129898    
Bug Blocks:    
Attachments:
Description Flags
Debugger dashboard - paused at full size
none
paused at small size, full width
none
oops, dup
none
paused at small size, narrow width
none
the patch timothy: review+

Description BJ Burg 2014-03-07 14:02:08 PST
this is going to be sweet!
Comment 1 BJ Burg 2014-03-09 19:12:42 PDT
Created attachment 226266 [details]
Debugger dashboard - paused at full size
Comment 2 BJ Burg 2014-03-09 19:13:10 PDT
Created attachment 226267 [details]
paused at small size, full width
Comment 3 BJ Burg 2014-03-09 19:13:31 PDT
Created attachment 226268 [details]
oops, dup
Comment 4 BJ Burg 2014-03-09 19:14:28 PDT
Created attachment 226269 [details]
paused at small size, narrow width
Comment 5 BJ Burg 2014-03-09 19:44:05 PDT
Created attachment 226274 [details]
the patch
Comment 6 BJ Burg 2014-03-09 19:45:31 PDT
this patch depends on 129898, so the bots will have trouble.
Comment 7 Timothy Hatcher 2014-03-09 20:11:39 PDT
Comment on attachment 226274 [details]
the patch

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

> Source/WebInspectorUI/UserInterface/Models/DebuggerDashboard.js:35
> +    __proto__: WebInspector.Object.prototype,
> +
> +};

Nit: stray newline.

> Source/WebInspectorUI/UserInterface/Views/ButtonNavigationItem.css:63
> +    background-color: #999;

Nit: rgb(153, 153, 153)

> Source/WebInspectorUI/UserInterface/Views/DebuggerDashboardView.css:73
> +    from { opacity: 0.7; -webkit-transform: scale(1); }
> +    to { opacity: 1; -webkit-transform: scale(1.1); }

Not sure I would do scale. But I would have to see it.

> Source/WebInspectorUI/UserInterface/Views/DebuggerDashboardView.js:58
> +WebInspector.DebuggerDashboardView.FunctionIconStyleClassName = WebInspector.CallFrameTreeElement.FunctionIconStyleClassName;
> +WebInspector.DebuggerDashboardView.EventListenerIconStyleClassName = WebInspector.CallFrameTreeElement.EventListenerIconStyleClassName;

There are other places in the code where we cross reference directly. I like this approach better though.

> Source/WebInspectorUI/UserInterface/Views/DebuggerDashboardView.js:85
> +        // This is more than likely an event listener function with an "on" prefix and it is
> +        // as long or longer than the shortest event listener name -- "oncut".
> +        if (callFrame.functionName && callFrame.functionName.startsWith("on") && callFrame.functionName.length >= 5)
> +            iconClassName = WebInspector.DebuggerDashboardView.EventListenerIconStyleClassName;

We should probably have a helper for this logic. I think this is the third copy of it.
Comment 8 BJ Burg 2014-03-10 11:06:01 PDT
Comment on attachment 226274 [details]
the patch

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

>> Source/WebInspectorUI/UserInterface/Views/DebuggerDashboardView.css:73
>> +    to { opacity: 1; -webkit-transform: scale(1.1); }
> 
> Not sure I would do scale. But I would have to see it.

It's subtle enough that it looks more like a pulse instead of a corny ad. But it could definitely be tweaked further with your help. The glyph could probably be darker and have some sort of bezel to make it stand out better.

>> Source/WebInspectorUI/UserInterface/Views/DebuggerDashboardView.js:85
>> +            iconClassName = WebInspector.DebuggerDashboardView.EventListenerIconStyleClassName;
> 
> We should probably have a helper for this logic. I think this is the third copy of it.

This version omits the native call frame check since we can't pause in/on those as the top call frame (AFAIK?)
Comment 9 BJ Burg 2014-03-10 11:09:16 PDT
Committed r165383: <http://trac.webkit.org/changeset/165383>