Implement paint flashing via GraphicsLayers in the WK2 inspector overlay
Created attachment 236943 [details] Patch
<rdar://problem/18095991>
Attachment 236943 [details] did not pass style-queue: ERROR: Source/WebCore/WebCore.exp.in:0: Source/WebCore/WebCore.exp.in should be sorted, use Tools/Scripts/sort-export-file script [list/order] [5] Total errors found: 1 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 236943 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=236943&action=review > Source/WebKit2/WebProcess/WebCoreSupport/WebInspectorClient.cpp:54 > + virtual void notifyAnimationEnded(const GraphicsLayer* layer, const String&) This should have override. > Source/WebKit2/WebProcess/WebCoreSupport/WebInspectorClient.h:93 > + HashSet<WebCore::GraphicsLayer*> m_paintRectLayers; It might be worth noting that this should use a HashSet<std::unique_ptr<WebCore::GraphicsLayer>> but can't due to HashSet issues (and point to a bug).
Comment on attachment 236943 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=236943&action=review > Source/WebCore/inspector/InspectorInstrumentation.cpp:503 > + if (InspectorPageAgent* pageAgent = instrumentingAgents->inspectorPageAgent()) { > + LayoutRect absoluteRect = LayoutRect(renderer->localToAbsoluteQuad(FloatRect(rect)).boundingBox()); > + FrameView* view = renderer->document().view(); > + > + LayoutRect rootRect = absoluteRect; > + if (!view->frame().isMainFrame()) { > + IntRect rootViewRect = view->contentsToRootView(pixelSnappedIntRect(absoluteRect)); > + rootRect = view->frame().mainFrame().view()->rootViewToContents(rootViewRect); > + } > + pageAgent->didPaint(rootRect); > + } Typically InspectorInstrumentation is just meant to be a very thin layer calling out to the Agents. Technically it could probably be auto generated. This logic could go in InspectorPageAgent. But I'm fine with it being here. We can do our own clean up later if we need to. > Source/WebKit2/WebProcess/WebCoreSupport/WebInspectorClient.cpp:46 > +class RepaintIndicatorLayerClient : public GraphicsLayerClient { Mark the class as final? > Source/WebKit2/WebProcess/WebCoreSupport/WebInspectorClient.cpp:139 > + m_page->installPageOverlay(m_paintRectOverlay, PageOverlay::FadeMode::DoNotFade); I think m_paintRectOverlay should be uninstalled in ~WebInspectorClient. Otherwise, won't this stay around because it is RefCounted and PageOverlayController will keep a reference. > Source/WebKit2/WebProcess/WebCoreSupport/WebInspectorClient.cpp:162 > + paintLayer->addAnimation(fadeKeyframes, FloatSize(), opacityAnimation.get(), "opacity", 0); Nit: ASCIILiteral("opacity") > Source/WebKit2/WebProcess/WebCoreSupport/WebInspectorClient.h:72 > + virtual bool overridesShowPaintRects() override { return true; } Nit: Ideally this would be a const method.
I addressed the comments. https://trac.webkit.org/r172864