Implement the logic to kick off image decoding jobs for TileManager

cc/image.cc

Index: cc/image.cc
diff --git a/cc/image.cc b/cc/image.cc
new file mode 100644
index 0000000000000000000000000000000000000000..306ba489559acdba0e24cd56cfcfa8a8d0921d9d
--- /dev/null
+++ b/cc/image.cc
@@ -0,0 +1,31 @@
+// Copyright 2012 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 "cc/image.h"
+
+#include "skia/ext/lazy_pixel_ref.h"
+
+namespace cc {
+
+scoped_refptr<Image> Image::Create(SkPixelRef* pixel_ref) {
+  return make_scoped_refptr(new Image(skia::AdoptRef(pixel_ref)));
+}
+
+Image::Image(const skia::RefPtr<SkPixelRef>& pixel_ref) :

[Alpha Left Google, 2012/12/05 20:24:28]

I don't really know skia::RefPtr but it looks strange that a RefPtr needs
const&.

[qinmin, 2012/12/07 05:06:28]

Somehow this is commonly used in cc/, I am not sure why this is on purpose or
just convention.

On 2012/12/05 20:24:28, Alpha wrote:

+    pixel_ref_(pixel_ref) {
+}
+
+Image::~Image() {}
+
+void Image::Decode() {
+  static_cast<skia::LazyPixelRef*>(pixel_ref_.get())->Decode();

[Alpha Left Google, 2012/12/05 20:24:28]

Nat can give more comments about this interface.

But what I think is that Decode() is an intermediate step. Right now Decode() is
implemented to decode and then release resource. So you're not guaranteed that
after Decode() the image will stay there. It's temporary in the sense that we're
sloppy now but we'll eventually do:

PrepareToDecode();
DispatchDecodeJobs();
DispatchRasterJob();
ReleaseDecodeResources();

This way we don't get random thrashing and redecoding. If we see this as an
intermediate step then it's fine for me as long as there's comments for what we
want to do later, or a bug filed to document this.

[nduca, 2012/12/06 08:46:53]

I think we should make redecoding work. E.g. this should work:
DispatchOneTile
  image1.PrepareToDecode()->False
  DispatchDecodeTask(image1)

DecodeTask(image1)

TileMgr::DidDecode(image1)
  DispatchTileThatUsesImage1

ImageDecodingStore::DiscardCacheFor(image1)

RasterizeImage1Tile
  drawpicture
    internally locks picture and discovers no cache entry
      redecodes inline

We can track how many inline redecodes we get, and see if we get it often. Iff
and only iff we get them, we can discuss mitigation. IMO, we can avoid (possibly
completely) in a lot of cases by opting prioritizing pending tiles over new
decoding work in the dispatching logic. Similarly, we could also track on the
dispatch side whether kicking another decode would cause an evict, and if so,
handle it. This is a more heuristic way to deal with the problem you're worrying
about Alpha, but it should be considerably simpler to maintain and less prone
livelocks.

[qinmin, 2012/12/07 05:06:28]

So we need a freeCachedPixelRef() in lazy_pixel_ref.h to allow the image decoder
to purge the cache for a particular pixelref. But then we are facing the problem
that the cache could grow out of control.

On 2012/12/06 08:46:53, nduca wrote:

+}
+
+bool Image::PrepareToDecode() {
+  // TODO(qinmin): passing the correct clip rect to PrepareToDecode().
+  return static_cast<skia::LazyPixelRef*>(pixel_ref_.get())->PrepareToDecode(

[Alpha Left Google, 2012/12/05 20:24:28]

I think WebKit code for PrepareToDecode leaks resource but it least it won't
crash..

+      skia::LazyPixelRef::PrepareParams());
+}
+
+}  // namespace cc