Pass vector<OverlayStrategy> instead of string.

CompositorOverlayCandidateValidatorOzone took a string that was really a
comma separated list of overlay strategies. The string was parsed into a
list of callbacks that were later used to create the appropriate
strategy objects.

This CL changes CompositorOverlayCandidateValidatorOzone to take a list
of OverlayStrategy enums instead. The enum already existed for UMA and
it contains an entry for each overlay strategy. Use the list of enum
values to directly instantiate the strategy objects, saving a layer
indirection.

Move the enum to //components/viz/common so it can be used with IPC
later. Also move the code to parse a string containing a comma separated
list of strategies, as this is used by the --enable-hardware-overlays
flag.

Bug: 930173
Change-Id: Ie5d6d501e3117cf4be0fcf29ac8bdf29744041da
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1505839Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#638598}

components/viz/common/BUILD.gn

@@ -82,6 +82,8 @@ viz_component("common") {
   sources = [
     "constants.cc",
     "constants.h",
+    "display/overlay_strategy.cc",
+    "display/overlay_strategy.h",
     "display/renderer_settings.cc",
     "display/renderer_settings.h",
     "features.cc",
@@ -254,6 +256,7 @@ viz_source_set("unit_tests") {
   never_build_jumbo = true
   testonly = true
   sources = [
+    "display/overlay_strategy_unittest.cc",
     "frame_sinks/begin_frame_args_unittest.cc",
     "frame_sinks/begin_frame_source_unittest.cc",
     "frame_sinks/copy_output_util_unittest.cc",

components/viz/common/display/overlay_strategy.cc

+// Copyright 2019 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 "components/viz/common/display/overlay_strategy.h"
+
+#include "base/logging.h"
+#include "base/strings/string_split.h"
+
+namespace viz {
+
+std::vector<OverlayStrategy> ParseOverlayStategies(
+    const std::string& strategies_string) {
+  std::vector<OverlayStrategy> strategies;
+
+  auto strategy_names = base::SplitStringPiece(
+      strategies_string, ",", base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
+  for (auto& strategy_name : strategy_names) {
+    if (strategy_name == "single-fullscreen") {
+      strategies.push_back(OverlayStrategy::kFullscreen);
+    } else if (strategy_name == "single-on-top") {
+      strategies.push_back(OverlayStrategy::kSingleOnTop);
+    } else if (strategy_name == "underlay") {
+      strategies.push_back(OverlayStrategy::kUnderlay);
+    } else if (strategy_name == "cast") {
+      strategies.push_back(OverlayStrategy::kUnderlayCast);
+    } else {
+      LOG(ERROR) << "Unrecognized overlay strategy " << strategy_name;
+    }
+  }
+  return strategies;
+}
+
+}  // namespace viz

components/viz/common/display/overlay_strategy.h

+// Copyright 2019 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 COMPONENTS_VIZ_COMMON_DISPLAY_OVERLAY_STRATEGY_H_
+#define COMPONENTS_VIZ_COMMON_DISPLAY_OVERLAY_STRATEGY_H_
+
+#include <string>
+#include <vector>
+
+#include "components/viz/common/viz_common_export.h"
+
+namespace viz {
+
+// Enum used for UMA histogram. These enum values must not be changed or
+// reused.
+enum class OverlayStrategy {
+  kUnknown = 0,
+  kNoStrategyUsed = 1,
+  kFullscreen = 2,
+  kSingleOnTop = 3,
+  kUnderlay = 4,
+  kUnderlayCast = 5,
+  kMaxValue = kUnderlayCast,
+};
+
+// Parses a comma separated list of overlay strategy types and returns a list
+// of the corresponding OverlayStrategy enum values.
+VIZ_COMMON_EXPORT std::vector<OverlayStrategy> ParseOverlayStategies(
+    const std::string& strategies_string);
+
+}  // namespace viz
+
+#endif  // COMPONENTS_VIZ_COMMON_DISPLAY_OVERLAY_STRATEGY_H_

components/viz/common/display/overlay_strategy_unittest.cc

+// Copyright 2019 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 "components/viz/common/display/overlay_strategy.h"
+
+#include <string>
+#include <vector>
+
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using testing::IsEmpty;
+using testing::UnorderedElementsAre;
+
+namespace viz {
+
+TEST(ParseOverlayStrategiesTest, ParseEmptyList) {
+  std::vector<OverlayStrategy> strategies = ParseOverlayStategies("");
+  EXPECT_THAT(strategies, IsEmpty());
+}
+
+TEST(ParseOverlayStrategiesTest, ParseFullList) {
+  std::vector<OverlayStrategy> strategies =
+      ParseOverlayStategies("single-fullscreen,single-on-top,underlay,cast");
+
+  EXPECT_THAT(strategies, UnorderedElementsAre(OverlayStrategy::kFullscreen,
+                                               OverlayStrategy::kSingleOnTop,
+                                               OverlayStrategy::kUnderlay,
+                                               OverlayStrategy::kUnderlayCast));
+}
+
+TEST(ParseOverlayStrategiesTest, BadValue) {
+  std::vector<OverlayStrategy> strategies =
+      ParseOverlayStategies("single-fullscreen,bad-value,underlay");
+
+  // The string "bad-value" doesn't correspond to an overlay strategy so it
+  // should be skipped.
+  EXPECT_THAT(strategies, UnorderedElementsAre(OverlayStrategy::kFullscreen,
+                                               OverlayStrategy::kUnderlay));
+}
+
+}  // namespace viz

components/viz/service/display/overlay_processor.cc

@@ -46,8 +46,8 @@ class SendPromotionHintsBeforeReturning {
 
 }  // namespace
 
-OverlayProcessor::StrategyType OverlayProcessor::Strategy::GetUMAEnum() const {
-  return StrategyType::kUnknown;
+OverlayStrategy OverlayProcessor::Strategy::GetUMAEnum() const {
+  return OverlayStrategy::kUnknown;
 }
 
 OverlayProcessor::OverlayProcessor(OutputSurface* surface)
@@ -194,7 +194,7 @@ void OverlayProcessor::ProcessForOverlays(
   UMA_HISTOGRAM_ENUMERATION("Viz.DisplayCompositor.OverlayStrategy",
                             successful_strategy
                                 ? successful_strategy->GetUMAEnum()
-                                : StrategyType::kNoStrategyUsed);
+                                : OverlayStrategy::kNoStrategyUsed);
 
   TRACE_COUNTER1(TRACE_DISABLED_BY_DEFAULT("viz.debug.overlay_planes"),
                  "Scheduled overlay planes", candidates->size());

components/viz/service/display/overlay_processor.h

@@ -9,6 +9,7 @@
 
 #include "base/containers/flat_map.h"
 #include "base/macros.h"
+#include "components/viz/common/display/overlay_strategy.h"
 #include "components/viz/common/quads/render_pass.h"
 #include "components/viz/service/display/ca_layer_overlay.h"
 #include "components/viz/service/display/dc_layer_overlay.h"
@@ -24,18 +25,6 @@ class OutputSurface;
 
 class VIZ_SERVICE_EXPORT OverlayProcessor {
  public:
-  // Enum used for UMA histogram. These enum values must not be changed or
-  // reused.
-  enum class StrategyType {
-    kUnknown = 0,
-    kNoStrategyUsed = 1,
-    kFullscreen = 2,
-    kSingleOnTop = 3,
-    kUnderlay = 4,
-    kUnderlayCast = 5,
-    kMaxValue = kUnderlayCast,
-  };
-
   using FilterOperationsMap =
       base::flat_map<RenderPassId, cc::FilterOperations*>;
 
@@ -55,7 +44,7 @@ class VIZ_SERVICE_EXPORT OverlayProcessor {
         OverlayCandidateList* candidates,
         std::vector<gfx::Rect>* content_bounds) = 0;
 
-    virtual StrategyType GetUMAEnum() const;
+    virtual OverlayStrategy GetUMAEnum() const;
   };
   using StrategyList = std::vector<std::unique_ptr<Strategy>>;
 

components/viz/service/display/overlay_strategy_fullscreen.cc

@@ -69,8 +69,8 @@ bool OverlayStrategyFullscreen::Attempt(
   return true;
 }
 
-OverlayProcessor::StrategyType OverlayStrategyFullscreen::GetUMAEnum() const {
-  return OverlayProcessor::StrategyType::kFullscreen;
+OverlayStrategy OverlayStrategyFullscreen::GetUMAEnum() const {
+  return OverlayStrategy::kFullscreen;
 }
 
 }  // namespace viz

components/viz/service/display/overlay_strategy_fullscreen.h

@@ -30,7 +30,7 @@ class VIZ_SERVICE_EXPORT OverlayStrategyFullscreen
       OverlayCandidateList* candidate_list,
       std::vector<gfx::Rect>* content_bounds) override;
 
-  OverlayProcessor::StrategyType GetUMAEnum() const override;
+  OverlayStrategy GetUMAEnum() const override;
 
  private:
   OverlayCandidateValidator* capability_checker_;  // Weak.

components/viz/service/display/overlay_strategy_single_on_top.cc

@@ -75,8 +75,8 @@ bool OverlayStrategySingleOnTop::TryOverlay(
   return false;
 }
 
-OverlayProcessor::StrategyType OverlayStrategySingleOnTop::GetUMAEnum() const {
-  return OverlayProcessor::StrategyType::kSingleOnTop;
+OverlayStrategy OverlayStrategySingleOnTop::GetUMAEnum() const {
+  return OverlayStrategy::kSingleOnTop;
 }
 
 }  // namespace viz

components/viz/service/display/overlay_strategy_single_on_top.h

@@ -28,7 +28,7 @@ class VIZ_SERVICE_EXPORT OverlayStrategySingleOnTop
       OverlayCandidateList* candidate_list,
       std::vector<gfx::Rect>* content_bounds) override;
 
-  OverlayProcessor::StrategyType GetUMAEnum() const override;
+  OverlayStrategy GetUMAEnum() const override;
 
  private:
   bool TryOverlay(QuadList* quad_list,

components/viz/service/display/overlay_strategy_underlay.cc

@@ -100,8 +100,8 @@ bool OverlayStrategyUnderlay::Attempt(
   return false;
 }
 
-OverlayProcessor::StrategyType OverlayStrategyUnderlay::GetUMAEnum() const {
-  return OverlayProcessor::StrategyType::kUnderlay;
+OverlayStrategy OverlayStrategyUnderlay::GetUMAEnum() const {
+  return OverlayStrategy::kUnderlay;
 }
 
 }  // namespace viz

components/viz/service/display/overlay_strategy_underlay.h

@@ -44,7 +44,7 @@ class VIZ_SERVICE_EXPORT OverlayStrategyUnderlay
       OverlayCandidateList* candidate_list,
       std::vector<gfx::Rect>* content_bounds) override;
 
-  OverlayProcessor::StrategyType GetUMAEnum() const override;
+  OverlayStrategy GetUMAEnum() const override;
 
  private:
   OverlayCandidateValidator* capability_checker_;  // Weak.

components/viz/service/display/overlay_strategy_underlay_cast.cc

@@ -122,8 +122,8 @@ bool OverlayStrategyUnderlayCast::Attempt(
   return found_underlay;
 }
 
-OverlayProcessor::StrategyType OverlayStrategyUnderlayCast::GetUMAEnum() const {
-  return OverlayProcessor::StrategyType::kUnderlayCast;
+OverlayStrategy OverlayStrategyUnderlayCast::GetUMAEnum() const {
+  return OverlayStrategy::kUnderlayCast;
 }
 
 // static

components/viz/service/display/overlay_strategy_underlay_cast.h

@@ -38,7 +38,7 @@ class VIZ_SERVICE_EXPORT OverlayStrategyUnderlayCast
       base::RepeatingCallback<void(const gfx::RectF&, gfx::OverlayTransform)>;
   static void SetOverlayCompositedCallback(const OverlayCompositedCallback& cb);
 
-  OverlayProcessor::StrategyType GetUMAEnum() const override;
+  OverlayStrategy GetUMAEnum() const override;
 
  private:
   // Keep track if an overlay is being used on the previous frame.

components/viz/service/display_embedder/compositor_overlay_candidate_validator_ozone.cc

@@ -8,8 +8,6 @@
 
 #include <utility>
 
-#include "base/bind.h"
-#include "base/strings/string_split.h"
 #include "components/viz/service/display/overlay_strategy_fullscreen.h"
 #include "components/viz/service/display/overlay_strategy_single_on_top.h"
 #include "components/viz/service/display/overlay_strategy_underlay.h"
@@ -17,60 +15,44 @@
 #include "ui/ozone/public/overlay_candidates_ozone.h"
 
 namespace viz {
-namespace {
-// Templated function used to create an OverlayProcessor::Strategy
-// of type |S|.
-template <typename S>
-std::unique_ptr<OverlayProcessor::Strategy> MakeOverlayStrategy(
-    CompositorOverlayCandidateValidatorOzone* capability_checker) {
-  return std::make_unique<S>(capability_checker);
-}
-
-}  // namespace
 
 // |overlay_candidates| is an object used to answer questions about possible
 // overlays configurations.
-// |strategies_string| is a comma-separated string containing all the overlay
-// strategies that should be returned by GetStrategies.
-// If |strategies_string| is empty "single-on-top,underlay" will be used as
-// default.
+// |strategies_string| is a list of overlay strategies that should be returned
+// by GetStrategies.
 CompositorOverlayCandidateValidatorOzone::
     CompositorOverlayCandidateValidatorOzone(
         std::unique_ptr<ui::OverlayCandidatesOzone> overlay_candidates,
-        std::string strategies_string)
+        std::vector<OverlayStrategy> strategies)
     : overlay_candidates_(std::move(overlay_candidates)),
-      software_mirror_active_(false) {
-  if (!strategies_string.length())
-    strategies_string = "single-on-top,underlay";
-
-  for (const auto& strategy_name :
-       base::SplitStringPiece(strategies_string, ",", base::TRIM_WHITESPACE,
-                              base::SPLIT_WANT_NONEMPTY)) {
-    if (strategy_name == "single-fullscreen") {
-      strategies_instantiators_.push_back(
-          base::BindRepeating(MakeOverlayStrategy<OverlayStrategyFullscreen>));
-    } else if (strategy_name == "single-on-top") {
-      strategies_instantiators_.push_back(
-          base::BindRepeating(MakeOverlayStrategy<OverlayStrategySingleOnTop>));
-    } else if (strategy_name == "underlay") {
-      strategies_instantiators_.push_back(
-          base::BindRepeating(MakeOverlayStrategy<OverlayStrategyUnderlay>));
-    } else if (strategy_name == "cast") {
-      strategies_instantiators_.push_back(base::BindRepeating(
-          MakeOverlayStrategy<OverlayStrategyUnderlayCast>));
-    } else {
-      LOG(WARNING) << "Unrecognized overlay strategy " << strategy_name;
-    }
-  }
-}
+      strategies_(std::move(strategies)) {}
 
 CompositorOverlayCandidateValidatorOzone::
     ~CompositorOverlayCandidateValidatorOzone() {}
 
 void CompositorOverlayCandidateValidatorOzone::GetStrategies(
     OverlayProcessor::StrategyList* strategies) {
-  for (auto& instantiator : strategies_instantiators_)
-    strategies->push_back(instantiator.Run(this));
+  for (OverlayStrategy strategy : strategies_) {
+    switch (strategy) {
+      case OverlayStrategy::kFullscreen:
+        strategies->push_back(
+            std::make_unique<OverlayStrategyFullscreen>(this));
+        break;
+      case OverlayStrategy::kSingleOnTop:
+        strategies->push_back(
+            std::make_unique<OverlayStrategySingleOnTop>(this));
+        break;
+      case OverlayStrategy::kUnderlay:
+        strategies->push_back(std::make_unique<OverlayStrategyUnderlay>(this));
+        break;
+      case OverlayStrategy::kUnderlayCast:
+        strategies->push_back(
+            std::make_unique<OverlayStrategyUnderlayCast>(this));
+        break;
+      default:
+        NOTREACHED();
+    }
+  }
 }
 
 bool CompositorOverlayCandidateValidatorOzone::AllowCALayerOverlays() {

components/viz/service/display_embedder/compositor_overlay_candidate_validator_ozone.h

@@ -6,9 +6,10 @@
 #define COMPONENTS_VIZ_SERVICE_DISPLAY_EMBEDDER_COMPOSITOR_OVERLAY_CANDIDATE_VALIDATOR_OZONE_H_
 
 #include <memory>
+#include <vector>
 
-#include "base/callback.h"
 #include "base/macros.h"
+#include "components/viz/common/display/overlay_strategy.h"
 #include "components/viz/service/display_embedder/compositor_overlay_candidate_validator.h"
 #include "components/viz/service/viz_service_export.h"
 #include "ui/gfx/native_widget_types.h"
@@ -24,7 +25,7 @@ class VIZ_SERVICE_EXPORT CompositorOverlayCandidateValidatorOzone
  public:
   CompositorOverlayCandidateValidatorOzone(
       std::unique_ptr<ui::OverlayCandidatesOzone> overlay_candidates,
-      std::string strategies_string);
+      std::vector<OverlayStrategy> strategies);
   ~CompositorOverlayCandidateValidatorOzone() override;
 
   // OverlayCandidateValidator implementation.
@@ -38,14 +39,8 @@ class VIZ_SERVICE_EXPORT CompositorOverlayCandidateValidatorOzone
 
  private:
   std::unique_ptr<ui::OverlayCandidatesOzone> overlay_candidates_;
-  // Callback declaration to allocate a new OverlayProcessor::Strategy.
-  using StrategyInstantiator =
-      base::RepeatingCallback<std::unique_ptr<OverlayProcessor::Strategy>(
-          CompositorOverlayCandidateValidatorOzone*)>;
-  // List callbacks used to instantiate OverlayProcessor::Strategy
-  // as defined by |strategies_string| paramter in the constructor.
-  std::vector<StrategyInstantiator> strategies_instantiators_;
-  bool software_mirror_active_;
+  const std::vector<OverlayStrategy> strategies_;
+  bool software_mirror_active_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(CompositorOverlayCandidateValidatorOzone);
 };

content/browser/compositor/gpu_process_transport_factory.cc

@@ -274,7 +274,8 @@ CreateOverlayCandidateValidator(
     std::unique_ptr<ui::OverlayCandidatesOzone> overlay_candidates =
         ozone_platform->GetOverlayManager()->CreateOverlayCandidates(widget);
     validator.reset(new viz::CompositorOverlayCandidateValidatorOzone(
-        std::move(overlay_candidates), enable_overlay_flag));
+        std::move(overlay_candidates),
+        viz::ParseOverlayStategies(enable_overlay_flag)));
   }
 #elif defined(OS_MACOSX)
   // Overlays are only supported through the remote layer API.

content/browser/compositor/reflector_impl_unittest.cc

@@ -67,11 +67,10 @@ class TestOverlayCandidatesOzone : public ui::OverlayCandidatesOzone {
 std::unique_ptr<viz::CompositorOverlayCandidateValidator>
 CreateTestValidatorOzone() {
 #if defined(USE_OZONE)
-  return std::unique_ptr<viz::CompositorOverlayCandidateValidator>(
-      new viz::CompositorOverlayCandidateValidatorOzone(
-          std::unique_ptr<ui::OverlayCandidatesOzone>(
-              new TestOverlayCandidatesOzone()),
-          ""));
+  std::vector<viz::OverlayStrategy> strategies = {
+      viz::OverlayStrategy::kSingleOnTop, viz::OverlayStrategy::kUnderlay};
+  return std::make_unique<viz::CompositorOverlayCandidateValidatorOzone>(
+      std::make_unique<TestOverlayCandidatesOzone>(), std::move(strategies));
 #else
   return nullptr;
 #endif  // defined(USE_OZONE)