Prevent recreation of UI resource holder when created with resource ID

cc/layers/ui_resource_layer.cc

 // Copyright 2013 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/layers/ui_resource_layer.h"
 
 #include "cc/layers/ui_resource_layer_impl.h"
 #include "cc/resources/prioritized_resource.h"
 #include "cc/resources/resource_update.h"
 #include "cc/resources/resource_update_queue.h"
 #include "cc/resources/scoped_ui_resource.h"
 #include "cc/resources/ui_resource_bitmap.h"
 #include "cc/trees/layer_tree_host.h"
 
 namespace cc {
 
 
 namespace {
 
 class ScopedUIResourceHolder : public UIResourceLayer::UIResourceHolder {
  public:
   static scoped_ptr<ScopedUIResourceHolder> Create(LayerTreeHost* host,
                                             const SkBitmap& skbitmap) {
     return make_scoped_ptr(new ScopedUIResourceHolder(host, skbitmap));
   }
   UIResourceId id() override { return resource_->id(); }
 
+  bool NeedsRecreation() override { return false; }
+
  private:
   ScopedUIResourceHolder(LayerTreeHost* host, const SkBitmap& skbitmap) {
     resource_ = ScopedUIResource::Create(host, UIResourceBitmap(skbitmap));
   }
 
   scoped_ptr<ScopedUIResource> resource_;
 };
 
 class SharedUIResourceHolder : public UIResourceLayer::UIResourceHolder {
  public:
   static scoped_ptr<SharedUIResourceHolder> Create(UIResourceId id) {
     return make_scoped_ptr(new SharedUIResourceHolder(id));
   }
 
   UIResourceId id() override { return id_; }
 
+  bool NeedsRecreation() override { return true; }
+
  private:
   explicit SharedUIResourceHolder(UIResourceId id) : id_(id) {}
 
   UIResourceId id_;
 };
 
 }  // anonymous namespace
 
 UIResourceLayer::UIResourceHolder::~UIResourceHolder() {}
 
@@ skipping 51 matching lines @@
   if (host == layer_tree_host())
     return;
 
   Layer::SetLayerTreeHost(host);
 
   // Recreate the resource hold against the new LTH.
   RecreateUIResourceHolder();
 }
 
 void UIResourceLayer::RecreateUIResourceHolder() {
-  ui_resource_holder_ = nullptr;
   if (layer_tree_host() && !bitmap_.empty()) {
     ui_resource_holder_ =
         ScopedUIResourceHolder::Create(layer_tree_host(), bitmap_);
+  } else if (ui_resource_holder_ && !ui_resource_holder_->NeedsRecreation()) {

[danakj, 2014/11/20 16:38:08]

I think we could possibly make this simpler to follow. What if we move the
clearing of ui_resource_holder_ to the Set* methods?

If you have a bitmap in SetBitmap, then you'll make a new ScopedUIResourceHolder
here. If you don't have a bitmap in SetBitmap, then you'll have nothing to clear
here.

Likewise, if you have a resource_id in SetUIResourceID then you don't want to
clear it here, and if you didn't have a resource_id, then there's nothing to
destroy here.

Do you even need the NeedsRecreation bit then?

[Changwan Ryu, 2014/11/20 20:52:26]

I'm not sure I'm following you correctly, but I'm concerned about
SetLayerTreeHost() logic then.

When layer tree host changes, we set empty resource holder only when
1. layer tree host is invalid or
2. bitmap is empty but previously it was scoped resource holder.

otherwise we will create a new scoped resource holder.

But it seems that the condition #2 cannot be detected without NeedsRecreation
bit.

Could you explain a bit more about SetLayerTreeHost workflow?

[danakj, 2014/11/20 20:56:04]

You only have a bitmap_ != null if you called SetBitmap and have a
ScopedUIResource as a result. In all other cases you either don't want to clear
the ui_resource_holder_, or it is already empty, I think?

[Changwan Ryu, 2014/11/20 21:29:40]

How about now?

+    ui_resource_holder_ = nullptr;
   }
   UpdateDrawsContent(HasDrawableContent());
 }
 
 void UIResourceLayer::SetBitmap(const SkBitmap& skbitmap) {
   bitmap_ = skbitmap;
 
   RecreateUIResourceHolder();
   SetNeedsCommit();
 }
 
 void UIResourceLayer::SetUIResourceId(UIResourceId resource_id) {
   if (ui_resource_holder_ && ui_resource_holder_->id() == resource_id)
     return;
 
   if (resource_id)
     ui_resource_holder_ = SharedUIResourceHolder::Create(resource_id);
   else
     ui_resource_holder_ = nullptr;
 
   UpdateDrawsContent(HasDrawableContent());
   SetNeedsCommit();
 }
 
+UIResourceId UIResourceLayer::GetUIResourceId() {
+  if (ui_resource_holder_)
+    return ui_resource_holder_->id();
+  return 0;
+}
+
 bool UIResourceLayer::HasDrawableContent() const {
   return ui_resource_holder_ && ui_resource_holder_->id() &&
          Layer::HasDrawableContent();
 }
 
 void UIResourceLayer::PushPropertiesTo(LayerImpl* layer) {
   Layer::PushPropertiesTo(layer);
   UIResourceLayerImpl* layer_impl = static_cast<UIResourceLayerImpl*>(layer);
 
   if (!ui_resource_holder_) {
     layer_impl->SetUIResourceId(0);
   } else {
     DCHECK(layer_tree_host());
 
     gfx::Size image_size =
         layer_tree_host()->GetUIResourceSize(ui_resource_holder_->id());
     layer_impl->SetUIResourceId(ui_resource_holder_->id());
     layer_impl->SetImageBounds(image_size);
     layer_impl->SetUV(uv_top_left_, uv_bottom_right_);
     layer_impl->SetVertexOpacity(vertex_opacity_);
   }
 }
 
 }  // namespace cc