Window Placement: Gate screenschange events on permission

Only fire screenschange for frames with the Window Placement permission.

Fire isMultiScreen changes for all visible frames (e.g. 1 <-> 2 screens)
(obviates pollling isMultiScreen(), which is exposed without permission)
Add a TODO to postpone events instead of dropping, and refine criteria.

Update test expectations and add TODO to test events with permission.

Bug: 1080690, 1109989
Test: event only fires for permissioned sites or isMultiScreen changes.
Change-Id: I5b46bd6944bc4b923b6357e9b370c80ce14a09b9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2375671Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#802356}

content/browser/screen_enumeration/screen_change_monitor.cc

@@ -9,7 +9,8 @@
 
 namespace content {
 
-ScreenChangeMonitor::ScreenChangeMonitor(base::RepeatingClosure callback)
+ScreenChangeMonitor::ScreenChangeMonitor(
+    base::RepeatingCallback<void(bool)> callback)
     : callback_(callback) {
 // TODO(crbug.com/1071233): Investigate test failures (crashes?) on Fuchsia.
 #if !defined(OS_FUCHSIA)
@@ -31,8 +32,10 @@ void ScreenChangeMonitor::OnScreensChange() {
     if (cached_displays_ == displays)
       return;
 
+    const bool is_multi_screen_changed =
+        (cached_displays_.size() > 1) != (displays.size() > 1);
     cached_displays_ = displays;
-    callback_.Run();
+    callback_.Run(is_multi_screen_changed);
   }
 }
 

content/browser/screen_enumeration/screen_change_monitor.h

@@ -13,7 +13,9 @@ namespace content {
 // Monitors system screen information and runs a callback on changes.
 class ScreenChangeMonitor : public display::DisplayObserver {
  public:
-  explicit ScreenChangeMonitor(base::RepeatingClosure callback);
+  // The callback is run on any screen changes; the bool parameter is true iff
+  // the plurality of connected screens changed (e.g. 1 screen <-> 2 screens).
+  explicit ScreenChangeMonitor(base::RepeatingCallback<void(bool)> callback);
   ~ScreenChangeMonitor() override;
 
   ScreenChangeMonitor(const ScreenChangeMonitor&) = delete;
@@ -30,7 +32,7 @@ class ScreenChangeMonitor : public display::DisplayObserver {
                                uint32_t changed_metrics) override;
 
   // The callback to run on screen change events.
-  base::RepeatingClosure callback_;
+  base::RepeatingCallback<void(bool)> callback_;
 
   // The most recent display information, cached to detect meaningful changes.
   std::vector<display::Display> cached_displays_;

content/browser/screen_enumeration/screen_enumeration_browsertest.cc

@@ -200,11 +200,14 @@ IN_PROC_BROWSER_TEST_F(FakeScreenEnumerationTest, MAYBE_IsMultiScreenFaked) {
 // TODO(crbug.com/1042990): Windows crashes static casting to ScreenWin.
 // TODO(crbug.com/1042990): Android requires a GetDisplayNearestView overload.
 #if defined(OS_ANDROID) || defined(OS_WIN)
-#define MAYBE_OnScreensChange DISABLED_OnScreensChange
+#define MAYBE_OnScreensChangeNoPermission DISABLED_OnScreensChangeNoPermission
 #else
-#define MAYBE_OnScreensChange OnScreensChange
+#define MAYBE_OnScreensChangeNoPermission OnScreensChangeNoPermission
 #endif
-IN_PROC_BROWSER_TEST_F(FakeScreenEnumerationTest, MAYBE_OnScreensChange) {
+// Sites with no permission only get an event if isMultiScreen() changes.
+// TODO(crbug.com/1119974): Need content_browsertests permission controls.
+IN_PROC_BROWSER_TEST_F(FakeScreenEnumerationTest,
+                       MAYBE_OnScreensChangeNoPermission) {
   ASSERT_TRUE(NavigateToURL(test_shell(), GetTestUrl(nullptr, "empty.html")));
   ASSERT_EQ(true,
             EvalJs(test_shell()->web_contents(), "'onscreenschange' in self"));
@@ -214,20 +217,34 @@ IN_PROC_BROWSER_TEST_F(FakeScreenEnumerationTest, MAYBE_OnScreensChange) {
   )";
   EXPECT_EQ(0, EvalJs(test_shell()->web_contents(), kSetOnScreensChange));
 
+  // isMultiScreen() changes from false to true here, so an event is sent.
+  EXPECT_EQ(false, EvalJs(test_shell()->web_contents(), kIsMultiScreenScript));
   screen()->display_list().AddDisplay({1, gfx::Rect(100, 100, 801, 802)},
-                                      display::DisplayList::Type::PRIMARY);
+                                      display::DisplayList::Type::NOT_PRIMARY);
+  EXPECT_EQ(true, EvalJs(test_shell()->web_contents(), kIsMultiScreenScript));
   EXPECT_EQ("1", EvalJs(test_shell()->web_contents(), "document.title"));
 
+  // isMultiScreen() remains unchanged, so no event is sent.
   screen()->display_list().AddDisplay({2, gfx::Rect(901, 100, 801, 802)},
                                       display::DisplayList::Type::NOT_PRIMARY);
-  EXPECT_EQ("2", EvalJs(test_shell()->web_contents(), "document.title"));
+  EXPECT_EQ(true, EvalJs(test_shell()->web_contents(), kIsMultiScreenScript));
+  EXPECT_EQ("1", EvalJs(test_shell()->web_contents(), "document.title"));
 
+  // isMultiScreen() remains unchanged, so no event is sent.
   EXPECT_NE(0u, screen()->display_list().UpdateDisplay(
                     {2, gfx::Rect(902, 100, 801, 802)}));
-  EXPECT_EQ("3", EvalJs(test_shell()->web_contents(), "document.title"));
+  EXPECT_EQ(true, EvalJs(test_shell()->web_contents(), kIsMultiScreenScript));
+  EXPECT_EQ("1", EvalJs(test_shell()->web_contents(), "document.title"));
 
+  // isMultiScreen() remains unchanged, so no event is sent.
   screen()->display_list().RemoveDisplay(2);
-  EXPECT_EQ("4", EvalJs(test_shell()->web_contents(), "document.title"));
+  EXPECT_EQ(true, EvalJs(test_shell()->web_contents(), kIsMultiScreenScript));
+  EXPECT_EQ("1", EvalJs(test_shell()->web_contents(), "document.title"));
+
+  // isMultiScreen() changes from true to false here, so an event is sent.
+  screen()->display_list().RemoveDisplay(1);
+  EXPECT_EQ(false, EvalJs(test_shell()->web_contents(), kIsMultiScreenScript));
+  EXPECT_EQ("2", EvalJs(test_shell()->web_contents(), "document.title"));
 }
 
 }  // namespace content

content/browser/web_contents/web_contents_impl.cc

@@ -1284,10 +1284,19 @@ WebContentsView* WebContentsImpl::GetView() const {
   return view_.get();
 }
 
-void WebContentsImpl::OnScreensChange() {
+void WebContentsImpl::OnScreensChange(bool is_multi_screen_changed) {
+  // Send |is_multi_screen_changed| events to all visible frames, but limit
+  // other events to frames with the Window Placement permission. This obviates
+  // the most pressing need for sites to poll isMultiScreen(), which is exposed
+  // without explicit permission, while also protecting privacy.
+  // TODO(crbug.com/1109989): Postpone events; refine utility/privacy balance.
   for (FrameTreeNode* node : frame_tree_.Nodes()) {
     RenderFrameHostImpl* rfh = node->current_frame_host();
-    rfh->GetAssociatedLocalFrame()->OnScreensChange();
+    if ((is_multi_screen_changed &&
+         rfh->GetVisibilityState() == PageVisibilityState::kVisible) ||
+        WindowPlacementGranted(rfh)) {
+      rfh->GetAssociatedLocalFrame()->OnScreensChange();
+    }
   }
 }
 

content/browser/web_contents/web_contents_impl.h

@@ -261,7 +261,10 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents,
 
   WebContentsView* GetView() const;
 
-  void OnScreensChange();
+  // Called on screen information changes; |is_multi_screen_changed| is true iff
+  // the plurality of connected screens changed (e.g. 1 screen <-> 2 screens).
+  void OnScreensChange(bool is_multi_screen_changed);
+
   void OnScreenOrientationChange();
 
   ScreenOrientationProvider* GetScreenOrientationProviderForTesting() const {