Bug 176988 - Web Inspector: REGRESSION(r222057): recording state doesn't update when changing actions
Summary: Web Inspector: REGRESSION(r222057): recording state doesn't update when chang...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on: 176936
Blocks: WebInspectorCanvasRecording
  Show dependency treegraph
 
Reported: 2017-09-15 00:55 PDT by Devin Rousso
Modified: 2017-09-27 12:25 PDT (History)
4 users (show)

See Also:


Attachments
Patch (9.75 KB, patch)
2017-09-15 00:57 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (11.22 KB, patch)
2017-09-15 17:43 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (11.21 KB, patch)
2017-09-15 17:44 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Devin Rousso 2017-09-15 00:55:27 PDT
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.
Comment 1 Devin Rousso 2017-09-15 00:57:27 PDT
Created attachment 320878 [details]
Patch
Comment 2 Matt Baker 2017-09-15 13:44:12 PDT
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.
Comment 3 Devin Rousso 2017-09-15 17:40:52 PDT
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.
Comment 4 Devin Rousso 2017-09-15 17:43:23 PDT
Created attachment 320978 [details]
Patch
Comment 5 Devin Rousso 2017-09-15 17:44:50 PDT
Created attachment 320979 [details]
Patch

Forgot to change bug title
Comment 6 WebKit Commit Bot 2017-09-15 19:12:20 PDT
Comment on attachment 320979 [details]
Patch

Clearing flags on attachment: 320979

Committed r222124: <http://trac.webkit.org/changeset/222124>
Comment 7 WebKit Commit Bot 2017-09-15 19:12:22 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Radar WebKit Bug Importer 2017-09-27 12:25:54 PDT
<rdar://problem/34693265>