[Webkit-unassigned] [Bug 137278] Web Inspector: add 2D/WebGL canvas instrumentation infrastructure

[bugzilla-daemon at webkit.org]

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


Timothy Hatcher <timothy at apple.com> changed:

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




--- Comment #9 from Timothy Hatcher <timothy at apple.com>  2014-10-07 14:42:57 PST ---
(From update of attachment 239375)
View in context: https://bugs.webkit.org/attachment.cgi?id=239375&action=review

Looking real good. Some more tweaks and it will be ready!

> Source/WebCore/html/HTMLCanvasElement.h:36
> +#include <wtf/HashSet.h>

Not needed now?

> Source/WebInspectorUI/UserInterface/Controllers/CanvasManager.js:49
> +    canvasesForFrame: function(frameId, callback)

Spell out frameIdentifier.

This can use promises. The callback system is on its way out. It should clean up this function a lot!

See: http://trac.webkit.org/wiki/WebInspectorCodingStyleGuide#UnderstandingandUsingPromises

Some examples are DebuggerManager.pause and ReplayManager.getSession.

> Source/WebInspectorUI/UserInterface/Views/CanvasTreeElement.js:43
> +WebInspector.CanvasTreeElement.prototype = {
> +    constructor: WebInspector.CanvasTreeElement
> +};
> +
> +WebInspector.CanvasTreeElement.prototype.__proto__ = WebInspector.GeneralTreeElement.prototype;

__proto__ should move into the block next to constructor property.

> Source/WebInspectorUI/UserInterface/Views/FrameTreeElement.js:208
> +            for (var i = 0; i < canvases.length; ++i)
> +                this._addTreeElementForRepresentedObject(canvases[i]);

Should do:
for (var canvas of canvases)
    this._addTreeElementForRepresentedObject(canvas);

> Source/WebInspectorUI/UserInterface/Views/FrameTreeElement.js:564
> +        if (representedObject instanceof WebInspector.ShaderProgram) {
> +            if (!this._shadersFolderTreeElement)
> +                this._shadersFolderTreeElement = createFolderTreeElement.call(this, "shaders", WebInspector.UIString("Shaders"));
> +            return this._shadersFolderTreeElement;
> +        }

Sounds like this will change, since Shaders will go under the owning Canvas.

> Source/WebInspectorUI/UserInterface/Views/ShaderProgramTreeElement.js:41
> +WebInspector.ShaderProgramTreeElement.prototype = {
> +    constructor: WebInspector.ShaderProgramTreeElement
> +};
> +
> +WebInspector.ShaderProgramTreeElement.prototype.__proto__ = WebInspector.GeneralTreeElement.prototype;

Ditto re: __proto__.

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