Bug 176988 – Web Inspector: REGRESSION(r222057): recording state doesn't update when changing actions

• Component: Web Inspector
• Status: RESOLVED FIXED

[Devin Rousso]

The changes in <https://webkit.org/b/176936> caused the details sidebars to first request that the recording's actions promise resolved before attempting to pull information from the context/action.  In the case of the Trace panel, this was fine since it doesn't need the context to render its content.  For the State panel, however, it needs to be able to access the context before `restore()` is called.  Since the recording's actions promise will resolve on the next tick, the synchronous `WI.RecordingContentView.prototype._generateContentCanvas2D()` will run to completion before the State panel has a chance to do so, meaning that the State panel will access the context after it has been `restore()`d.

[Devin Rousso]

Created attachment 320878 [details]
Patch

[Matt Baker]

Comment on attachment 320878 [details]
Patch

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

r=me

> Source/WebInspectorUI/UserInterface/Views/RecordingStateDetailsSidebarPanel.js:33
> +        this._action = NaN;

Should be null instead of NaN.

> Source/WebInspectorUI/UserInterface/Views/RecordingStateDetailsSidebarPanel.js:84
> +    _generateDetailsCanvas2D(action, context, options = {})

A more descriptive name would be nice. How about `_populateCanvasStateDataGrid`? I think having the name be canvas-type agnostic is fine.

> Source/WebInspectorUI/UserInterface/Views/RecordingTraceDetailsSidebarPanel.js:82
> +            noTraceDataMessageElement.textContent = WI.UIString("No Trace Data");

What about "Call Stack Unavailable" or "No Call Stack" instead? As an aside, I think we should standardize on referring to traces/stack traces as call stacks in user-facing text. Variables names for things can still be trace, etc.

[Devin Rousso]

Comment on attachment 320878 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Views/RecordingStateDetailsSidebarPanel.js:84
>> +    _generateDetailsCanvas2D(action, context, options = {})
> 
> A more descriptive name would be nice. How about `_populateCanvasStateDataGrid`? I think having the name be canvas-type agnostic is fine.

In the future, we may want to display state information about WebGL contexts (such as which buffer is bound).  I think that keeping this name as is makes it easier to distinguish what context type will be used when generating the content.  Furthermore, this matches the naming style of RecordingNavigationSidebarPanel.

[Devin Rousso]

Created attachment 320978 [details]
Patch

[Devin Rousso]

Created attachment 320979 [details]
Patch

Forgot to change bug title

[WebKit Commit Bot]

Comment on attachment 320979 [details]
Patch

Clearing flags on attachment: 320979

Committed r222124: <http://trac.webkit.org/changeset/222124>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/34693265>