[blimp] Add SkPicture caching support.

blimp/client/feature/compositor/blimp_client_picture_cache.cc

+// Copyright 2016 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 "blimp/client/feature/compositor/blimp_client_picture_cache.h"
+
+#include <stdint.h>
+#include <memory>
+#include <unordered_map>

[Kevin M, 2016/06/06 23:33:58]

already included in .h, ditto for stdint, memory

[nyquist, 2016/06/10 22:02:23]

Done.

+#include <utility>
+#include <vector>
+
+#include "base/logging.h"
+#include "blimp/common/compositor/blimp_picture_cache_registry.h"
+#include "cc/proto/picture_cache.h"
+#include "third_party/skia/include/core/SkPicture.h"
+#include "third_party/skia/include/core/SkRefCnt.h"
+#include "third_party/skia/include/core/SkStream.h"
+
+namespace blimp {
+namespace client {
+
+namespace {
+// Helper function to deserialize the content of |picture_data| into an
+// SkPicture.
+sk_sp<const SkPicture> DeserializePicture(
+    SkPicture::InstallPixelRefProc pixel_deserializer,
+    const cc::PictureData& picture_data) {
+  SkMemoryStream stream(picture_data.data);
+  return SkPicture::MakeFromStream(&stream, pixel_deserializer);
+}
+
+}  // namespace
+
+BlimpClientPictureCache::BlimpClientPictureCache(
+    SkPicture::InstallPixelRefProc pixel_deserializer)
+    : pixel_deserializer_(pixel_deserializer) {}
+
+BlimpClientPictureCache::~BlimpClientPictureCache() = default;
+
+sk_sp<const SkPicture> BlimpClientPictureCache::GetPicture(
+    uint32_t engine_picture_id) {
+  DCHECK(pictures_.find(engine_picture_id) != pictures_.end());
+  return pictures_[engine_picture_id];
+}
+
+void BlimpClientPictureCache::ApplyCacheUpdate(
+    const cc::PictureCacheUpdate& cache_update) {
+  for (const cc::PictureData& picture_data : cache_update) {
+    sk_sp<const SkPicture> deserialized_picture =
+        DeserializePicture(pixel_deserializer_, picture_data);
+    DCHECK(pictures_.find(picture_data.unique_id) == pictures_.end());

[Kevin M, 2016/06/06 23:33:58]

DCHECK_EQ(expected, actual)

[nyquist, 2016/06/10 22:02:24]

I'm not sure I like that for iterators. Are you certain you want this?

+
+    pictures_[picture_data.unique_id] = std::move(deserialized_picture);
+
+#if DCHECK_IS_ON()
+    last_added_.insert(picture_data.unique_id);
+#endif
+  }
+}
+
+void BlimpClientPictureCache::Flush() {
+  std::vector<uint32_t> added;
+  std::vector<uint32_t> removed;
+  registry_.Commit(&added, &removed);
+
+#if DCHECK_IS_ON()
+  // Verify that the incoming cache update matches the new items.
+  DCHECK_EQ(added.size(), last_added_.size());
+  DCHECK(std::unordered_set<uint32_t>(added.begin(), added.end()) ==
+         last_added_);
+  last_added_.clear();
+#endif
+
+  for (const auto& it : removed) {
+    auto entry = pictures_.find(it);
+    DCHECK(entry != pictures_.end());

[Kevin M, 2016/06/06 23:33:58]

DCHECK_NE

[Kevin M, 2016/06/06 23:33:58]

Is it possible that this could happen at release time, or is this just a sanity
check?

If it's the former, then can we handle this error?

[nyquist, 2016/06/10 22:02:24]

It's just a sanity check. If it happens it's a bug.

I'm not sure I like using the DCHECK_NE here, since the error message isn't
really helpful compared to just using the DCHECK

+    pictures_.erase(entry);
+  }
+}
+
+void BlimpClientPictureCache::MarkPictureForUnregistration(
+    uint32_t engine_picture_id) {
+  registry_.DecrementRefCount(engine_picture_id);
+}
+
+void BlimpClientPictureCache::MarkPictureForRegistration(
+    uint32_t engine_picture_id) {
+  registry_.IncrementRefCount(engine_picture_id);
+}
+
+}  // namespace client
+}  // namespace blimp