Add content setting icons for PiP 2.0 window

The UI mocks requires the camera & microphone icon only, but since many testing machines do not have camera installed for testing, we also add the geo location icon to test similar permissions. Now that the permission can be updated in both the normal browser window and the PiP window, sync the content settings in both places for the icons to show the right status. Add a PictureInPictureWindowManager::GetChildWebContents() API to get the document PiP window web contents for syncing purposes. Bug: 1346734 Change-Id: I4a5a912ffa101470d0b938733f4bb17a5b262410 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3894815 Reviewed-by: Fr <beaufort.francois@gmail.com> Reviewed-by: Balazs Engedy <engedy@chromium.org> Reviewed-by: Avi Drissman <avi@chromium.org> Commit-Queue: Yiren Wang <yrw@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Cr-Commit-Position: refs/heads/main@{#1055137}
chromium · Oct 5, 2022 · 2699997 · 2699997
1 parent ec3557b
commit 2699997
Show file tree

Hide file tree

Showing 27 changed files with 286 additions and 17 deletions.
diff --git a/chrome/browser/DEPS b/chrome/browser/DEPS
@@ -491,6 +491,8 @@ include_rules = [
   "+chrome/browser/ui/views/chrome_browser_main_extra_parts_views_linux.h",
   "+chrome/browser/ui/views/chrome_browser_main_extra_parts_views_linux_x11.h",
   "+chrome/browser/ui/views/extensions/request_file_system_dialog_view.h",
+  "+chrome/browser/ui/views/frame/browser_view.h",
+  "+chrome/browser/ui/views/frame/picture_in_picture_browser_frame_view.h",
   "+chrome/browser/ui/views/notifications/request_pin_view_chromeos.h",
   "+chrome/browser/ui/views/try_chrome_dialog_win/try_chrome_dialog.h",
 

diff --git a/chrome/browser/ash/arc/pip/arc_picture_in_picture_window_controller_impl.cc b/chrome/browser/ash/arc/pip/arc_picture_in_picture_window_controller_impl.cc
@@ -46,4 +46,9 @@ ArcPictureInPictureWindowControllerImpl::GetWebContents() {
   return nullptr;
 }
 
+content::WebContents*
+ArcPictureInPictureWindowControllerImpl::GetChildWebContents() {
+  return nullptr;
+}
+
 }  // namespace arc
diff --git a/chrome/browser/ash/arc/pip/arc_picture_in_picture_window_controller_impl.h b/chrome/browser/ash/arc/pip/arc_picture_in_picture_window_controller_impl.h
@@ -40,6 +40,7 @@ class ArcPictureInPictureWindowControllerImpl
   void CloseAndFocusInitiator() override;
   void OnWindowDestroyed(bool should_pause_video) override;
   content::WebContents* GetWebContents() override;
+  content::WebContents* GetChildWebContents() override;
 
  private:
   arc::ArcPipBridge* const arc_pip_bridge_;

diff --git a/chrome/browser/content_settings/chrome_content_settings_utils.cc b/chrome/browser/content_settings/chrome_content_settings_utils.cc
@@ -13,6 +13,8 @@
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/location_bar/location_bar.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/views/frame/browser_view.h"
+#include "chrome/browser/ui/views/frame/picture_in_picture_browser_frame_view.h"
 #include "content/public/browser/web_contents.h"
 #endif
 
@@ -40,6 +42,19 @@ void UpdateLocationBarUiForWebContents(content::WebContents* web_contents) {
   LocationBar* location_bar = browser->window()->GetLocationBar();
   if (location_bar)
     location_bar->UpdateContentSettingsIcons();
+
+// TODO(https://crbug.com/1346734): Enable this on all platforms.
+#if BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS)
+  // The document PiP window does not have a location bar, but has some content
+  // setting views that need to be updated too.
+  if (browser->is_type_picture_in_picture()) {
+    BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser);
+    auto* frame_view = static_cast<PictureInPictureBrowserFrameView*>(
+        browser_view->frame()->GetFrameView());
+    frame_view->UpdateContentSettingsIcons();
+  }
+#endif
+
 #endif
 }
 

