[Webkit-unassigned] [Bug 89678] Web Inspector: Support 'Restart frame' in inspector frontend

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=89678


Pavel Feldman <pfeldman at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #148840|review?                     |review-
               Flag|                            |




--- Comment #2 from Pavel Feldman <pfeldman at chromium.org>  2012-06-21 11:11:46 PST ---
(From update of attachment 148840)
View in context: https://bugs.webkit.org/attachment.cgi?id=148840&action=review

> Source/WebCore/inspector/front-end/CallStackSidebarPane.js:110
> +    contributeToContextMenu: function(contextMenu) {

{ should go to the next line

> Source/WebCore/inspector/front-end/CallStackSidebarPane.js:111
>          contextMenu.appendItem(WebInspector.UIString("Copy Stack Trace"), this._copyStackTrace.bind(this));

I'd rather move this into placard. You can call _copyStackTrace from there (it is private to this file).

> Source/WebCore/inspector/front-end/CallStackSidebarPane.js:188
> +    _placardContextMenu: function(event) {

{ on the next line

> Source/WebCore/inspector/front-end/CallStackSidebarPane.js:190
> +        contextMenu.appendItem(WebInspector.UIString("Restart Frame"), this._restartFrame.bind(this), !WebInspector.debuggerModel.canSetScriptSource());

canSetScriptSource is constant for the port. You should not disable it on that basis, you should just add the item depending on the value.

> Source/WebCore/inspector/front-end/CallStackSidebarPane.js:196
> +    _restartFrame: function() {

{ on the next line

> Source/WebCore/inspector/front-end/DebuggerModel.js:552
> +    callStackModified: function(newCallFrames, details) {

{ on the next line

> Source/WebCore/inspector/front-end/DebuggerModel.js:554
> +        if (details && details["stack_update_needs_step_in"]) {

No need for { } for the upper branch

> Source/WebCore/inspector/front-end/DebuggerModel.js:555
> +            DebuggerAgent.stepInto();

You should do this in the backend.

> Source/WebCore/inspector/front-end/DebuggerModel.js:740
> +            if (!error) {

no need for { }

> Source/WebCore/inspector/front-end/DebuggerModel.js:743
> +            if (callback) {

ditto

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.