Snap the omnibox popup widget window to the pixel boundary

This patch manually snaps the omnibox dropdown popup widget window to
the physical pixel grid. On CrOS the popup is treated as a top level
window which is by default snapped to the root window.

This patch also lazily marks the root windows as snapped. The root
windows are snapped by default so marking them snapped makes sense.

Bug: 843250
Change-Id: I1dd20ea57522e4987d23ba7dcaa3ae891bbc7182
Component: Omnibox popup, desktop window tree host
Reviewed-on: https://chromium-review.googlesource.com/1139078Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Commit-Queue: Malay Keshav <malaykeshav@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577929}

chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc

@@ -45,6 +45,10 @@
 #include "ui/views/widget/widget.h"
 #include "ui/views/window/non_client_view.h"
 
+#if defined(USE_AURA)
+#include "ui/wm/core/window_util.h"
+#endif  // defined(USE_AURA)
+
 namespace {
 
 // Cache the shadow images so that potentially expensive shadow drawing isn't
@@ -194,6 +198,12 @@ class OmniboxPopupContentsView::AutocompletePopupWidget
       animator_->SetTargetBounds(bounds);
     else
       SetBounds(bounds);
+
+#if defined(USE_AURA)
+    // TODO(malaykeshav): Remove this manual snap when we start snapping each
+    // window to its parent window. See https://crbug.com/863268 for more info.
+    wm::SnapWindowToPixelBoundary(GetNativeWindow());
+#endif  // defined(USE_AURA)
   }
 
   void ShowAnimated() {

chrome/browser/ui/views/omnibox/omnibox_popup_contents_view_browsertest.cc

@@ -37,7 +37,9 @@
 #include "ui/views/widget/widget.h"
 
 #if defined(USE_AURA)
+#include "ui/aura/window.h"
 #include "ui/native_theme/native_theme_dark_aura.h"
+#include "ui/wm/core/window_properties.h"
 #endif
 
 #if defined(USE_X11)
@@ -170,6 +172,12 @@ views::Widget* OmniboxPopupContentsViewTest::CreatePopupForTestQuery() {
 IN_PROC_BROWSER_TEST_P(OmniboxPopupContentsViewTest, PopupAlignment) {
   views::Widget* popup = CreatePopupForTestQuery();
 
+#if defined(USE_AURA)
+  popup_view()->UpdatePopupAppearance();
+  EXPECT_TRUE(
+      popup->GetNativeWindow()->GetProperty(wm::kSnapChildrenToPixelBoundary));
+#endif  // defined(USE_AURA)
+
   if (GetParam() == WIDE) {
     EXPECT_EQ(toolbar()->width(), popup->GetRestoredBounds().width());
   } else {

ui/wm/core/window_util.cc

@@ -212,15 +212,31 @@ bool HasTransientAncestor(const aura::Window* window,
 void SnapWindowToPixelBoundary(aura::Window* window) {
   // TODO(malaykeshav): We want to snap each window layer to its parent window
   // layer. See https://crbug.com/863268 for more info.
-  window->SetProperty(wm::kSnapChildrenToPixelBoundary, true);
-  aura::Window* snapped_ancestor = window->parent();
-  while (snapped_ancestor) {
-    if (snapped_ancestor->GetProperty(wm::kSnapChildrenToPixelBoundary)) {
-      ui::SnapLayerToPhysicalPixelBoundary(snapped_ancestor->layer(),
+
+  // Root window is already snapped by default.
+  if (window->IsRootWindow()) {
+    window->SetProperty(wm::kSnapChildrenToPixelBoundary, true);
+    return;
+  }
+
+  aura::Window* ancestor_window = window->parent();
+  while (ancestor_window) {
+    bool is_ancestor_window_snapped =
+        ancestor_window->GetProperty(wm::kSnapChildrenToPixelBoundary);
+
+    // Root windows are already snapped by default. Just mark them as snapped.
+    if (ancestor_window->IsRootWindow() && !is_ancestor_window_snapped) {
+      ancestor_window->SetProperty(wm::kSnapChildrenToPixelBoundary, true);
+      is_ancestor_window_snapped = true;
+    }
+
+    if (is_ancestor_window_snapped) {
+      window->SetProperty(wm::kSnapChildrenToPixelBoundary, true);
+      ui::SnapLayerToPhysicalPixelBoundary(ancestor_window->layer(),
                                            window->layer());
       return;
     }
-    snapped_ancestor = snapped_ancestor->parent();
+    ancestor_window = ancestor_window->parent();
   }
 }
 

ui/wm/core/window_util_unittest.cc

@@ -12,6 +12,7 @@
 #include "ui/aura/window.h"
 #include "ui/compositor/layer.h"
 #include "ui/compositor/layer_tree_owner.h"
+#include "ui/wm/core/window_properties.h"
 
 namespace wm {
 
@@ -99,4 +100,22 @@ TEST_F(WindowUtilTest, RecreateLayersWithClosure) {
   EXPECT_EQ(window12->layer(), window1->layer()->children()[1]);
 }
 
+// Test if the root window is always snapped.
+TEST_F(WindowUtilTest, CheckRootWindowAlwaysSnapped) {
+  std::unique_ptr<aura::Window> window11(
+      aura::test::CreateTestWindowWithId(1, root_window()));
+  std::unique_ptr<aura::Window> window12(
+      aura::test::CreateTestWindowWithId(2, root_window()));
+
+  EXPECT_TRUE(root_window()->IsRootWindow());
+
+  wm::SnapWindowToPixelBoundary(window12.get());
+
+  // Root window is always marked as snapped.
+  EXPECT_TRUE(root_window()->GetProperty(wm::kSnapChildrenToPixelBoundary));
+
+  EXPECT_TRUE(window12->GetProperty(wm::kSnapChildrenToPixelBoundary));
+  EXPECT_FALSE(window11->GetProperty(wm::kSnapChildrenToPixelBoundary));
+}
+
 }  // namespace wm