diff --git a/chrome/browser/content_settings/page_specific_content_settings_delegate.cc b/chrome/browser/content_settings/page_specific_content_settings_delegate.cc
@@ -14,6 +14,7 @@
 #include "chrome/browser/media/webrtc/media_capture_devices_dispatcher.h"
 #include "chrome/browser/media/webrtc/media_stream_capture_indicator.h"
 #include "chrome/browser/permissions/permission_decision_auto_blocker_factory.h"
+#include "chrome/browser/picture_in_picture/picture_in_picture_window_manager.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
@@ -205,6 +206,29 @@ PageSpecificContentSettingsDelegate::GetMicrophoneCameraState() {
   return state;
 }
 
+content::WebContents* PageSpecificContentSettingsDelegate::
+    MaybeGetSyncedWebContentsForPictureInPicture(
+        content::WebContents* web_contents) {
+  DCHECK(web_contents);
+  content::WebContents* parent_web_contents =
+      PictureInPictureWindowManager::GetInstance()->GetWebContents();
+  content::WebContents* child_web_contents =
+      PictureInPictureWindowManager::GetInstance()->GetChildWebContents();
+
+  // For document picture-in-picture window, return the opener web contents.
+  if (web_contents == child_web_contents) {
+    DCHECK(parent_web_contents);
+    return parent_web_contents;
+  }
+
+  // For browser window that has opened a document picture-in-picture window,
+  // return the PiP window web contents.
+  if ((web_contents == parent_web_contents) && child_web_contents) {
+    return child_web_contents;
+  }
+  return nullptr;
+}
+
 void PageSpecificContentSettingsDelegate::OnContentAllowed(
     ContentSettingsType type) {
   if (!(type == ContentSettingsType::GEOLOCATION ||

diff --git a/chrome/browser/content_settings/page_specific_content_settings_delegate.h b/chrome/browser/content_settings/page_specific_content_settings_delegate.h
@@ -85,6 +85,8 @@ class PageSpecificContentSettingsDelegate
       const std::string& media_stream_selected_video_device) override;
   content_settings::PageSpecificContentSettings::MicrophoneCameraState
   GetMicrophoneCameraState() override;
+  content::WebContents* MaybeGetSyncedWebContentsForPictureInPicture(
+      content::WebContents* web_contents) override;
   void OnContentAllowed(ContentSettingsType type) override;
   void OnContentBlocked(ContentSettingsType type) override;
   void OnStorageAccessAllowed(StorageType storage_type,

diff --git a/...e/browser/picture_in_picture/document_picture_in_picture_window_controller_browsertest.cc b/...e/browser/picture_in_picture/document_picture_in_picture_window_controller_browsertest.cc
@@ -200,6 +200,7 @@ IN_PROC_BROWSER_TEST_F(DocumentPictureInPictureWindowControllerBrowserTest,
                        MAYBE_CreateTwice) {
   LoadTabAndEnterPictureInPicture(browser());
 
+  ASSERT_TRUE(window_controller()->GetWebContents());
   ASSERT_TRUE(window_controller()->GetChildWebContents());
   DestructionObserver w(window_controller()->GetChildWebContents());
 

diff --git a/chrome/browser/picture_in_picture/picture_in_picture_window_manager.cc b/chrome/browser/picture_in_picture/picture_in_picture_window_manager.cc
@@ -117,13 +117,21 @@ void PictureInPictureWindowManager::ExitPictureInPicture() {
     CloseWindowInternal();
 }
 
-content::WebContents* PictureInPictureWindowManager::GetWebContents() {
+content::WebContents* PictureInPictureWindowManager::GetWebContents() const {
   if (!pip_window_controller_)
     return nullptr;
 
   return pip_window_controller_->GetWebContents();
 }
 
+content::WebContents* PictureInPictureWindowManager::GetChildWebContents()
+    const {
+  if (!pip_window_controller_)
+    return nullptr;
+
+  return pip_window_controller_->GetChildWebContents();
+}
+
 void PictureInPictureWindowManager::CreateWindowInternal(
     content::WebContents* web_contents) {
   video_web_contents_observer_ =

diff --git a/chrome/browser/picture_in_picture/picture_in_picture_window_manager.h b/chrome/browser/picture_in_picture/picture_in_picture_window_manager.h
@@ -54,7 +54,12 @@ class PictureInPictureWindowManager {
 
   void ExitPictureInPicture();
 
-  content::WebContents* GetWebContents();
+  // Gets the web contents in the opener browser window.
+  content::WebContents* GetWebContents() const;
+
+  // Gets the web contents in the PiP window. This only applies to document PiP
+  // and will be null for video PiP.
+  content::WebContents* GetChildWebContents() const;
 
  private:
   friend struct base::DefaultSingletonTraits<PictureInPictureWindowManager>;

diff --git a/chrome/browser/picture_in_picture/video_picture_in_picture_window_controller_browsertest.cc b/chrome/browser/picture_in_picture/video_picture_in_picture_window_controller_browsertest.cc
@@ -98,6 +98,7 @@ class MockVideoPictureInPictureWindowController
   MOCK_METHOD0(UpdateLayerBounds, void());
   MOCK_METHOD0(IsPlayerActive, bool());
   MOCK_METHOD0(GetWebContents, content::WebContents*());
+  MOCK_METHOD0(GetChildWebContents, content::WebContents*());
   MOCK_METHOD0(TogglePlayPause, bool());
   MOCK_METHOD0(SkipAd, void());
   MOCK_METHOD0(NextTrack, void());
@@ -1239,6 +1240,12 @@ IN_PROC_BROWSER_TEST_F(VideoPictureInPictureWindowControllerBrowserTest,
   // Show the non-WebContents based Picture-in-Picture window controller.
   EXPECT_CALL(mock_controller(), Show());
   pip_window_manager->EnterPictureInPictureWithController(&mock_controller());
+
+  EXPECT_CALL(mock_controller(), GetWebContents());
+  pip_window_manager->GetWebContents();
+
+  EXPECT_CALL(mock_controller(), GetChildWebContents());
+  pip_window_manager->GetChildWebContents();
 }
 
 IN_PROC_BROWSER_TEST_F(VideoPictureInPictureWindowControllerBrowserTest,

diff --git a/chrome/browser/ui/views/bubble_anchor_util_views.cc b/chrome/browser/ui/views/bubble_anchor_util_views.cc
@@ -27,12 +27,15 @@ AnchorConfiguration GetPageInfoAnchorConfiguration(Browser* browser,
             browser_view->GetLocationBarView()->location_icon_view(),
             views::BubbleBorder::TOP_LEFT};
 
+// TODO(https://crbug.com/1346734): Enable this on all platforms.
+#if BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS)
   if (anchor == kLocationBar && browser_view->GetIsPictureInPictureType()) {
     auto* frame_view = static_cast<PictureInPictureBrowserFrameView*>(
         browser_view->frame()->GetFrameView());
     return {frame_view->GetLocationIconView(),
             frame_view->GetLocationIconView(), views::BubbleBorder::TOP_LEFT};
   }
+#endif
 
   if (anchor == kCustomTabBar && browser_view->toolbar()->custom_tab_bar())
     return {browser_view->toolbar()->custom_tab_bar(),

diff --git a/chrome/browser/ui/views/frame/picture_in_picture_browser_frame_view.cc b/chrome/browser/ui/views/frame/picture_in_picture_browser_frame_view.cc
@@ -5,6 +5,8 @@
 #include "chrome/browser/ui/views/frame/picture_in_picture_browser_frame_view.h"
 #include "chrome/app/vector_icons/vector_icons.h"
 #include "chrome/browser/picture_in_picture/picture_in_picture_window_manager.h"
+#include "chrome/browser/ui/browser_content_setting_bubble_model_delegate.h"
+#include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/color/chrome_color_id.h"
 #include "chrome/browser/ui/views/chrome_typography.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
@@ -104,6 +106,23 @@ PictureInPictureBrowserFrameView::PictureInPictureBrowserFrameView(
           .Build());
   controls_container_view_->SetFlexForView(window_title_, 1);
 
+  // Creates the content setting models. Currently we only support geo location
+  // and camera and microphone settings.
+  constexpr ContentSettingImageModel::ImageType kContentSettingImageOrder[] = {
+      ContentSettingImageModel::ImageType::GEOLOCATION,
+      ContentSettingImageModel::ImageType::MEDIASTREAM};
+  std::vector<std::unique_ptr<ContentSettingImageModel>> models;
+  for (auto type : kContentSettingImageOrder)
+    models.push_back(ContentSettingImageModel::CreateForContentType(type));
+
+  // Creates the content setting views.
+  for (auto& model : models) {
+    auto image_view = std::make_unique<ContentSettingImageView>(
+        std::move(model), this, this, font_list);
+    content_setting_views_.push_back(
+        controls_container_view_->AddChildView(std::move(image_view)));
+  }
+
   // Creates the back to tab button.
   back_to_tab_button_ = controls_container_view_->AddChildView(
       std::make_unique<BackToTabButton>(base::BindRepeating(
@@ -163,6 +182,11 @@ int PictureInPictureBrowserFrameView::NonClientHitTest(
       GetCloseControlsBounds().Contains(point))
     return HTCLIENT;
 
+  for (size_t i = 0; i < content_setting_views_.size(); i++) {
+    if (GetContentSettingViewBounds(i).Contains(point))
+      return HTCLIENT;
+  }
+
   // Allow dragging and resizing the window.
   int window_component = GetHTComponentForFrame(
       point, gfx::Insets(kWindowBorderThickness), kResizeAreaCornerSize,
@@ -204,6 +228,8 @@ void PictureInPictureBrowserFrameView::OnThemeChanged() {
   controls_container_view_->SetBackground(views::CreateSolidBackground(
       SkColorSetA(color_provider->GetColor(kColorPipWindowControlsBackground),
                   SK_AlphaOPAQUE)));
+  for (ContentSettingImageView* view : content_setting_views_)
+    view->SetIconColor(color_provider->GetColor(kColorOmniboxResultsIcon));
 }
 
 void PictureInPictureBrowserFrameView::Layout() {
@@ -298,27 +324,74 @@ SkColor PictureInPictureBrowserFrameView::GetIconLabelBubbleBackgroundColor()
   return GetColorProvider()->GetColor(kColorLocationBarBackground);
 }
 
+///////////////////////////////////////////////////////////////////////////////
+// ContentSettingImageView::Delegate implementations:
+
+bool PictureInPictureBrowserFrameView::ShouldHideContentSettingImage() {
+  return false;
+}
+
+content::WebContents*
+PictureInPictureBrowserFrameView::GetContentSettingWebContents() {
+  // Use the opener web contents for content settings since it has full info
+  // such as last committed URL, etc. that are called to be used.
+  return GetWebContents();
+}
+
+ContentSettingBubbleModelDelegate*
+PictureInPictureBrowserFrameView::GetContentSettingBubbleModelDelegate() {
+  // Use the opener browser delegate to open any new tab.
+  Browser* browser = chrome::FindBrowserWithWebContents(GetWebContents());
+  return browser->content_setting_bubble_model_delegate();
+}
+
+///////////////////////////////////////////////////////////////////////////////
+// GeolocationManager::PermissionObserver implementations:
+void PictureInPictureBrowserFrameView::OnSystemPermissionUpdated(
+    device::LocationSystemPermissionStatus new_status) {
+  // Update icons if the macOS location permission is updated.
+  UpdateContentSettingsIcons();
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // PictureInPictureBrowserFrameView implementations:
+gfx::Rect PictureInPictureBrowserFrameView::ConvertControlViewBounds(
+    views::View* control_view) const {
+  gfx::RectF bounds(control_view->GetMirroredBounds());
+  views::View::ConvertRectToTarget(controls_container_view_, this, &bounds);
+  return gfx::ToEnclosingRect(bounds);
+}
 
 gfx::Rect PictureInPictureBrowserFrameView::GetLocationIconViewBounds() const {
   DCHECK(location_icon_view_);
-  return location_icon_view_->GetMirroredBounds();
+  return ConvertControlViewBounds(location_icon_view_);
+}
+
+gfx::Rect PictureInPictureBrowserFrameView::GetContentSettingViewBounds(
+    size_t index) const {
+  DCHECK(index < content_setting_views_.size());
+  return ConvertControlViewBounds(content_setting_views_[index]);
 }
 
 gfx::Rect PictureInPictureBrowserFrameView::GetBackToTabControlsBounds() const {
   DCHECK(back_to_tab_button_);
-  return back_to_tab_button_->GetMirroredBounds();
+  return ConvertControlViewBounds(back_to_tab_button_);
 }
 
 gfx::Rect PictureInPictureBrowserFrameView::GetCloseControlsBounds() const {
   DCHECK(close_image_button_);
-  return close_image_button_->GetMirroredBounds();
+  return ConvertControlViewBounds(close_image_button_);
 }
 
 LocationIconView* PictureInPictureBrowserFrameView::GetLocationIconView() {
   return location_icon_view_;
 }
 
+void PictureInPictureBrowserFrameView::UpdateContentSettingsIcons() {
+  for (auto* view : content_setting_views_) {
+    view->Update();
+  }
+}
+
 BEGIN_METADATA(PictureInPictureBrowserFrameView, BrowserNonClientFrameView)
 END_METADATA
diff --git a/chrome/browser/ui/views/frame/picture_in_picture_browser_frame_view.h b/chrome/browser/ui/views/frame/picture_in_picture_browser_frame_view.h
@@ -8,10 +8,12 @@
 #include "chrome/browser/ui/toolbar/chrome_location_bar_model_delegate.h"
 #include "chrome/browser/ui/views/frame/browser_frame.h"
 #include "chrome/browser/ui/views/frame/browser_non_client_frame_view.h"
+#include "chrome/browser/ui/views/location_bar/content_setting_image_view.h"
 #include "chrome/browser/ui/views/location_bar/location_icon_view.h"
 #include "chrome/browser/ui/views/overlay/close_image_button.h"
 #include "components/omnibox/browser/location_bar_model.h"
 #include "content/public/browser/web_contents.h"
+#include "services/device/public/cpp/geolocation/geolocation_manager.h"
 #include "ui/base/metadata/metadata_header_macros.h"
 #include "ui/views/controls/image_view.h"
 #include "ui/views/layout/box_layout_view.h"
@@ -20,10 +22,13 @@ namespace views {
 class Label;
 }
 
-class PictureInPictureBrowserFrameView : public BrowserNonClientFrameView,
-                                         public ChromeLocationBarModelDelegate,
-                                         public LocationIconView::Delegate,
-                                         public IconLabelBubbleView::Delegate {
+class PictureInPictureBrowserFrameView
+    : public BrowserNonClientFrameView,
+      public ChromeLocationBarModelDelegate,
+      public LocationIconView::Delegate,
+      public IconLabelBubbleView::Delegate,
+      public ContentSettingImageView::Delegate,
+      public device::GeolocationManager::PermissionObserver {
  public:
   METADATA_HEADER(PictureInPictureBrowserFrameView);
 
@@ -74,13 +79,31 @@ class PictureInPictureBrowserFrameView : public BrowserNonClientFrameView,
   SkColor GetIconLabelBubbleSurroundingForegroundColor() const override;
   SkColor GetIconLabelBubbleBackgroundColor() const override;
 
+  // ContentSettingImageView::Delegate:
+  bool ShouldHideContentSettingImage() override;
+  content::WebContents* GetContentSettingWebContents() override;
+  ContentSettingBubbleModelDelegate* GetContentSettingBubbleModelDelegate()
+      override;
+
+  // GeolocationManager::PermissionObserver:
+  void OnSystemPermissionUpdated(
+      device::LocationSystemPermissionStatus new_status) override;
+
+  // Convert the bounds of a child view of |controls_container_view_| to use
+  // the system's coordinate system.
+  gfx::Rect ConvertControlViewBounds(views::View* control_view) const;
+
   // Gets the bounds of the controls.
   gfx::Rect GetLocationIconViewBounds() const;
+  gfx::Rect GetContentSettingViewBounds(size_t index) const;
   gfx::Rect GetBackToTabControlsBounds() const;
   gfx::Rect GetCloseControlsBounds() const;
 
   LocationIconView* GetLocationIconView();
 
+  // Updates the state of the images showing the content settings status.
+  void UpdateContentSettingsIcons();
+
  private:
   // A model required to use LocationIconView.
   std::unique_ptr<LocationBarModel> location_bar_model_;
@@ -94,6 +117,10 @@ class PictureInPictureBrowserFrameView : public BrowserNonClientFrameView,
   raw_ptr<LocationIconView> location_icon_view_ = nullptr;
 
   raw_ptr<views::Label> window_title_ = nullptr;
+
+  // The content setting views for icons and bubbles.
+  std::vector<ContentSettingImageView*> content_setting_views_;
+
   raw_ptr<CloseImageButton> close_image_button_ = nullptr;
   raw_ptr<views::View> back_to_tab_button_ = nullptr;
 };