Fix use_image_texture_target inconsistencies

content/renderer/render_thread_impl.cc

Index: content/renderer/render_thread_impl.cc
diff --git a/content/renderer/render_thread_impl.cc b/content/renderer/render_thread_impl.cc
index 0845883e185e198d5d91837bd9fae4da9262772c..46afc5a27a25fb0dd5fa5f8da55984429098bfc4 100644
--- a/content/renderer/render_thread_impl.cc
+++ b/content/renderer/render_thread_impl.cc
@@ -412,17 +412,33 @@ void CreateEmbeddedWorkerSetup(
   new EmbeddedWorkerSetupImpl(std::move(request));
 }
 
-void StringToUintVector(const std::string& str, std::vector<unsigned>* vector) {
-  DCHECK(vector->empty());
-  std::vector<std::string> pieces = base::SplitString(
-      str, ",", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
-  DCHECK_EQ(pieces.size(), static_cast<size_t>(gfx::BufferFormat::LAST) + 1);
-  for (size_t i = 0; i < pieces.size(); ++i) {
-    unsigned number = 0;
-    bool succeed = base::StringToUint(pieces[i], &number);
-    DCHECK(succeed);
-    vector->push_back(number);
+// Converts a serialized ImageTextureTargetsMap back to the runtime format.
+// Serialization takes the form:
+//   "usage,format,target;usage,format,target;...;usage,format,target"
+cc::RendererSettings::ImageTextureTargetsMap StringToImageTextureTargetsMap(
+    const std::string& str) {
+  cc::RendererSettings::ImageTextureTargetsMap map;
+  std::vector<std::string> entries =
+      base::SplitString(str, ";", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
+  for (const auto& entry : entries) {
+    std::vector<std::string> fields = base::SplitString(
+        entry, ",", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
+    DCHECK_EQ(fields.size(), 3);

[enne (OOO), 2016/07/15 21:39:32]

Mmm, is this safe if the dcheck fails? Should it be a CHECK?

I think I'd feel better if the encoding/decoding of command line args were in
some separate utility class that could be unit tested.

[ericrk, 2016/07/19 20:22:43]

Changed these to CHECKs and added a unit test. We should never end up with
invalid data here, so crashing seems appropriate.

Moved this to a helper class (along with serialization function), and added a
unit test.

+    uint32_t usage = 0;
+    uint32_t format = 0;
+    uint32_t target = 0;
+    bool succeeded = base::StringToUint(fields[0], &usage) &&
+                     base::StringToUint(fields[1], &format) &&
+                     base::StringToUint(fields[2], &target);
+    DCHECK(succeeded);
+    DCHECK_LE(usage, static_cast<uint32_t>(gfx::BufferUsage::LAST));
+    DCHECK_LE(format, static_cast<uint32_t>(gfx::BufferFormat::LAST));
+    map.emplace(cc::RendererSettings::ImageTextureTargetKey(
+                    static_cast<gfx::BufferUsage>(usage),
+                    static_cast<gfx::BufferFormat>(format)),
+                target);
   }
+  return map;
 }
 
 scoped_refptr<ContextProviderCommandBuffer> CreateOffscreenContext(
@@ -757,7 +773,8 @@ void RenderThreadImpl::Init(
 
   std::string image_texture_target_string =
       command_line.GetSwitchValueASCII(switches::kContentImageTextureTarget);
-  StringToUintVector(image_texture_target_string, &use_image_texture_targets_);
+  use_image_texture_targets_ =
+      StringToImageTextureTargetsMap(image_texture_target_string);
 
   if (command_line.HasSwitch(switches::kDisableLCDText)) {
     is_lcd_text_enabled_ = false;
@@ -1455,16 +1472,11 @@ media::GpuVideoAcceleratorFactories* RenderThreadImpl::GetGpuFactories() {
 #else
       cmd_line->HasSwitch(switches::kEnableGpuMemoryBufferVideoFrames);
 #endif
-  std::vector<unsigned> image_texture_targets;
-  std::string video_frame_image_texture_target_string =
-      cmd_line->GetSwitchValueASCII(switches::kVideoImageTextureTarget);
-  StringToUintVector(video_frame_image_texture_target_string,
-                     &image_texture_targets);
 
   gpu_factories_.push_back(RendererGpuVideoAcceleratorFactories::Create(
       std::move(gpu_channel_host), base::ThreadTaskRunnerHandle::Get(),
       media_task_runner, std::move(media_context_provider),
-      enable_gpu_memory_buffer_video_frames, image_texture_targets,
+      enable_gpu_memory_buffer_video_frames, use_image_texture_targets_,
       enable_video_accelerator));
   return gpu_factories_.back();
 }
@@ -1595,7 +1607,8 @@ bool RenderThreadImpl::IsElasticOverscrollEnabled() {
   return is_elastic_overscroll_enabled_;
 }
 
-std::vector<unsigned> RenderThreadImpl::GetImageTextureTargets() {
+const cc::RendererSettings::ImageTextureTargetsMap&
+RenderThreadImpl::GetImageTextureTargets() {
   return use_image_texture_targets_;
 }