Whitelist commands between {Begin,End}RasterCHROMIUM

Flag an error if not in whitelist. Skip over rejected commands.

I used a fake (InProcCommandBuffer) instead of a mock
because it was too difficult to inject (e.g. if you use a fake
GrContext, you have to use fake GrBackendTexture, etc).

Bug: 836928
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I7e3a996fb17491b755f0423d205b385c437f7509
Reviewed-on: https://chromium-review.googlesource.com/1057891
Commit-Queue: Jonathan Backer <backer@chromium.org>
Reviewed-by: enne <enne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558423}

gpu/BUILD.gn

@@ -247,6 +247,7 @@ test("gl_tests") {
     "//base",
     "//base/test:test_support",
     "//base/third_party/dynamic_annotations",
+    "//cc/paint",
     "//components/viz/test:test_support",
     "//gpu/command_buffer/client:gles2_c_lib",
     "//gpu/command_buffer/client:gles2_implementation",

gpu/command_buffer/service/raster_decoder.cc

@@ -253,6 +253,22 @@ ScopedPixelUnpackState::~ScopedPixelUnpackState() {
   state_->RestoreUnpackState();
 }
 
+bool AllowedBetweenBeginEndRaster(CommandId command) {
+  switch (command) {
+    case kCreateTransferCacheEntryINTERNAL:
+    case kDeleteTransferCacheEntryINTERNAL:
+    case kEndRasterCHROMIUM:
+    case kFinish:
+    case kFlush:
+    case kGetError:
+    case kRasterCHROMIUM:
+    case kUnlockTransferCacheEntryINTERNAL:
+      return true;
+    default:
+      return false;
+  }
+}
+
 }  // namespace
 
 class RasterDecoderImpl final : public RasterDecoder,
@@ -1375,12 +1391,12 @@ error::Error RasterDecoderImpl::DoCommandsImpl(unsigned int num_commands,
   const volatile CommandBufferEntry* cmd_data =
       static_cast<const volatile CommandBufferEntry*>(buffer);
   int process_pos = 0;
-  unsigned int command = 0;
+  CommandId command = static_cast<CommandId>(0);
 
   while (process_pos < num_entries && result == error::kNoError &&
          commands_to_process_--) {
     const unsigned int size = cmd_data->value_header.size;
-    command = cmd_data->value_header.command;
+    command = static_cast<CommandId>(cmd_data->value_header.command);
 
     if (size == 0) {
       result = error::kInvalidSize;
@@ -1401,6 +1417,17 @@ error::Error RasterDecoderImpl::DoCommandsImpl(unsigned int num_commands,
     unsigned int command_index = command - kFirstRasterCommand;
     if (command_index < arraysize(command_info)) {
       const CommandInfo& info = command_info[command_index];
+      if (sk_surface_) {
+        if (!AllowedBetweenBeginEndRaster(command)) {
+          LOCAL_SET_GL_ERROR(
+              GL_INVALID_OPERATION, GetCommandName(command),
+              "Unexpected command between BeginRasterCHROMIUM and "
+              "EndRasterCHROMIUM");
+          process_pos += size;
+          cmd_data += size;
+          continue;
+        }
+      }
       unsigned int info_arg_count = static_cast<unsigned int>(info.arg_count);
       if ((info.arg_flags == cmd::kFixed && arg_count == info_arg_count) ||
           (info.arg_flags == cmd::kAtLeastN && arg_count >= info_arg_count)) {

gpu/ipc/client/DEPS

@@ -11,6 +11,7 @@ specific_include_rules = {
     "+components/viz/test/test_gpu_memory_buffer_manager.h",
   ],
   "raster_in_process_context_tests.cc": [
+    "+cc/paint/color_space_transfer_cache_entry.h",
     "+components/viz/common/resources/resource_format.h",
     "+components/viz/common/resources/resource_format_utils.h",
     "+components/viz/test/test_gpu_memory_buffer_manager.h",

gpu/ipc/client/raster_in_process_context_tests.cc

@@ -6,6 +6,7 @@
 
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
+#include "cc/paint/color_space_transfer_cache_entry.h"
 #include "components/viz/common/resources/resource_format.h"
 #include "components/viz/common/resources/resource_format_utils.h"
 #include "components/viz/test/test_gpu_memory_buffer_manager.h"
@@ -153,4 +154,36 @@ TEST_F(RasterInProcessCommandBufferTest, TexStorage2DImage) {
   EXPECT_EQ(static_cast<GLenum>(GL_NO_ERROR), ri_->GetError());
 }
 
+TEST_F(RasterInProcessCommandBufferTest,
+       WhitelistBetweenBeginEndRasterCHROMIUM) {
+  // Check for GPU and driver support
+  if (!context_->GetCapabilities().supports_oop_raster) {
+    return;
+  }
+
+  // Create texture and allocate storage.
+  GLuint texture_id =
+      ri_->CreateTexture(/*use_buffer=*/false, kBufferUsage, kResourceFormat);
+  ri_->TexStorage2D(texture_id, 1, kBufferSize.width(), kBufferSize.height());
+  EXPECT_EQ(static_cast<GLenum>(GL_NO_ERROR), ri_->GetError());
+
+  // Call BeginRasterCHROMIUM.
+  cc::RasterColorSpace color_space(gfx::ColorSpace::CreateSRGB(), 0);
+  ri_->BeginRasterCHROMIUM(texture_id, /*sk_color=*/0, /*msaa_sample_count=*/0,
+                           /*can_use_lcd_text=*/false,
+                           viz::ResourceFormatToClosestSkColorType(
+                               /*gpu_compositing=*/true, kResourceFormat),
+                           color_space);
+  EXPECT_EQ(static_cast<GLenum>(GL_NO_ERROR), ri_->GetError());
+
+  // Should flag an error this command is not allowed between a Begin and
+  // EndRasterCHROMIUM.
+  ri_->CreateTexture(/*use_buffer=*/false, kBufferUsage, kResourceFormat);
+  EXPECT_EQ(static_cast<GLenum>(GL_INVALID_OPERATION), ri_->GetError());
+
+  // Confirm that we skip over without error.
+  ri_->EndRasterCHROMIUM();
+  EXPECT_EQ(static_cast<GLenum>(GL_NO_ERROR), ri_->GetError());
+}
+
 }  // namespace gpu