Add R_8 GPU memory buffers format.

content/common/gpu/client/gpu_memory_buffer_impl.cc

Index: content/common/gpu/client/gpu_memory_buffer_impl.cc
diff --git a/content/common/gpu/client/gpu_memory_buffer_impl.cc b/content/common/gpu/client/gpu_memory_buffer_impl.cc
index 208261dd0831e940f8f858b63a4169888c706db8..7903d3964a9b4bc6d9555c1e0dc5543645c858e3 100644
--- a/content/common/gpu/client/gpu_memory_buffer_impl.cc
+++ b/content/common/gpu/client/gpu_memory_buffer_impl.cc
@@ -80,35 +80,30 @@ GpuMemoryBufferImpl* GpuMemoryBufferImpl::FromClientBuffer(
 bool GpuMemoryBufferImpl::StrideInBytes(size_t width,
                                         Format format,
                                         size_t* stride_in_bytes) {
-  base::CheckedNumeric<size_t> s = width;
+  base::CheckedNumeric<size_t> checked_stride = width;
   switch (format) {
-    case ATCIA:
-    case DXT5:
-      *stride_in_bytes = width;
-      return true;
-    case ATC:
-    case DXT1:
-    case ETC1:
+    case GpuMemoryBuffer::R_8:
+      checked_stride = (checked_stride + 3) % 4;

[reveman, 2015/04/01 13:14:33]

I assume this is a typo and should be "(width + 3) & ~0x3"

[Daniele Castagna, 2015/04/01 21:58:34]

Ops, I was trying to break some test to see if this code was tested anywhere
with the new enum and I left it in. After adding R_8 to
GpuMemoryBufferFactorySharedMemory::GetSupportedGpuMemoryBufferConfigurations
the GpuMemoryBufferFactoryTest correctly started to fail.

+      break;
+    case GpuMemoryBuffer::ATCIA:
+    case GpuMemoryBuffer::DXT5:  // 'checked_stride' is the same as 'width'.

[reveman, 2015/04/01 13:14:33]

nit: no need for checked_stride in this case

[Daniele Castagna, 2015/04/01 21:58:34]

Done.

+      break;
+    case GpuMemoryBuffer::ATC:
+    case GpuMemoryBuffer::DXT1:
+    case GpuMemoryBuffer::ETC1:
       DCHECK_EQ(width % 2, 0U);

[reveman, 2015/04/01 13:14:33]

nit: ..., 0u);

[Daniele Castagna, 2015/04/01 21:58:34]

Done.

-      s /= 2;
-      if (!s.IsValid())
-        return false;
-
-      *stride_in_bytes = s.ValueOrDie();
-      return true;
-    case RGBA_8888:
-    case RGBX_8888:
-    case BGRA_8888:
-      s *= 4;
-      if (!s.IsValid())
-        return false;
-
-      *stride_in_bytes = s.ValueOrDie();
-      return true;
+      checked_stride /= 2;

[reveman, 2015/04/01 13:14:33]

no need for checked_stride in this case.

[Daniele Castagna, 2015/04/01 21:58:34]

Done.

+      break;
+    case GpuMemoryBuffer::RGBA_8888:
+    case GpuMemoryBuffer::RGBX_8888:
+    case GpuMemoryBuffer::BGRA_8888:
+      checked_stride *= 4;
+      break;
   }
-
-  NOTREACHED();

[reveman, 2015/04/01 13:14:33]

I like to keep this condition. ie detect if format becomes an invalid value at
runtime instead of silently returning |width| as stride. I would prefer if the
base::CheckedNumeric was moved into only the cases where needed as I think that
makes it more clear when those checks are needed.

[Daniele Castagna, 2015/04/01 21:58:34]

Done as you suggested.

For the sake of arguing: I think leaving CheckedNumeric everywhere clearly
conveys the idea that 'width' is not to be trusted and has to be validated.

Good point about NOTREACHED(). Isn't 'format' potentially coming from the
renderer (from ChildProcessHostMsg_SyncAllocateGpuMemoryBuffer) as for 'width'?
Should we validate it and return false instead of asserting?

[reveman, 2015/04/01 23:38:55]

Ok, fair enough.
The IPC system will automatically verify that format is a valid enum value as it
arrives. This NOTREACHED is only in case we mess it up in the GPU process after
the IPC arrives.

-  return false;
+  if (!checked_stride.IsValid())
+    return false;
+  *stride_in_bytes = checked_stride.ValueOrDie();
+  return true;
 }
 
 // static
@@ -120,6 +115,7 @@ size_t GpuMemoryBufferImpl::NumberOfPlanesForGpuMemoryBufferFormat(
     case gfx::GpuMemoryBuffer::Format::DXT1:
     case gfx::GpuMemoryBuffer::Format::DXT5:
     case gfx::GpuMemoryBuffer::Format::ETC1:
+    case gfx::GpuMemoryBuffer::Format::R_8:
     case gfx::GpuMemoryBuffer::Format::RGBA_8888:
     case gfx::GpuMemoryBuffer::Format::RGBX_8888:
     case gfx::GpuMemoryBuffer::Format::BGRA_8888: