camera: Pass internal screen's rotation angle to VCD delegate directly

Currently ScreenObserverDelegate pass a reference of Display to VCD delegate when screen's event happens. However, it is not safe to pass reference if we can't control its lifetime. In this case, copy a Display object and pass it to the task runner is much safer. Moreover, since VCD delegate only need screen's rotation angle, this CL only passed the angle to task runner. Bug: b:281837116 Test: Rotation works well when rotating screen in tablet mode. Test: tast run DUT camera.CCAUIScreenRotate.* Change-Id: Ia3684107c49b95543e54bd31fb154bfa160aa567 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5086898 Commit-Queue: Sean Li <seannli@google.com> Reviewed-by: Kam Kwankajornkiet <kamchonlathorn@chromium.org> Cr-Commit-Position: refs/heads/main@{#1236941}
qweri0p · Dec 13, 2023 · 73ad199 · 73ad199
1 parent 163be2d
commit 73ad199
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 26 deletions.
diff --git a/media/capture/video/chromeos/display_rotation_observer.cc b/media/capture/video/chromeos/display_rotation_observer.cc
@@ -51,15 +51,21 @@ void ScreenObserverDelegate::OnDisplayMetricsChanged(
   DCHECK(display_task_runner_->BelongsToCurrentThread());
   if (!(metrics & DISPLAY_METRIC_ROTATION))
     return;
-  SendDisplayRotation(display);
+  if (display.IsInternal()) {
+    SendInternalDisplayRotation(display.rotation() * 90);
+  }
 }
 
 void ScreenObserverDelegate::AddObserverOnDisplayThread() {
   DCHECK(display_task_runner_->BelongsToCurrentThread());
   display::Screen* screen = display::Screen::GetScreen();
-  if (screen) {
-    display_observer_.emplace(this);
-    SendDisplayRotation(screen->GetPrimaryDisplay());
+  if (!screen) {
+    return;
+  }
+  display_observer_.emplace(this);
+  display::Display display = screen->GetPrimaryDisplay();
+  if (display.IsInternal()) {
+    SendInternalDisplayRotation(display.rotation() * 90);
   }
 }
 
@@ -69,21 +75,20 @@ void ScreenObserverDelegate::RemoveObserverOnDisplayThread() {
 }
 
 // Post the screen rotation change from the UI thread to capture thread
-void ScreenObserverDelegate::SendDisplayRotation(
-    const display::Display& display) {
+void ScreenObserverDelegate::SendInternalDisplayRotation(int rotation) {
   DCHECK(display_task_runner_->BelongsToCurrentThread());
   delegate_task_runner_->PostTask(
       FROM_HERE,
       base::BindOnce(
-          &ScreenObserverDelegate::SendDisplayRotationOnCaptureThread, this,
-          display));
+          &ScreenObserverDelegate::SendInternalDisplayRotationOnCaptureThread,
+          this, rotation));
 }
 
-void ScreenObserverDelegate::SendDisplayRotationOnCaptureThread(
-    const display::Display& display) {
+void ScreenObserverDelegate::SendInternalDisplayRotationOnCaptureThread(
+    int rotation) {
   DCHECK(delegate_task_runner_->BelongsToCurrentThread());
   if (observer_)
-    observer_->SetDisplayRotation(display);
+    observer_->SetInternalDisplayRotation(rotation);
 }
 
 }  // namespace media
diff --git a/media/capture/video/chromeos/display_rotation_observer.h b/media/capture/video/chromeos/display_rotation_observer.h
@@ -16,7 +16,7 @@ namespace media {
 
 class DisplayRotationObserver {
  public:
-  virtual void SetDisplayRotation(const display::Display& display) = 0;
+  virtual void SetInternalDisplayRotation(int rotation) = 0;
 };
 
 // Registers itself as an observer at |display::Screen::GetScreen()| and
@@ -53,9 +53,10 @@ class ScreenObserverDelegate
   void AddObserverOnDisplayThread();
   void RemoveObserverOnDisplayThread();
 
-  // Post the screen rotation change from the display thread to capture thread
-  void SendDisplayRotation(const display::Display& display);
-  void SendDisplayRotationOnCaptureThread(const display::Display& display);
+  // Post the screen rotation change of the internal display from the display
+  // thread to capture thread.
+  void SendInternalDisplayRotation(int rotation);
+  void SendInternalDisplayRotationOnCaptureThread(int rotation);
 
   base::WeakPtr<DisplayRotationObserver> observer_;
 

diff --git a/media/capture/video/chromeos/video_capture_device_chromeos_delegate.cc b/media/capture/video/chromeos/video_capture_device_chromeos_delegate.cc
@@ -344,11 +344,10 @@ void VideoCaptureDeviceChromeOSDelegate::CloseDevice(
     power_manager_client_proxy_->UnblockSuspend(unblock_suspend_token);
 }
 
-void VideoCaptureDeviceChromeOSDelegate::SetDisplayRotation(
-    const display::Display& display) {
+void VideoCaptureDeviceChromeOSDelegate::SetInternalDisplayRotation(
+    int rotation) {
   DCHECK(capture_task_runner_->BelongsToCurrentThread());
-  if (display.IsInternal())
-    SetRotation(display.rotation() * 90);
+  SetRotation(rotation);
 }
 
 void VideoCaptureDeviceChromeOSDelegate::SetRotation(int rotation) {

diff --git a/media/capture/video/chromeos/video_capture_device_chromeos_delegate.h b/media/capture/video/chromeos/video_capture_device_chromeos_delegate.h
@@ -18,12 +18,6 @@
 #include "media/capture/video/video_capture_device_descriptor.h"
 #include "media/capture/video_capture_types.h"
 
-namespace display {
-
-class Display;
-
-}  // namespace display
-
 namespace media {
 
 class CameraHalDelegate;
@@ -69,7 +63,7 @@ class CAPTURE_EXPORT VideoCaptureDeviceChromeOSDelegate final
   void CloseDevice(base::UnguessableToken unblock_suspend_token);
 
   // DisplayRotationDelegate implementation.
-  void SetDisplayRotation(const display::Display& display) final;
+  void SetInternalDisplayRotation(int rotation) final;
   void SetRotation(int rotation);
 
   const VideoCaptureDeviceDescriptor device_descriptor_;