Revert of Revert of Timeline: fix URLs and owner elements for image d…

…ecodes of background/border images (patchset #1 id:1 of https://codereview.chromium.org/766133002/) Reason for revert: This will re-land the original CL, as it appears to have no effect on the perf test referred to in BUG=437496 Original issue's description: > Revert of Timeline: fix URLs and owner elements for image decodes of background/border images (patchset #2 id:20001 of https://codereview.chromium.org/755633002/) > > Reason for revert: > Speculative revert to see whether this really could be a reson for https://code.google.com/p/chromium/issues/detail?id=437496 > > Original issue's description: > > Timeline: fix URLs and owner elements for image decodes of background/border images > > > > We rely on PaintImage events to provide context information available > > on the rendering level (image URLs and owning node id) for nested > > DecodeImage and ResizeImage events. However, we only had PaintImage > > emitted when painting an Image element. This adds similar events for > > images that are painted as backgrounds and borders. > > > > BUG=434905 > > > > Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=186030 > > TBR=chrishtr@chromium.org,yurys@chromium.org > NOTREECHECKS=true > NOTRY=true > BUG=434905 > > Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=186218 TBR=chrishtr@chromium.org,yurys@chromium.org NOTREECHECKS=true NOTRY=true BUG=434905 Review URL: https://codereview.chromium.org/768353002 git-svn-id: svn://svn.chromium.org/blink/trunk@186307 bbb929c8-8fbe-4397-9dbb-9b2b20218538
nwjs · Dec 2, 2014 · 71b85a0 · 71b85a0
1 parent 4140b55
commit 71b85a0
Show file tree

Hide file tree

Showing 8 changed files with 125 additions and 24 deletions.
diff --git a/LayoutTests/inspector/tracing/decode-resize-expected.txt b/LayoutTests/inspector/tracing/decode-resize-expected.txt
@@ -1,25 +1,66 @@
 Tests the instrumentation of a DecodeImage and ResizeImage events
 
-
+event: Decode Image
+imageURL: .../inspector/tracing/resources/big.png
+backendNodeId: present
+event: Decode Image
+imageURL: .../inspector/tracing/resources/big.png?background
+backendNodeId: present
+event: Decode Image
+imageURL: .../inspector/tracing/resources/big.png?border
+backendNodeId: present
 event: Decode Image
 imageURL: .../inspector/tracing/resources/test.bmp
 backendNodeId: present
 event: Decode Image
+imageURL: .../inspector/tracing/resources/test.bmp?background
+backendNodeId: present
+event: Decode Image
+imageURL: .../inspector/tracing/resources/test.bmp?border
+backendNodeId: present
+event: Decode Image
 imageURL: .../inspector/tracing/resources/test.gif
 backendNodeId: present
 event: Decode Image
+imageURL: .../inspector/tracing/resources/test.gif?background
+backendNodeId: present
+event: Decode Image
+imageURL: .../inspector/tracing/resources/test.gif?border
+backendNodeId: present
+event: Decode Image
 imageURL: .../inspector/tracing/resources/test.ico
 backendNodeId: present
 event: Decode Image
+imageURL: .../inspector/tracing/resources/test.ico?background
+backendNodeId: present
+event: Decode Image
+imageURL: .../inspector/tracing/resources/test.ico?border
+backendNodeId: present
+event: Decode Image
 imageURL: .../inspector/tracing/resources/test.jpg
 backendNodeId: present
 event: Decode Image
+imageURL: .../inspector/tracing/resources/test.jpg?background
+backendNodeId: present
+event: Decode Image
+imageURL: .../inspector/tracing/resources/test.jpg?border
+backendNodeId: present
+event: Decode Image
 imageURL: .../inspector/tracing/resources/test.png
 backendNodeId: present
 event: Decode Image
+imageURL: .../inspector/tracing/resources/test.png?background
+backendNodeId: present
+event: Decode Image
+imageURL: .../inspector/tracing/resources/test.png?border
+backendNodeId: present
+event: Decode Image
 imageURL: .../inspector/tracing/resources/test.webp
 backendNodeId: present
 event: Decode Image
-imageURL: .../inspector/tracing/resources/big.png
+imageURL: .../inspector/tracing/resources/test.webp?background
+backendNodeId: present
+event: Decode Image
+imageURL: .../inspector/tracing/resources/test.webp?border
 backendNodeId: present
 
diff --git a/LayoutTests/inspector/tracing/decode-resize.html b/LayoutTests/inspector/tracing/decode-resize.html
@@ -1,69 +1,102 @@
 <html>
 <head>
 <style>
-div#img-container {
+div {
+    display: inline-block;
+}
+
+div.img-container {
     position: relative;
     width: 99px;
     height: 99px;
     overflow: clip;
 }
+
+.background, .border {
+    width: 25px;
+    height: 25px;
+};
+
+.border {
+    border-width: 12px;
+}
+
 </style>
 <script src="../../http/tests/inspector/inspector-test.js"></script>
 <script src="../../http/tests/inspector/timeline-test.js"></script>
 <script src="../tracing-test.js"></script>
 <script>
 
 var images = [
+    ["./resources/test.webp", "25", "25"],
     ["./resources/test.bmp", "25", "25"],
     ["./resources/test.gif", "25", "25"],
     ["./resources/test.ico", "25", "25"],
     ["./resources/test.jpg", "25", "25"],
     ["./resources/test.png", "25", "25"],
-    ["./resources/test.webp", "25", "25"],
     ["./resources/big.png", "150", "150"]
 ];
 
 function showImages(callback)
 {
     var nextImageIndex = 0;
-    var imgElement = document.getElementById("img-container").firstElementChild;
 
-    imgElement.addEventListener("load", imageLoaded);
     addImages();
 
     function addImages()
     {
         if (nextImageIndex >= images.length) {
-            imgElement.removeEventListener("load", imageLoaded);
-            callback();
+            testRunner.displayAsyncThen(callback);
             return;
         }
 
-        var image = images[nextImageIndex++];
+        var imgContainer = document.createElement("div");
+        imgContainer.className = "img-container";
+        document.body.appendChild(imgContainer);
+
+        var imgElement = document.createElement("img");
+        imgElement.addEventListener("load", testRunner.displayAsyncThen.bind(testRunner, addImages));
+        imgContainer.appendChild(imgElement);
 
+        var backgroundElement = document.createElement("div");
+        backgroundElement.className = "background";
+        document.body.appendChild(backgroundElement);
+
+        var borderElement = document.createElement("div");
+        borderElement.className = "border";
+        document.body.appendChild(borderElement);
+
+        var image = images[nextImageIndex++];
         imgElement.width = image[1];
         imgElement.height = image[2];
         imgElement.src = image[0];
-    }
-
-    function imageLoaded()
-    {
-        testRunner.displayAsyncThen(addImages);
+        backgroundElement.style.backgroundImage = "url(" + image[0] + "?background)";
+        borderElement.style.borderImage = "url(" + image[0] + "?border)";
     }
 }
 
 
 function test()
 {
-    var imageCount = 0;
-
-    InspectorTest.invokeWithTracing("showImages", onTracingComplete);
+    InspectorTest.invokeWithTracing("showImages", InspectorTest.safeWrap(onTracingComplete));
     function onTracingComplete()
     {
+        function isDecodeImageEvent(event)
+        {
+            return event.name === WebInspector.TimelineModel.RecordType.DecodeImage;
+        }
+        function compareImageURLs(a, b)
+        {
+            var urlA = InspectorTest.formatters.formatAsURL(a.imageURL || "<missing>");
+            var urlB = InspectorTest.formatters.formatAsURL(b.imageURL || "<missing>");
+            return urlA.localeCompare(urlB);
+        }
         var events = InspectorTest.tracingTimelineModel().inspectedTargetEvents();
-        for (var i = 0; i < events.length; ++i) {
-            var event = events[i];
-            if (events[i].name !== WebInspector.TimelineModel.RecordType.DecodeImage)
+        var sortedDecodeEvents = events.filter(isDecodeImageEvent).sort(compareImageURLs);
+        for (var i = 0; i < sortedDecodeEvents.length; ++i) {
+            var event = sortedDecodeEvents[i];
+            // Skip duplicate events, as long as they have the imageURL
+            if (i && event.imageURL && event.imageURL === sortedDecodeEvents[i - 1].imageURL)
                 continue;
             InspectorTest.addResult("event: " + event.name);
             InspectorTest.addResult("imageURL: " + InspectorTest.formatters.formatAsURL(event.imageURL));
@@ -80,6 +113,5 @@
 <p>
 Tests the instrumentation of a DecodeImage and ResizeImage events
 </p>
-<div id="img-container"><img></div>
 </body>
 </html>
diff --git a/Source/core/fetch/ImageResource.cpp b/Source/core/fetch/ImageResource.cpp
@@ -492,8 +492,10 @@ void ImageResource::changedInRect(const blink::Image* image, const IntRect& rect
 bool ImageResource::currentFrameKnownToBeOpaque(const RenderObject* renderer)
 {
     blink::Image* image = imageForRenderer(renderer);
-    if (image->isBitmapImage())
+    if (image->isBitmapImage()) {
+        TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("devtools.timeline"), "PaintImage", "data", InspectorPaintImageEvent::data(renderer, *this));
         image->nativeImageForCurrentFrame(); // force decode
+    }
     return image->currentFrameKnownToBeOpaque();
 }
 

diff --git a/Source/core/fetch/ImageResource.h b/Source/core/fetch/ImageResource.h
@@ -58,7 +58,10 @@ class ImageResource final : public Resource, public ImageObserver {
     blink::Image* image(); // Returns the nullImage() if the image is not available yet.
     blink::Image* imageForRenderer(const RenderObject*); // Returns the nullImage() if the image is not available yet.
     bool hasImage() const { return m_image.get(); }
-    bool currentFrameKnownToBeOpaque(const RenderObject*); // Side effect: ensures decoded image is in cache, therefore should only be called when about to draw the image.
+    // Side effect: ensures decoded image is in cache, therefore should only be called when about to draw the image.
+    // FIXME: Decoding image on the main thread is expensive, so rather than forcing decode, consider returning false
+    // when image is not decoded yet, as we do in case of deferred decoding.
+    bool currentFrameKnownToBeOpaque(const RenderObject*);
 
     static std::pair<blink::Image*, float> brokenImage(float deviceScaleFactor); // Returns an image and the image's resolution scale factor.
     bool willPaintBrokenImage() const;

diff --git a/Source/core/inspector/InspectorTraceEvents.cpp b/Source/core/inspector/InspectorTraceEvents.cpp
@@ -599,6 +599,23 @@ PassRefPtr<TraceEvent::ConvertableToTraceFormat> InspectorPaintImageEvent::data(
     return value.release();
 }
 
+PassRefPtr<TraceEvent::ConvertableToTraceFormat> InspectorPaintImageEvent::data(const RenderObject& owningRenderer, const StyleImage& styleImage)
+{
+    RefPtr<TracedValue> value = TracedValue::create();
+    setGeneratingNodeInfo(value.get(), &owningRenderer, "nodeId");
+    if (const ImageResource* resource = styleImage.cachedImage())
+        value->setString("url", resource->url().string());
+    return value.release();
+}
+
+PassRefPtr<TraceEvent::ConvertableToTraceFormat> InspectorPaintImageEvent::data(const RenderObject* owningRenderer, const ImageResource& imageResource)
+{
+    RefPtr<TracedValue> value = TracedValue::create();
+    setGeneratingNodeInfo(value.get(), owningRenderer, "nodeId");
+    value->setString("url", imageResource.url().string());
+    return value.release();
+}
+
 static size_t usedHeapSize()
 {
     HeapInfo info;

diff --git a/Source/core/inspector/InspectorTraceEvents.h b/Source/core/inspector/InspectorTraceEvents.h
@@ -22,6 +22,7 @@ class Event;
 class ExecutionContext;
 class FrameView;
 class GraphicsLayer;
+class ImageResource;
 class KURL;
 class LayoutRect;
 class LocalFrame;
@@ -33,6 +34,7 @@ class RenderObject;
 class ResourceRequest;
 class ResourceResponse;
 class StyleChangeReasonForTracing;
+class StyleImage;
 class TracedValue;
 class WorkerThread;
 class XMLHttpRequest;
@@ -210,6 +212,8 @@ class InspectorPaintEvent {
 class InspectorPaintImageEvent {
 public:
     static PassRefPtr<TraceEvent::ConvertableToTraceFormat> data(const RenderImage&);
+    static PassRefPtr<TraceEvent::ConvertableToTraceFormat> data(const RenderObject&, const StyleImage&);
+    static PassRefPtr<TraceEvent::ConvertableToTraceFormat> data(const RenderObject*, const ImageResource&);
 };
 
 class InspectorCommitLoadEvent {

diff --git a/Source/core/paint/BoxPainter.cpp b/Source/core/paint/BoxPainter.cpp
@@ -478,6 +478,7 @@ void BoxPainter::paintFillLayerExtended(RenderBoxModelObject& obj, const PaintIn
                 context->setColorFilter(ColorFilterLuminanceToAlpha);
             InterpolationQuality previousInterpolationQuality = context->imageInterpolationQuality();
             context->setImageInterpolationQuality(interpolationQuality);
+            TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("devtools.timeline"), "PaintImage", "data", InspectorPaintImageEvent::data(obj, *bgImage));
             context->drawTiledImage(image.get(), geometry.destRect(), geometry.phase(), geometry.tileSize(),
                 compositeOp, bgLayer.blendMode(), geometry.spaceSize());
             context->setImageInterpolationQuality(previousInterpolationQuality);
@@ -971,6 +972,7 @@ bool BoxPainter::paintNinePieceImage(RenderBoxModelObject& obj, GraphicsContext*
     float topSideScale = drawTop ? (float)topWidth / topSlice : 1;
     float bottomSideScale = drawBottom ? (float)bottomWidth / bottomSlice : 1;
 
+    TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("devtools.timeline"), "PaintImage", "data", InspectorPaintImageEvent::data(obj, *styleImage));
     if (drawLeft) {
         // Paint the top and bottom left corners.
 

diff --git a/Source/devtools/front_end/timeline/TimelineUIUtils.js b/Source/devtools/front_end/timeline/TimelineUIUtils.js
@@ -566,7 +566,7 @@ WebInspector.TimelineUIUtils._buildTraceEventDetailsSynchronously = function(eve
     case recordTypes.DecodeImage:
     case recordTypes.ResizeImage:
     case recordTypes.DrawLazyPixelRef:
-        relatedNodeLabel = WebInspector.UIString("Image element");
+        relatedNodeLabel = WebInspector.UIString("Owner element");
         if (event.imageURL)
             contentHelper.appendElementRow(WebInspector.UIString("Image URL"), WebInspector.linkifyResourceAsNode(event.imageURL));
         break;