Summary: | Don't use ScriptProfiler to find canvases for instrumentation | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dean Jackson <dino> | ||||
Component: | Web Inspector | Assignee: | Dean Jackson <dino> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | commit-queue, esprehn+autocc, graouts, joepeck, timothy, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Dean Jackson
2013-08-13 16:54:00 PDT
Created attachment 208690 [details]
Patch
Comment on attachment 208690 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=208690&action=review r=me > Source/WebCore/ChangeLog:10 > + InspectorCanvasAgent::findFramesWithUninstrumentedCanvases uses a ScriptProfiler to walk the tree > + looking for canvas elements. This is currently not implemented in JSC, but we can do this directly > + with DOM methods. We're only looking for Canvas elements that have a context, so there isn't a need > + for this abstract walking object. So we discussed this on IRC and this doesn't take into account -webkit-canvas(…) CSS Canvases or even Shadow DOM Canvases if they existed. I don't know if the original code handled those, but it is something that we should eventually handle. I think you could add a FIXME in source code. > Source/WebCore/html/HTMLCanvasElement.h:194 > +inline bool isHTMLCanvasElement(Node* node) > +{ > + return node->hasTagName(HTMLNames::canvasTag); > +} This one shouldn't be needed, the const one should work just fine with non-const Node*'s passed in. > Source/WebCore/inspector/InspectorCanvasAgent.cpp:270 > + if (!frame->document() || frame->page() != m_pageAgent->page()) I suspect the `page != pageAgent->page` check at the end of this can be removed now that we are walking this explicit page's frame tree. > Source/WebCore/inspector/InspectorCanvasAgent.cpp:278 > + if (canvas->renderingContext()) > + m_framesWithUninstrumentedCanvases.set(frame, true); You can `break;` inside this if statement to break out of this inner for loop. Committed r154033: <http://trac.webkit.org/changeset/154033> |