[cc] Evict UIResources when the renderer is not visible

cc/trees/layer_tree_host.h

Index: cc/trees/layer_tree_host.h
diff --git a/cc/trees/layer_tree_host.h b/cc/trees/layer_tree_host.h
index e362f84b4ca09789afc7c0ea734edab6f5039e9a..aec2d5fa28503ed45f56c759646c37d109f8a1a3 100644
--- a/cc/trees/layer_tree_host.h
+++ b/cc/trees/layer_tree_host.h
@@ -90,6 +90,7 @@ 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
   };
 
@@ -98,6 +99,7 @@ struct CC_EXPORT UIResourceRequest {
   UIResourceRequestType type;
   UIResourceId id;
   scoped_refptr<UIResourceBitmap> bitmap;
+  uint64 eviction_count_recreated;
 };
 
 class CC_EXPORT LayerTreeHost : NON_EXPORTED_BASE(public RateLimiterClient) {
@@ -287,6 +289,13 @@ class CC_EXPORT LayerTreeHost : NON_EXPORTED_BASE(public RateLimiterClient) {
   // 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_; }
 
@@ -323,14 +332,13 @@ class CC_EXPORT LayerTreeHost : NON_EXPORTED_BASE(public RateLimiterClient) {
 
   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);