Splitting GpuMemoryAllocation into two parts: for renderer and browser

content/common/gpu/gpu_memory_allocation.h

 // Copyright (c) 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.
 
 #ifndef CONTENT_COMMON_GPU_GPU_MEMORY_ALLOCATION_H_
 #define CONTENT_COMMON_GPU_GPU_MEMORY_ALLOCATION_H_
 #pragma once
 
 #include "base/basictypes.h"
 
-// These are memory allocation limits assigned to a context.
-// They will change over time, given memory availability, and browser state
-// Exceeding these limits for an unreasonable amount of time will cause context
-// to be lost.
-class GpuMemoryAllocation {
+// These are per context memory allocation limits set by the GpuMemoryManager
+// and assigned to the browser and renderer context.
+// They will change over time, given memory availability, and browser state.
+
+
+// Memory Allocation which will be assigned to the renderer context.
+class GpuMemoryAllocationForRenderer {

[piman, 2012/03/15 20:14:32]

style nit: if it has public fields, and essentially behaves like a struct, then
we make it a struct. If it's a class, then it should have no public fields. See
style guide.

[mmocny, 2012/03/15 20:39:27]

Fair, this is meant to act as a struct.

On 2012/03/15 20:14:32, piman wrote:

  public:
   enum {
     INVALID_RESOURCE_SIZE = -1
   };
 
+  // Exceeding this limit for an unreasonable amount of time may cause context
+  // to be lost.
   size_t gpu_resource_size_in_bytes;
-  bool has_frontbuffer;
-  bool has_backbuffer;
+  bool suggest_have_backbuffer;
 
+  GpuMemoryAllocationForRenderer()
+      : gpu_resource_size_in_bytes(0),
+        suggest_have_backbuffer(false) {
+  }
+
+  GpuMemoryAllocationForRenderer(size_t gpu_resource_size_in_bytes,
+                                 bool suggest_have_backbuffer)
+      : gpu_resource_size_in_bytes(gpu_resource_size_in_bytes),
+        suggest_have_backbuffer(suggest_have_backbuffer) {
+  }
+
+  friend inline bool operator==(const GpuMemoryAllocationForRenderer& lhs,

[piman, 2012/03/15 20:14:32]

No need for friend or inline. Also, if it's a non-static member, it should use
the syntax for the member operator (i.e. 'this' is the lhs, and it takes only
one param).

[mmocny, 2012/03/15 20:39:27]

Since this is the global non-member version of op==, it does need the friend
keyword to allow defining it here (though it doesn't actually need friendship). 
I am not actually sure if it strictly needs inline, it would if it were static
or defined out-of-class.

I'll concede that this is a C++ dark corner and may be awkward, and since I
prefer to leave it defined inside the class I'll change to use the member
version.

On 2012/03/15 20:14:32, piman wrote:

+                                const GpuMemoryAllocationForRenderer& rhs) {
+    return lhs.gpu_resource_size_in_bytes == rhs.gpu_resource_size_in_bytes &&
+        lhs.suggest_have_backbuffer == rhs.suggest_have_backbuffer;
+  }
+
+  friend inline bool operator!=(const GpuMemoryAllocationForRenderer& lhs,
+                                const GpuMemoryAllocationForRenderer& rhs) {
+    return !(lhs == rhs);
+  }
+};
+
+// Memory Allocation which will be assigned to the browser.
+class GpuMemoryAllocationForBrowser {

[piman, 2012/03/15 20:14:32]

class->struct.

+ public:
+  bool suggest_have_frontbuffer;
+
+  GpuMemoryAllocationForBrowser()
+      : suggest_have_frontbuffer(false) {
+  }
+
+  GpuMemoryAllocationForBrowser(bool suggest_have_frontbuffer)
+      : suggest_have_frontbuffer(suggest_have_frontbuffer) {
+  }
+
+  friend inline bool operator==(const GpuMemoryAllocationForBrowser& lhs,
+                                const GpuMemoryAllocationForBrowser& rhs) {
+      return lhs.suggest_have_frontbuffer == rhs.suggest_have_frontbuffer;
+  }
+
+  friend inline bool operator!=(const GpuMemoryAllocationForBrowser& lhs,
+                                const GpuMemoryAllocationForBrowser& rhs) {
+    return !(lhs == rhs);
+  }
+};
+
+// Combination of the above two Memory Allocations which will be created by the
+// GpuMemoryManager.
+class GpuMemoryAllocation : public GpuMemoryAllocationForRenderer,
+                            public GpuMemoryAllocationForBrowser {
+ public:
   GpuMemoryAllocation()
-      : gpu_resource_size_in_bytes(0),
-        has_frontbuffer(false),
-        has_backbuffer(false) {
+      : GpuMemoryAllocationForRenderer(),
+        GpuMemoryAllocationForBrowser() {
   }
 
   GpuMemoryAllocation(size_t gpu_resource_size_in_bytes,
-                      bool has_frontbuffer,
-                      bool has_backbuffer)
-      : gpu_resource_size_in_bytes(gpu_resource_size_in_bytes),
-        has_frontbuffer(has_frontbuffer),
-        has_backbuffer(has_backbuffer) {
+                      bool suggest_have_backbuffer,
+                      bool suggest_have_frontbuffer)
+      : GpuMemoryAllocationForRenderer(gpu_resource_size_in_bytes,
+                                       suggest_have_backbuffer),
+        GpuMemoryAllocationForBrowser(suggest_have_frontbuffer) {
+  }
+
+  friend inline bool operator==(const GpuMemoryAllocation& lhs,
+                                const GpuMemoryAllocation& rhs) {
+      return static_cast<const GpuMemoryAllocationForRenderer&>(lhs) ==
+             static_cast<const GpuMemoryAllocationForRenderer&>(rhs) &&
+             static_cast<const GpuMemoryAllocationForBrowser&>(lhs) ==
+             static_cast<const GpuMemoryAllocationForBrowser&>(rhs);
+  }
+
+  friend inline bool operator!=(const GpuMemoryAllocation& lhs,
+                                const GpuMemoryAllocation& rhs) {
+    return !(lhs == rhs);
   }
 };
 
-inline bool operator==(const GpuMemoryAllocation& lhs,
-                       const GpuMemoryAllocation& rhs) {
-  return lhs.gpu_resource_size_in_bytes == rhs.gpu_resource_size_in_bytes &&
-      lhs.has_frontbuffer == rhs.has_frontbuffer &&
-      lhs.has_backbuffer == rhs.has_backbuffer;
-}
-
-inline bool operator!=(const GpuMemoryAllocation& lhs,
-                       const GpuMemoryAllocation& rhs) {
-  return !(lhs == rhs);
-}
-
 #endif // CONTENT_COMMON_GPU_GPU_MEMORY_ALLOCATION_H_