ash: Scrollable VC bubble effects section

BubbleView now makes the effects controls sections children of a
ScrollView. This is done in (overridden) ViewAddedToWidget() because
BubbleDialogDelegate::GetBubbleBounds() requires a parent widget,
which isn't officially assigned until after the call to ShowBubble()
in VideoConferenceTray::ToggleBubble().

The "return to app" button is added to a top-level FlexLayout in the
BubbleView constructor, as it is not part of the scrollable area.

The fake controller now owns a
fake_video_conference::EffectRepository, which contains an instance of
each of the fake effects. Now individual effects can be added/removed
as needed without requiring changes to the fake controller. To include
the effects in the fake_video_conference::EffectRepository, the
feature "VcControlsUiFakeEffects" must be enabled.

Bug: b/262767123
Change-Id: Ica668ef64f179a9835df05fa8a1e00620b18f4d7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4112846
Reviewed-by: Alex Newcomer <newcomer@chromium.org>
Commit-Queue: Roger Tinkoff <rtinkoff@google.com>
Cr-Commit-Position: refs/heads/main@{#1084733}

ash/constants/ash_features.cc

@@ -2079,6 +2079,12 @@ BASE_FEATURE(kUserActivityPrediction,
 // Enable or disable the ChromeOS video conferencing controls UI.
 BASE_FEATURE(kVcControlsUi, "VcControlsUi", base::FEATURE_DISABLED_BY_DEFAULT);
 
+// Enable or disable the fake effects for ChromeOS video conferencing controls
+// UI. Only meaningful in the emulator.
+BASE_FEATURE(kVcControlsUiFakeEffects,
+             "VcControlsUiFakeEffects",
+             base::FEATURE_DISABLED_BY_DEFAULT);
+
 // Enable or disable multitouch for virtual keyboard on ChromeOS.
 BASE_FEATURE(kVirtualKeyboardMultitouch,
              "VirtualKeyboardMultitouch",
@@ -2236,8 +2242,9 @@ bool AreCaptureModeDemoToolsEnabled() {
 }
 
 bool AreContextualNudgesEnabled() {
-  if (!IsHideShelfControlsInTabletModeEnabled())
+  if (!IsHideShelfControlsInTabletModeEnabled()) {
     return false;
+  }
   return base::FeatureList::IsEnabled(kContextualNudges);
 }
 
@@ -3178,6 +3185,10 @@ bool IsVcControlsUiEnabled() {
   return base::FeatureList::IsEnabled(kVcControlsUi);
 }
 
+bool IsVcControlsUiFakeEffectsEnabled() {
+  return base::FeatureList::IsEnabled(kVcControlsUiFakeEffects);
+}
+
 bool IsViewPpdEnabled() {
   return base::FeatureList::IsEnabled(kEnableViewPpd);
 }

ash/constants/ash_features.h

@@ -593,6 +593,7 @@ BASE_DECLARE_FEATURE(kUseStorkSmdsServerAddress);
 COMPONENT_EXPORT(ASH_CONSTANTS) BASE_DECLARE_FEATURE(kUseWallpaperStagingUrl);
 COMPONENT_EXPORT(ASH_CONSTANTS) BASE_DECLARE_FEATURE(kUserActivityPrediction);
 COMPONENT_EXPORT(ASH_CONSTANTS) BASE_DECLARE_FEATURE(kVcControlsUi);
+COMPONENT_EXPORT(ASH_CONSTANTS) BASE_DECLARE_FEATURE(kVcControlsUiFakeEffects);
 COMPONENT_EXPORT(ASH_CONSTANTS)
 BASE_DECLARE_FEATURE(kVirtualKeyboardBorderedKey);
 COMPONENT_EXPORT(ASH_CONSTANTS)
@@ -876,6 +877,7 @@ COMPONENT_EXPORT(ASH_CONSTANTS) bool IsVCBackgroundBlurEnabled();
 COMPONENT_EXPORT(ASH_CONSTANTS) bool IsVCBackgroundReplaceEnabled();
 COMPONENT_EXPORT(ASH_CONSTANTS) bool IsVCPortraitRelightingEnabled();
 COMPONENT_EXPORT(ASH_CONSTANTS) bool IsVcControlsUiEnabled();
+COMPONENT_EXPORT(ASH_CONSTANTS) bool IsVcControlsUiFakeEffectsEnabled();
 COMPONENT_EXPORT(ASH_CONSTANTS) bool IsViewPpdEnabled();
 COMPONENT_EXPORT(ASH_CONSTANTS) bool IsWallpaperFastRefreshEnabled();
 COMPONENT_EXPORT(ASH_CONSTANTS) bool IsWallpaperFullScreenPreviewEnabled();

ash/system/video_conference/bubble/bubble_view.cc

@@ -12,7 +12,10 @@
 #include "ash/system/video_conference/effects/video_conference_tray_effects_manager.h"
 #include "ash/system/video_conference/video_conference_tray_controller.h"
 #include "ui/views/border.h"
+#include "ui/views/controls/scroll_view.h"
+#include "ui/views/layout/fill_layout.h"
 #include "ui/views/layout/flex_layout.h"
+#include "ui/views/layout/flex_layout_view.h"
 
 namespace ash::video_conference {
 
@@ -24,22 +27,54 @@ const int kBorderInsetDimension = 10;
 
 BubbleView::BubbleView(const InitParams& init_params,
                        VideoConferenceTrayController* controller)
-    : TrayBubbleView(init_params) {
+    : TrayBubbleView(init_params), controller_(controller) {
   SetID(BubbleViewID::kMainBubbleView);
 
+  // Add a `FlexLayout` for the entire view.
   views::FlexLayout* layout =
       SetLayoutManager(std::make_unique<views::FlexLayout>());
   layout->SetOrientation(views::LayoutOrientation::kVertical);
   layout->SetMainAxisAlignment(views::LayoutAlignment::kCenter);
   layout->SetCrossAxisAlignment(views::LayoutAlignment::kStretch);
 
+  // `ReturnToAppButton` resides in the top-level layout and isn't part of the
+  // scrollable area (that can't be added until the `BubbleView` officially has
+  // a parent waidget).
   AddChildView(std::make_unique<ReturnToAppButton>());
+}
+
+void BubbleView::AddedToWidget() {
+  // Create the `views::ScrollView` to house the effects sections. This has to
+  // be done here because `BubbleDialogDelegate::GetBubbleBounds` requires a
+  // parent widget, which isn't officially assigned until after the call to
+  // `ShowBubble` in `VideoConferenceTray::ToggleBubble`.
+  auto* scroll_view = AddChildView(std::make_unique<views::ScrollView>());
+  scroll_view->SetAllowKeyboardScrolling(false);
+  scroll_view->SetBackgroundColor(absl::nullopt);
 
-  if (controller->effects_manager().HasToggleEffects())
-    AddChildView(std::make_unique<ToggleEffectsView>(controller));
+  // TODO(b/262930924): Use the correct max_height.
+  scroll_view->ClipHeightTo(/*min_height=*/0, /*max_height=*/300);
+  scroll_view->SetDrawOverflowIndicator(false);
+  scroll_view->SetVerticalScrollBarMode(
+      views::ScrollView::ScrollBarMode::kHiddenButEnabled);
 
-  if (controller->effects_manager().HasSetValueEffects()) {
-    AddChildView(std::make_unique<SetValueEffectsView>(controller));
+  // `FlexLayout` makes the most sense for the contents of the
+  // `views::ScrollView`, since the effects sections are stacked vertically and
+  // need to expand to fill the bubble.
+  views::FlexLayoutView* layout =
+      scroll_view->SetContents(std::make_unique<views::FlexLayoutView>());
+  layout->SetOrientation(views::LayoutOrientation::kVertical);
+  layout->SetMainAxisAlignment(views::LayoutAlignment::kCenter);
+  layout->SetCrossAxisAlignment(views::LayoutAlignment::kStretch);
+
+  // Make the effects sections children of the `views::FlexLayoutView`, so that
+  // they scroll (if more effects are present than can fit in the available
+  // height).
+  if (controller_->effects_manager().HasToggleEffects()) {
+    layout->AddChildView(std::make_unique<ToggleEffectsView>(controller_));
+  }
+  if (controller_->effects_manager().HasSetValueEffects()) {
+    layout->AddChildView(std::make_unique<SetValueEffectsView>(controller_));
   }
 
   SetBorder(views::CreateEmptyBorder(

ash/system/video_conference/bubble/bubble_view.h

@@ -22,6 +22,13 @@ class BubbleView : public TrayBubbleView {
   BubbleView(const BubbleView&) = delete;
   BubbleView& operator=(const BubbleView&) = delete;
   ~BubbleView() override = default;
+
+  // views::View:
+  void AddedToWidget() override;
+
+ private:
+  // Unowned by `BubbleView`.
+  VideoConferenceTrayController* controller_;
 };
 
 }  // namespace video_conference

ash/system/video_conference/effects/fake_video_conference_effects.cc

@@ -6,9 +6,11 @@
 
 #include <string>
 
+#include "ash/constants/ash_features.h"
 #include "ash/resources/vector_icons/vector_icons.h"
 #include "ash/strings/grit/ash_strings.h"
 #include "ash/system/video_conference/effects/video_conference_tray_effects_manager_types.h"
+#include "ash/system/video_conference/fake_video_conference_tray_controller.h"
 #include "base/functional/bind.h"
 #include "ui/views/controls/button/button.h"
 
@@ -210,4 +212,57 @@ int SuperCutnessEffect::GetNumActivationsForTesting(int value) {
   return num_activations_for_testing_[value];
 }
 
+// This registers/unregisters all effects owned by `EffectRepository`.
+// Comment-out the `RegisterDelegate`/`UnregisterDelegate` calls for effects
+// that are not needed e.g. to test `ash::video_conference::BubbleView` with
+// only one or two registered effects.
+EffectRepository::EffectRepository(
+    ash::FakeVideoConferenceTrayController* controller)
+    : controller_(controller),
+      cat_ears_(std::make_unique<CatEarsEffect>()),
+      dog_fur_(std::make_unique<DogFurEffect>()),
+      spaceship_(std::make_unique<SpaceshipEffect>()),
+      office_bunny_(std::make_unique<OfficeBunnyEffect>()),
+      calm_forest_(std::make_unique<CalmForestEffect>()),
+      stylish_kitchen_(std::make_unique<StylishKitchenEffect>()),
+      greenhouse_(std::make_unique<GreenhouseEffect>()),
+      shaggy_fur_(std::make_unique<ShaggyFurEffect>()),
+      super_cuteness_(std::make_unique<SuperCutnessEffect>()) {
+  DCHECK(controller_);
+  if (features::IsVcControlsUiFakeEffectsEnabled()) {
+    controller_->effects_manager().RegisterDelegate(cat_ears_.get());
+    controller_->effects_manager().RegisterDelegate(dog_fur_.get());
+    controller_->effects_manager().RegisterDelegate(spaceship_.get());
+    controller_->effects_manager().RegisterDelegate(office_bunny_.get());
+    controller_->effects_manager().RegisterDelegate(calm_forest_.get());
+    controller_->effects_manager().RegisterDelegate(stylish_kitchen_.get());
+    controller_->effects_manager().RegisterDelegate(greenhouse_.get());
+    controller_->effects_manager().RegisterDelegate(shaggy_fur_.get());
+    controller_->effects_manager().RegisterDelegate(super_cuteness_.get());
+  }
+}
+
+EffectRepository::~EffectRepository() {
+  if (features::IsVcControlsUiFakeEffectsEnabled()) {
+    controller_->effects_manager().UnregisterDelegate(cat_ears_.get());
+    cat_ears_.reset();
+    controller_->effects_manager().UnregisterDelegate(dog_fur_.get());
+    dog_fur_.reset();
+    controller_->effects_manager().UnregisterDelegate(spaceship_.get());
+    spaceship_.reset();
+    controller_->effects_manager().UnregisterDelegate(office_bunny_.get());
+    office_bunny_.reset();
+    controller_->effects_manager().UnregisterDelegate(calm_forest_.get());
+    calm_forest_.reset();
+    controller_->effects_manager().UnregisterDelegate(stylish_kitchen_.get());
+    stylish_kitchen_.reset();
+    controller_->effects_manager().UnregisterDelegate(greenhouse_.get());
+    greenhouse_.reset();
+    controller_->effects_manager().UnregisterDelegate(shaggy_fur_.get());
+    shaggy_fur_.reset();
+    controller_->effects_manager().UnregisterDelegate(super_cuteness_.get());
+    super_cuteness_.reset();
+  }
+}
+
 }  // namespace ash::fake_video_conference

ash/system/video_conference/effects/fake_video_conference_effects.h

@@ -15,7 +15,11 @@ namespace gfx {
 struct VectorIcon;
 }  // namespace gfx
 
-namespace ash::fake_video_conference {
+namespace ash {
+
+class FakeVideoConferenceTrayController;
+
+namespace fake_video_conference {
 
 // A convenience base class, for creating a delegate that hosts the simplest
 // type of effect there is i.e. a toggle with only one state.
@@ -181,6 +185,33 @@ class ASH_EXPORT SuperCutnessEffect : public VcEffectsDelegate {
   std::vector<int> num_activations_for_testing_;
 };
 
-}  // namespace ash::fake_video_conference
+// A simple residence for any fake effects used for testing. For all of these
+// fake effects to be registered, the feature `VcControlsUiFakeEffects` must be
+// enabled.
+class EffectRepository {
+ public:
+  explicit EffectRepository(FakeVideoConferenceTrayController* controller);
+
+  EffectRepository(const EffectRepository&) = delete;
+  EffectRepository& operator=(const EffectRepository&) = delete;
+
+  ~EffectRepository();
+
+ private:
+  FakeVideoConferenceTrayController* controller_;
+  std::unique_ptr<CatEarsEffect> cat_ears_;
+  std::unique_ptr<DogFurEffect> dog_fur_;
+  std::unique_ptr<SpaceshipEffect> spaceship_;
+  std::unique_ptr<OfficeBunnyEffect> office_bunny_;
+  std::unique_ptr<CalmForestEffect> calm_forest_;
+  std::unique_ptr<StylishKitchenEffect> stylish_kitchen_;
+  std::unique_ptr<GreenhouseEffect> greenhouse_;
+  std::unique_ptr<ShaggyFurEffect> shaggy_fur_;
+  std::unique_ptr<SuperCutnessEffect> super_cuteness_;
+};
+
+}  // namespace fake_video_conference
+
+}  // namespace ash
 
 #endif  // ASH_SYSTEM_VIDEO_CONFERENCE_EFFECTS_FAKE_VIDEO_CONFERENCE_EFFECTS_H_

ash/system/video_conference/fake_video_conference_tray_controller.cc

@@ -4,11 +4,21 @@
 
 #include "ash/system/video_conference/fake_video_conference_tray_controller.h"
 
+#include "ash/system/video_conference/effects/fake_video_conference_effects.h"
 #include "chromeos/ash/components/audio/cras_audio_handler.h"
 #include "media/capture/video/chromeos/mojom/cros_camera_service.mojom-shared.h"
 
 namespace ash {
 
+FakeVideoConferenceTrayController::FakeVideoConferenceTrayController()
+    : effect_repository_(
+          std::make_unique<fake_video_conference::EffectRepository>(
+              /*controller=*/this)) {}
+
+FakeVideoConferenceTrayController::~FakeVideoConferenceTrayController() {
+  effect_repository_.reset();
+}
+
 void FakeVideoConferenceTrayController::SetCameraMuted(bool muted) {
   camera_muted_ = muted;
   OnCameraSWPrivacySwitchStateChanged(

ash/system/video_conference/fake_video_conference_tray_controller.h

@@ -10,19 +10,23 @@
 
 namespace ash {
 
+namespace fake_video_conference {
+class EffectRepository;
+}
+
 // A fake version of VideoConferenceTrayController that will be use in tests or
 // mocking in the emulator.
 class ASH_EXPORT FakeVideoConferenceTrayController
     : public VideoConferenceTrayController {
  public:
-  FakeVideoConferenceTrayController() = default;
+  FakeVideoConferenceTrayController();
 
   FakeVideoConferenceTrayController(const FakeVideoConferenceTrayController&) =
       delete;
   FakeVideoConferenceTrayController& operator=(
       const FakeVideoConferenceTrayController&) = delete;
 
-  ~FakeVideoConferenceTrayController() override = default;
+  ~FakeVideoConferenceTrayController() override;
 
   // VideoConferenceTrayController:
   void SetCameraMuted(bool muted) override;
@@ -35,6 +39,9 @@ class ASH_EXPORT FakeVideoConferenceTrayController
   // Indicates whether camera/microphone is muted.
   bool camera_muted_ = false;
   bool microphone_muted_ = false;
+
+  // General-purpose repository for fake effects.
+  std::unique_ptr<fake_video_conference::EffectRepository> effect_repository_;
 };
 
 }  // namespace ash