[cc] Evict UIResources when the renderer is not visible

cc/trees/layer_tree_host.h

 // Copyright 2011 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.
 
 #ifndef CC_TREES_LAYER_TREE_HOST_H_
 #define CC_TREES_LAYER_TREE_HOST_H_
 
 #include <limits>
 #include <list>
 #include <vector>
@@ skipping 72 matching lines @@
   bool avoid_pow2_textures;
   bool using_map_image;
   bool using_shared_memory_resources;
   bool using_discard_framebuffer;
 };
 
 struct CC_EXPORT UIResourceRequest {
   enum UIResourceRequestType {
     UIResourceCreate,
     UIResourceDelete,
+    UIResourceEvictionRecreated,

[aelias_OOO_until_Jul13, 2013/09/09 22:52:37]

Why is it necessary to track this in a different category than "Create"?  I'd
prefer to have the same enum value for both unless we have some compelling
reason.  The lost context case doesn't need a different value.

[ccameron, 2013/09/09 23:22:04]

I strongly prefer that this be done explicitly -- it is much easier for me trace
through the sequence mentally (see further elaboration below).

WRT using Create, consider the following situation.
1. main thread creates 1 resource
2. that resource is passed to the impl thread and allocated
3. main thread deletes that resource
4. a lost context happens (while the delete is still in the queue)
The LTI is perfectly up-to-date, but it won't know that because it didn't and
won't get a Create to tell it that.

Now, this is something of a straw-man because we can say "well, apply it to
Delete as well Create". The next comment discusses that issue in more detail.

     UIResourceInvalidRequest
   };
 
   UIResourceRequest();
   ~UIResourceRequest();
   UIResourceRequestType type;
   UIResourceId id;
   scoped_refptr<UIResourceBitmap> bitmap;
+  uint64 eviction_count_recreated;
 };
 
 class CC_EXPORT LayerTreeHost : NON_EXPORTED_BASE(public RateLimiterClient) {
  public:
   static scoped_ptr<LayerTreeHost> Create(
       LayerTreeHostClient* client,
       const LayerTreeSettings& settings,
       scoped_refptr<base::SingleThreadTaskRunner> impl_task_runner);
   virtual ~LayerTreeHost();
 
@@ skipping 169 matching lines @@
   // CreateUIResource creates a resource given a bitmap.  The bitmap is
   // generated via an interface function, which is called when initializing the
   // resource and when the resource has been lost (due to lost context).  The
   // parameter of the interface is a single boolean, which indicates whether the
   // resource has been lost or not.  CreateUIResource returns an Id of the
   // resource, which is always positive.
   virtual UIResourceId CreateUIResource(UIResourceClient* client);
   // Deletes a UI resource.  May safely be called more than once.
   virtual void DeleteUIResource(UIResourceId id);
 
+  // Record the eviction count sent by the LayerTreeHostImpl at BeginFrame.
+  // If the value of |ui_resource_eviction_count| is greater than the value of
+  // |ui_resource_eviction_count_recreated_| then add the recreation
+  // of all UI resources to the UIResourceRequestQueue and return true.
+  // Otherwise return false.
+  bool SetUIResourceEvictionCount(uint64 ui_resource_eviction_count);
+
   bool UsingSharedMemoryResources();
   int id() const { return tree_id_; }
 
  protected:
   LayerTreeHost(LayerTreeHostClient* client, const LayerTreeSettings& settings);
   bool Initialize(scoped_refptr<base::SingleThreadTaskRunner> impl_task_runner);
   bool InitializeForTesting(scoped_ptr<Proxy> proxy_for_testing);
 
  private:
   bool InitializeProxy(scoped_ptr<Proxy> proxy);
@@ skipping 16 matching lines @@
   void PrioritizeTextures(
       const RenderSurfaceLayerList& render_surface_layer_list,
       OverdrawMetrics* metrics);
   void SetPrioritiesForSurfaces(size_t surface_memory_bytes);
   void SetPrioritiesForLayers(const RenderSurfaceLayerList& update_list);
   size_t CalculateMemoryForRenderSurfaces(
       const RenderSurfaceLayerList& update_list);
 
   bool AnimateLayersRecursive(Layer* current, base::TimeTicks time);
 
-  void RecreateUIResources(bool resource_lost);
-
   typedef base::hash_map<UIResourceId, UIResourceClient*> UIResourceClientMap;
   UIResourceClientMap ui_resource_client_map_;
   int next_ui_resource_id_;
 
   typedef std::list<UIResourceRequest> UIResourceRequestQueue;
   UIResourceRequestQueue ui_resource_request_queue_;
+  uint64 ui_resource_eviction_count_recreated_;

[aelias_OOO_until_Jul13, 2013/09/09 22:52:37]

It doesn't make sense to me to use a count here.  I don't see how the precise
number of evicted resources is useful for anything.  It would make more sense to
either have a boolean, or have a list of evicted UIResourceIDs.

[ccameron, 2013/09/09 23:22:04]

This isn't the number of resource that have been evicted -- it is the number of
times that an eviction has happened. LMK if you have ideas for variable naming
here (I played with some others, but I thought that the comments in LTHI.h were
enough -- I can add more here).

That said, I feel very strongly that this is much easier to follow and verify
correctness of.

The reason for a counter instead of a boolean is that it isn't immediately clear
to me that a boolean will be sufficient.

In the case of the (largely symmetric) ContentsTexturesPurged situation, we use
a bool, but I'd prefer we didn't. We can use a bool in that case because the
value is updated in PushPropertiesTo rather than tree activation, so the fact
that only outstanding 1 commit can exist makes it so that we don't have about
having multiple pending "relevant" ContentsTexturesPurged events outstanding.

In particular, the following situation appears possible to me
1. to start, there exist some UI resources
2. a new resource is created on the main thread
3. that resource is passed to the impl thread and the queue is hung off a LTI
4. the UI old resources are evicted before the queue with the new resource is
processed
5. the LTI is activated, and the queue is processed

Now, if we use a bool, then, when the queue is processed, we will say "all
resources have been recreated, we're good". But we're not -- that queue doesn't
have the recreation of the resources destroyed in step 4.

If we use a bool, then when thinking about this code, we have to prove that the
above sequence can't happen. If we use this eviction count/sequence-number
scheme, we don't have to make this proof every time we consider modifying this
code.

[aelias_OOO_until_Jul13, 2013/09/09 23:44:30]

I agree with your reasoning that a bool is insufficient.  What about my other
suggestion, to store a list of evicted UIResourceIDs?  Or, equivalently, have an
additional bool in the struct stored in the map (Powei is making the LTH map
hold a struct in his NinePatchLayer patch) about whether a given resource needs
recreation?

 
   void CalculateLCDTextMetricsCallback(Layer* layer);
 
   bool animating_;
   bool needs_full_tree_sync_;
   bool needs_filter_context_;
 
   base::CancelableClosure prepaint_callback_;
 
   LayerTreeHostClient* client_;
@@ skipping 73 matching lines @@
   };
   LCDTextMetrics lcd_text_metrics_;
   int tree_id_;
 
   DISALLOW_COPY_AND_ASSIGN(LayerTreeHost);
 };
 
 }  // namespace cc
 
 #endif  // CC_TREES_LAYER_TREE_HOST_H_