Commit 065117e4 authored by Nicolás Peña Moreno's avatar Nicolás Peña Moreno Committed by Commit Bot

[ElementTiming] Add a test to prevent leak in ImageElementTiming

Currently we need to notify ImageElementTiming whenever an element will
be destroyed, so that it can be removed from the HashSet of images that
have been seen by ImageElementTiming. This CL adds unittests for this
behavior.

Bug: 879270
Change-Id: I7719b9c2bfc594cbf4070b1fe66c9e5325a3a552
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1506660Reviewed-by: default avatarSteve Kobes <skobes@chromium.org>
Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#638649}
parent 5277ec60
...@@ -45,7 +45,7 @@ class CORE_EXPORT ImageElementTiming final ...@@ -45,7 +45,7 @@ class CORE_EXPORT ImageElementTiming final
void Trace(blink::Visitor*) override; void Trace(blink::Visitor*) override;
private: private:
FRIEND_TEST_ALL_PREFIXES(ImageElementTimingTest, ImageInsideSVG); friend class ImageElementTimingTest;
// Callback for the swap promise. Reports paint timestamps. // Callback for the swap promise. Reports paint timestamps.
void ReportImagePaintSwapTime(WebLayerTreeView::SwapResult, void ReportImagePaintSwapTime(WebLayerTreeView::SwapResult,
base::TimeTicks timestamp); base::TimeTicks timestamp);
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#include "third_party/blink/renderer/core/paint/image_element_timing.h" #include "third_party/blink/renderer/core/paint/image_element_timing.h"
#include "third_party/blink/renderer/core/layout/layout_image.h" #include "third_party/blink/renderer/core/layout/layout_image.h"
#include "third_party/blink/renderer/core/layout/svg/layout_svg_image.h"
#include "third_party/blink/renderer/core/loader/resource/image_resource_content.h" #include "third_party/blink/renderer/core/loader/resource/image_resource_content.h"
#include "third_party/blink/renderer/core/testing/core_unit_test_helper.h" #include "third_party/blink/renderer/core/testing/core_unit_test_helper.h"
#include "third_party/blink/renderer/platform/graphics/static_bitmap_image.h" #include "third_party/blink/renderer/platform/graphics/static_bitmap_image.h"
...@@ -24,8 +25,17 @@ class ImageElementTimingTest : public RenderingTest { ...@@ -24,8 +25,17 @@ class ImageElementTimingTest : public RenderingTest {
return layout_image; return layout_image;
} }
const ImageElementTiming& GetImageElementTiming() { // Similar to above but for a LayoutSVGImage.
return ImageElementTiming::From(*GetDocument().domWindow()); LayoutSVGImage* SetSVGImageResource(const char* id, int width, int height) {
ImageResourceContent* content = CreateImageForTest(width, height);
auto* layout_image = ToLayoutSVGImage(GetLayoutObjectByElementId(id));
layout_image->ImageResource()->SetImageResource(content);
return layout_image;
}
const WTF::HashSet<const LayoutObject*>& GetImagesNotified() {
return ImageElementTiming::From(*GetDocument().domWindow())
.images_notified_;
} }
private: private:
...@@ -59,9 +69,44 @@ TEST_F(ImageElementTimingTest, ImageInsideSVG) { ...@@ -59,9 +69,44 @@ TEST_F(ImageElementTimingTest, ImageInsideSVG) {
// Enable compositing and also update document lifecycle. // Enable compositing and also update document lifecycle.
EnableCompositing(); EnableCompositing();
const ImageElementTiming& timing = GetImageElementTiming();
// |layout_image| should have had its paint notified to ImageElementTiming. // |layout_image| should have had its paint notified to ImageElementTiming.
EXPECT_TRUE(timing.images_notified_.Contains(layout_image)); EXPECT_TRUE(GetImagesNotified().Contains(layout_image));
}
TEST_F(ImageElementTimingTest, ImageRemoved) {
EnableCompositing();
GetDocument().SetBaseURLOverride(KURL("http://test.com"));
SetBodyInnerHTML(R"HTML(
<img id="target" style='width: 100px; height: 100px;'/>
)HTML");
LayoutImage* layout_image = SetImageResource("target", 5, 5);
ASSERT_TRUE(layout_image);
UpdateAllLifecyclePhasesForTest();
EXPECT_TRUE(GetImagesNotified().Contains(layout_image));
GetDocument().getElementById("target")->remove();
// |layout_image| should no longer be part of |images_notified| since it will
// be destroyed.
EXPECT_TRUE(GetImagesNotified().IsEmpty());
}
TEST_F(ImageElementTimingTest, SVGImageRemoved) {
EnableCompositing();
GetDocument().SetBaseURLOverride(KURL("http://test.com"));
SetBodyInnerHTML(R"HTML(
<svg>
<image id="target" style='width: 100px; height: 100px;'/>
</svg>
)HTML");
LayoutSVGImage* layout_image = SetSVGImageResource("target", 5, 5);
ASSERT_TRUE(layout_image);
UpdateAllLifecyclePhasesForTest();
EXPECT_TRUE(GetImagesNotified().Contains(layout_image));
GetDocument().getElementById("target")->remove();
// |layout_image| should no longer be part of |images_notified| since it will
// be destroyed.
EXPECT_TRUE(GetImagesNotified().IsEmpty());
} }
} // namespace blink } // namespace blink
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment