Fix use_image_texture_target inconsistencies

content/renderer/render_thread_impl.cc

 // 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.
 
 #include "content/renderer/render_thread_impl.h"
 
 #include <algorithm>
 #include <limits>
 #include <map>
 #include <utility>
@@ skipping 394 matching lines @@
 
  private:
   mojo::StrongBinding<mojom::EmbeddedWorkerSetup> binding_;
 };
 
 void CreateEmbeddedWorkerSetup(
     mojo::InterfaceRequest<mojom::EmbeddedWorkerSetup> request) {
   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(
     scoped_refptr<gpu::GpuChannelHost> gpu_channel_host,
     const gpu::SharedMemoryLimits& limits,
     bool support_locking,
     command_buffer_metrics::ContextType type,
     int32_t stream_id,
     gpu::GpuStreamPriority stream_priority) {
   DCHECK(gpu_channel_host);
@@ skipping 314 matching lines @@
   Boolean key_exists = false;
   Boolean value = CFPreferencesGetAppBooleanValue(
       key, kCFPreferencesCurrentApplication, &key_exists);
   is_elastic_overscroll_enabled_ = !key_exists || value;
 #else
   is_elastic_overscroll_enabled_ = false;
 #endif
 
   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;
   } else if (command_line.HasSwitch(switches::kEnableLCDText)) {
     is_lcd_text_enabled_ = true;
   } else {
 #if defined(OS_ANDROID)
     is_lcd_text_enabled_ = false;
 #else
     is_lcd_text_enabled_ = true;
@@ skipping 677 matching lines @@
   const bool enable_video_accelerator =
       !cmd_line->HasSwitch(switches::kDisableAcceleratedVideoDecode);
   const bool enable_gpu_memory_buffer_video_frames =
 #if defined(OS_MACOSX) || defined(OS_LINUX)
       !cmd_line->HasSwitch(switches::kDisableGpuMemoryBufferVideoFrames) &&
       !cmd_line->HasSwitch(switches::kDisableGpuCompositing) &&
       !gpu_channel_host->gpu_info().software_rendering;
 #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();
 }
 
 scoped_refptr<ContextProviderCommandBuffer>
 RenderThreadImpl::SharedMainThreadContextProvider() {
   DCHECK(IsMainThread());
   if (shared_main_thread_contexts_ &&
       shared_main_thread_contexts_->ContextGL()->GetGraphicsResetStatusKHR() ==
           GL_NO_ERROR)
@@ skipping 110 matching lines @@
 }
 
 bool RenderThreadImpl::IsGpuMemoryBufferCompositorResourcesEnabled() {
   return is_gpu_memory_buffer_compositor_resources_enabled_;
 }
 
 bool RenderThreadImpl::IsElasticOverscrollEnabled() {
   return is_elastic_overscroll_enabled_;
 }
 
-std::vector<unsigned> RenderThreadImpl::GetImageTextureTargets() {
+const cc::RendererSettings::ImageTextureTargetsMap&
+RenderThreadImpl::GetImageTextureTargets() {
   return use_image_texture_targets_;
 }
 
 scoped_refptr<base::SingleThreadTaskRunner>
 RenderThreadImpl::GetCompositorMainThreadTaskRunner() {
   return main_thread_compositor_task_runner_;
 }
 
 scoped_refptr<base::SingleThreadTaskRunner>
 RenderThreadImpl::GetCompositorImplThreadTaskRunner() {
@@ skipping 613 matching lines @@
       v8_memory_pressure_level == v8::MemoryPressureLevel::kCritical)
     v8_memory_pressure_level = v8::MemoryPressureLevel::kModerate;
 
   blink::mainThreadIsolate()->MemoryPressureNotification(
       v8_memory_pressure_level);
   blink::MemoryPressureNotificationToWorkerThreadIsolates(
       v8_memory_pressure_level);
 }
 
 }  // namespace content