Commit dc0c2db8 authored by Khushal's avatar Khushal Committed by Commit Bot

blink/canvas: Ensure ImageDecodeCache not used on context lost.

The SharedGpuContext is re-created on a context lost, which means any
user of the context's cache must ensure that its usage does not extend
the validity of the context. Ensure this by resetting the ImageProvider
on a context loss.

R=chrishtr@chromium.org, junov@chromium.org

Bug: 806629
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I33891c0a45f59de1741470d408dfa81184dc153a
Reviewed-on: https://chromium-review.googlesource.com/891982Reviewed-by: default avatardanakj <danakj@chromium.org>
Reviewed-by: default avatarAntoine Labour <piman@chromium.org>
Reviewed-by: default avatarChris Harrelson <chrishtr@chromium.org>
Commit-Queue: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534102}
parent 635e85a9
......@@ -37,6 +37,8 @@ class CC_PAINT_EXPORT SkiaPaintCanvas final : public PaintCanvas {
ImageProvider* image_provider = nullptr);
~SkiaPaintCanvas() override;
void reset_image_provider() { image_provider_ = nullptr; }
SkMetaData& getMetaData() override;
SkImageInfo imageInfo() const override;
......
......@@ -1044,6 +1044,7 @@ jumbo_component("platform") {
"graphics/VideoFrameResourceProvider.h",
"graphics/VideoFrameSubmitter.cpp",
"graphics/VideoFrameSubmitter.h",
"graphics/WebGraphicsContext3DProviderWrapper.cpp",
"graphics/WebGraphicsContext3DProviderWrapper.h",
"graphics/compositing/CompositedLayerRasterInvalidator.cpp",
"graphics/compositing/CompositedLayerRasterInvalidator.h",
......
......@@ -12,6 +12,7 @@ include_rules = [
"+base/location.h",
"+base/logging.h",
"+base/memory",
"+base/observer_list.h",
"+base/message_loop/message_loop.h",
"+base/metrics/histogram.h",
"+base/metrics/histogram_base.h",
......
......@@ -135,6 +135,8 @@ class ImageTrackingDecodeCache : public cc::StubDecodeCache {
cc::DecodedDrawImage GetDecodedImageForDraw(
const cc::DrawImage& image) override {
EXPECT_FALSE(disallow_cache_use_);
num_locked_images_++;
decoded_images_.push_back(image);
SkBitmap bitmap;
......@@ -147,10 +149,12 @@ class ImageTrackingDecodeCache : public cc::StubDecodeCache {
}
void set_budget_exceeded(bool exceeded) { budget_exceeded_ = exceeded; }
void set_disallow_cache_use(bool disallow) { disallow_cache_use_ = disallow; }
void DrawWithImageFinished(
const cc::DrawImage& image,
const cc::DecodedDrawImage& decoded_image) override {
EXPECT_FALSE(disallow_cache_use_);
num_locked_images_--;
}
......@@ -163,6 +167,7 @@ class ImageTrackingDecodeCache : public cc::StubDecodeCache {
std::vector<cc::DrawImage> decoded_images_;
int num_locked_images_ = 0;
bool budget_exceeded_ = false;
bool disallow_cache_use_ = false;
};
} // anonymous namespace
......@@ -179,7 +184,7 @@ class Canvas2DLayerBridgeTest : public Test {
return bridge;
}
void SetUp() override {
auto factory = [](FakeGLES2Interface* gl, cc::ImageDecodeCache* cache,
auto factory = [](FakeGLES2Interface* gl, ImageTrackingDecodeCache* cache,
bool* gpu_compositing_disabled)
-> std::unique_ptr<WebGraphicsContext3DProvider> {
*gpu_compositing_disabled = false;
......@@ -1369,4 +1374,31 @@ TEST_F(Canvas2DLayerBridgeTest, ImagesLockedUntilCacheLimit) {
EXPECT_EQ(image_decode_cache_.num_locked_images(), 0);
}
TEST_F(Canvas2DLayerBridgeTest, ImageCacheOnContextLost) {
// Disable deferral so we use the raster canvas directly.
auto color_params =
CanvasColorParams(kSRGBCanvasColorSpace, kF16CanvasPixelFormat, kOpaque);
Canvas2DLayerBridgePtr bridge(WTF::WrapUnique(new Canvas2DLayerBridge(
IntSize(300, 300), 0, Canvas2DLayerBridge::kEnableAcceleration,
color_params)));
bridge->DisableDeferral(DisableDeferralReason::kDisableDeferralReasonUnknown);
cc::PaintFlags flags;
std::vector<cc::DrawImage> images = {
cc::DrawImage(cc::CreateDiscardablePaintImage(gfx::Size(10, 10)),
SkIRect::MakeWH(10, 10), kNone_SkFilterQuality,
SkMatrix::I(), 0u, color_params.GetStorageGfxColorSpace()),
cc::DrawImage(cc::CreateDiscardablePaintImage(gfx::Size(20, 20)),
SkIRect::MakeWH(5, 5), kNone_SkFilterQuality, SkMatrix::I(),
0u, color_params.GetStorageGfxColorSpace())};
bridge->Canvas()->drawImage(images[0].paint_image(), 0u, 0u, nullptr);
// Lose the context and ensure that the image provider is not used.
bridge->GetResourceProvider()->OnContextDestroyed();
// We should unref all images on the cache when the context is destroyed.
EXPECT_EQ(image_decode_cache_.num_locked_images(), 0);
image_decode_cache_.set_disallow_cache_use(true);
bridge->Canvas()->drawImage(images[1].paint_image(), 0u, 0u, &flags);
}
} // namespace blink
......@@ -347,9 +347,15 @@ CanvasResourceProvider::CanvasResourceProvider(
: weak_ptr_factory_(this),
context_provider_wrapper_(std::move(context_provider_wrapper)),
size_(size),
color_params_(color_params) {}
color_params_(color_params) {
if (context_provider_wrapper_)
context_provider_wrapper_->AddObserver(this);
}
CanvasResourceProvider::~CanvasResourceProvider() = default;
CanvasResourceProvider::~CanvasResourceProvider() {
if (context_provider_wrapper_)
context_provider_wrapper_->RemoveObserver(this);
}
SkSurface* CanvasResourceProvider::GetSkSurface() const {
if (!surface_) {
......@@ -378,9 +384,18 @@ PaintCanvas* CanvasResourceProvider::Canvas() {
GetSkSurface()->getCanvas(), std::move(image_provider));
}
}
return canvas_.get();
}
void CanvasResourceProvider::OnContextDestroyed() {
if (canvas_image_provider_) {
DCHECK(canvas_);
canvas_->reset_image_provider();
canvas_image_provider_.reset();
}
}
void CanvasResourceProvider::ReleaseLockedImages() {
if (canvas_image_provider_)
canvas_image_provider_->ReleaseLockedImages();
......
......@@ -7,10 +7,11 @@
#include "base/memory/scoped_refptr.h"
#include "base/memory/weak_ptr.h"
#include "cc/paint/decode_stashing_image_provider.h"
#include "cc/paint/skia_paint_canvas.h"
#include "cc/raster/playback_image_provider.h"
#include "platform/geometry/IntSize.h"
#include "platform/graphics/CanvasColorParams.h"
#include "platform/graphics/WebGraphicsContext3DProviderWrapper.h"
#include "platform/wtf/Noncopyable.h"
#include "platform/wtf/Optional.h"
#include "platform/wtf/RefCounted.h"
......@@ -55,7 +56,8 @@ class WebGraphicsContext3DProviderWrapper;
// 2) use Canvas() to get a drawing interface
// 3) Call Snapshot() to acquire a bitmap with the rendered image in it.
class PLATFORM_EXPORT CanvasResourceProvider {
class PLATFORM_EXPORT CanvasResourceProvider
: public WebGraphicsContext3DProviderWrapper::DestructionObserver {
WTF_MAKE_NONCOPYABLE(CanvasResourceProvider);
public:
......@@ -79,6 +81,9 @@ class PLATFORM_EXPORT CanvasResourceProvider {
virtual scoped_refptr<CanvasResource> ProduceFrame() = 0;
scoped_refptr<StaticBitmapImage> Snapshot();
// WebGraphicsContext3DProvider::DestructionObserver implementation.
void OnContextDestroyed() override;
cc::PaintCanvas* Canvas();
void ReleaseLockedImages();
void FlushSkia() const;
......@@ -103,7 +108,7 @@ class PLATFORM_EXPORT CanvasResourceProvider {
return 0;
}
void Clear();
virtual ~CanvasResourceProvider();
~CanvasResourceProvider() override;
protected:
gpu::gles2::GLES2Interface* ContextGL() const;
......@@ -153,7 +158,7 @@ class PLATFORM_EXPORT CanvasResourceProvider {
IntSize size_;
CanvasColorParams color_params_;
Optional<CanvasImageProvider> canvas_image_provider_;
std::unique_ptr<cc::PaintCanvas> canvas_;
std::unique_ptr<cc::SkiaPaintCanvas> canvas_;
mutable sk_sp<SkSurface> surface_; // mutable for lazy init
std::unique_ptr<SkCanvas> xform_canvas_;
WTF::Vector<scoped_refptr<CanvasResource>> recycled_resources_;
......
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "platform/graphics/WebGraphicsContext3DProviderWrapper.h"
namespace blink {
WebGraphicsContext3DProviderWrapper::~WebGraphicsContext3DProviderWrapper() {
for (auto& observer : observers_)
observer.OnContextDestroyed();
}
void WebGraphicsContext3DProviderWrapper::AddObserver(
DestructionObserver* obs) {
observers_.AddObserver(obs);
}
void WebGraphicsContext3DProviderWrapper::RemoveObserver(
DestructionObserver* obs) {
observers_.RemoveObserver(obs);
}
} // namespace blink
......@@ -6,6 +6,7 @@
#define WebGraphicsContext3DProviderWrapper_h
#include "base/memory/weak_ptr.h"
#include "base/observer_list.h"
#include "platform/PlatformExport.h"
#include "platform/graphics/gpu/GraphicsContext3DUtils.h"
#include "public/platform/WebGraphicsContext3DProvider.h"
......@@ -14,12 +15,20 @@ namespace blink {
class PLATFORM_EXPORT WebGraphicsContext3DProviderWrapper {
public:
class DestructionObserver {
public:
virtual ~DestructionObserver() {}
virtual void OnContextDestroyed() = 0;
};
WebGraphicsContext3DProviderWrapper(
std::unique_ptr<WebGraphicsContext3DProvider> provider)
: context_provider_(std::move(provider)), weak_ptr_factory_(this) {
DCHECK(context_provider_);
utils_ = WTF::WrapUnique(new GraphicsContext3DUtils(GetWeakPtr()));
}
~WebGraphicsContext3DProviderWrapper();
base::WeakPtr<WebGraphicsContext3DProviderWrapper> GetWeakPtr() {
return weak_ptr_factory_.GetWeakPtr();
}
......@@ -29,9 +38,13 @@ class PLATFORM_EXPORT WebGraphicsContext3DProviderWrapper {
GraphicsContext3DUtils* Utils() { return utils_.get(); }
void AddObserver(DestructionObserver*);
void RemoveObserver(DestructionObserver*);
private:
std::unique_ptr<GraphicsContext3DUtils> utils_;
std::unique_ptr<WebGraphicsContext3DProvider> context_provider_;
base::ObserverList<DestructionObserver> observers_;
base::WeakPtrFactory<WebGraphicsContext3DProviderWrapper> weak_ptr_factory_;
};
......
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