MacViews: Fix window dragging on Sierra.

ui/views/cocoa/bridged_native_widget_interactive_uitest.mm

Index: ui/views/cocoa/bridged_native_widget_interactive_uitest.mm
diff --git a/ui/views/cocoa/bridged_native_widget_interactive_uitest.mm b/ui/views/cocoa/bridged_native_widget_interactive_uitest.mm
index 9dc032a2ffe76b19538048e2c244138cf16c16fa..bce3cf2b021f22f380872048ed6ac3796dd6a2da 100644
--- a/ui/views/cocoa/bridged_native_widget_interactive_uitest.mm
+++ b/ui/views/cocoa/bridged_native_widget_interactive_uitest.mm
@@ -198,9 +198,8 @@ class HitTestBridgedNativeWidget : public BridgedNativeWidget {
       : BridgedNativeWidget(widget) {}
 
   // BridgedNativeWidget:
-  bool ShouldRepostPendingLeftMouseDown(NSPoint location_in_window) override {
-    did_repost_ = BridgedNativeWidget::ShouldRepostPendingLeftMouseDown(
-        location_in_window);
+  bool ShouldRepostPendingLeftMouseDown(NSEvent* event) override {
+    did_repost_ = BridgedNativeWidget::ShouldRepostPendingLeftMouseDown(event);
 
     if (run_loop_)
       run_loop_->Quit();
@@ -323,15 +322,28 @@ TEST_F(BridgedNativeWidgetUITest, HitTest) {
   init_params.ownership = Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
   init_params.bounds = gfx::Rect(100, 200, 400, 300);
   widget.Init(init_params);
+
+  WidgetActivationWaiter activation_waiter(&widget, true);
   widget.Show();
+  activation_waiter.Wait();
 
   // Points inside the resize area.
   const NSPoint bottom_right_point = {398, 2};
   const NSPoint right_of_bottom_right = {398 + 10, 2};
 
   NSWindow* window = widget.GetNativeWindow();
+
+  // This is required since BridgedNativeWidget::SetVisibilityState initially
+  // suppresses visibility and ignores mouse events for opaque non modal
+  // dialogs. For the test, undo these changes.
+  [window setIgnoresMouseEvents:NO];

[karandeepb, 2016/11/08 02:59:03]

What would be a good workaround to avoid this, since this may bite us while
adding tests in future as well. One alternative is to not do the visibility
suppression for tests using the UNIT_TEST macro.

[tapted, 2016/11/08 05:03:17]

I think if WindowResizeHelperMac::Get()->task_runner() returns null then we
shouldn't do the delayed-visibility  stuff.

[karandeepb, 2016/11/09 04:44:20]

Weirdly, even after commenting this out, I can't seem to remove the [window
setIgnoresMouseEvents: NO] call in the test. That is, even with an initial value
of NO for ignoresMouseEvents, I still need the [window setIgnoresMouseEvents:
NO] call in the test. So somehow the call is having some side effect, which is
necessary for the test. This is also true for making it pass on master.

I found this
http://www.cocoabuilder.com/archive/cocoa/313246-transparent-window-not-getti...
but the call is still needed when the window is opaque and has alpha value of 1.

[tapted, 2016/11/09 05:06:56]

yah - OSX uses the alpha channel of the window for hit testing. I don't have an
explanation for why the event isn't getting through when there is no alpha
though. Perhaps the CALayer's alpha is influencing the hit testing as well as
the window alpha. (waiting for a frame may fix this, but we can't do that
without a GPU process. We could also set the layer's background color to white
instead of clear so we don't need to wait for an opaque frame -- something like
[[BridgedNativeWidget::compositor_superview_ layer]
setBackgroundColor:CGColorGetConstantColor(kCGColorWhite];).

If we can't get to the bottom of it, can you file a bug about this and update
the comment above?

(E.g. maybe we should still check WindowResizeHelperMac::Get()->task_runner() in
SetVisibilityState() - there might be surprises lurking there for tests later
on).

[karandeepb, 2016/11/09 06:33:22]

Yeah, you are correct. CALayer's alpha is also influencing hit testing. Updated
the comment to reflect the same.
Done. Although it seems a bit weird to use WindowResizeHelperMac when our end
goal is to just disable initial visibility suppression during tests. 

Also, due to the CALayer being transparent, we would need an explicit [window
setIgnoresMouseEvents: NO] in the tests anyway for cases where we send events
directly to the window server. It may be useful to have a method called
EnableMouseEventsForTesting on the BNW since this may cause confusion while
writing tests in the future.

[tapted, 2016/11/10 00:43:20]

No task runner means no GPU process, which could happen for other reasons, and
doesn't always happen in tests (e.g. browser_tests will have a task_runner).
E.g. it could also happen if we get the in-process context factory working, and
views_examples_exe (instead of just views_examples_with_content_exe).
I think this is OK - this test is kinda special. Things like ui::EventGenerator
don't rely on the window server to dispatch events.

[karandeepb, 2016/11/10 02:44:43]

Acknowledged. Also, updated the comment regarding WindowResizeHelperMac.

+  [window setAlphaValue:1.0];
+
   HitTestBridgedNativeWidget* bridge = native_widget->bridge();
 
+  const bool using_drag_event_monitor =
+      BridgedNativeWidget::ShouldUseDragEventMonitor();
+
   // Dragging the window should work.
   frame_view->set_hit_test_result(HTCAPTION);
   {
@@ -341,12 +353,16 @@ TEST_F(BridgedNativeWidgetUITest, HitTest) {
         NSMakePoint(20, 20), window);
     EXPECT_FALSE(bridge->IsDraggable());
     CGEventPost(kCGSessionEventTap, [mouse_down CGEvent]);
-    bridge->WaitForShouldRepost();
-    EXPECT_TRUE(bridge->did_repost());
-    EXPECT_TRUE(bridge->IsDraggable());
-    bridge->WaitForShouldRepost();
-    EXPECT_FALSE(bridge->did_repost());
-    EXPECT_FALSE(bridge->IsDraggable());
+    if (using_drag_event_monitor) {
+      bridge->WaitForShouldRepost();
+      EXPECT_TRUE(bridge->did_repost());
+      EXPECT_TRUE(bridge->IsDraggable());
+      bridge->WaitForShouldRepost();
+      EXPECT_FALSE(bridge->did_repost());
+      EXPECT_FALSE(bridge->IsDraggable());
+    } else {
+      WaitForEvent(NSLeftMouseDownMask);
+    }
 
     base::scoped_nsobject<WindowedNSNotificationObserver> ns_observer(

[tapted, 2016/11/08 05:03:17]

maybe we want to rename this to, e.g., move_observer -- it's easy to lose track
of what it's waiting for later on

[karandeepb, 2016/11/09 04:44:20]

Done.

         [[WindowedNSNotificationObserver alloc]
@@ -354,7 +370,7 @@ TEST_F(BridgedNativeWidgetUITest, HitTest) {
     NSEvent* mouse_drag = cocoa_test_event_utils::MouseEventAtPointInWindow(
         NSMakePoint(30, 30), NSLeftMouseDragged, window, 0);
     CGEventPost(kCGSessionEventTap, [mouse_drag CGEvent]);
-    [ns_observer wait];
+    EXPECT_TRUE([ns_observer wait]);
     EXPECT_EQ(110, [window frame].origin.x);
 
     NSEvent* mouse_up = cocoa_test_event_utils::MouseEventAtPointInWindow(
@@ -372,12 +388,16 @@ TEST_F(BridgedNativeWidgetUITest, HitTest) {
         bottom_right_point, window);
     EXPECT_FALSE(bridge->IsDraggable());
     CGEventPost(kCGSessionEventTap, [mouse_down CGEvent]);
-    bridge->WaitForShouldRepost();
-    EXPECT_TRUE(bridge->did_repost());
-    EXPECT_TRUE(bridge->IsDraggable());
-    bridge->WaitForShouldRepost();
-    EXPECT_FALSE(bridge->did_repost());
-    EXPECT_FALSE(bridge->IsDraggable());
+    if (using_drag_event_monitor) {
+      bridge->WaitForShouldRepost();
+      EXPECT_TRUE(bridge->did_repost());
+      EXPECT_TRUE(bridge->IsDraggable());
+      bridge->WaitForShouldRepost();
+      EXPECT_FALSE(bridge->did_repost());
+      EXPECT_FALSE(bridge->IsDraggable());
+    } else {
+      WaitForEvent(NSLeftMouseDownMask);
+    }
 
     base::scoped_nsobject<WindowedNSNotificationObserver> ns_observer(
         [[WindowedNSNotificationObserver alloc]
@@ -385,7 +405,7 @@ TEST_F(BridgedNativeWidgetUITest, HitTest) {
     NSEvent* mouse_drag = cocoa_test_event_utils::MouseEventAtPointInWindow(
         right_of_bottom_right, NSLeftMouseDragged, window, 0);
     CGEventPost(kCGSessionEventTap, [mouse_drag CGEvent]);
-    [ns_observer wait];
+    EXPECT_TRUE([ns_observer wait]);
     EXPECT_EQ(120, [window frame].origin.x);
 
     NSEvent* mouse_up = cocoa_test_event_utils::MouseEventAtPointInWindow(
@@ -412,6 +432,9 @@ TEST_F(BridgedNativeWidgetUITest, HitTest) {
     EXPECT_FALSE(bridge->IsDraggable());
     CGEventPost(kCGSessionEventTap, [mouse_down CGEvent]);
 
+    base::scoped_nsobject<WindowedNSNotificationObserver> ns_observer(
+        [[WindowedNSNotificationObserver alloc]
+            initForNotification:NSWindowDidResizeNotification]);
     NSEvent* mouse_drag = cocoa_test_event_utils::MouseEventAtPoint(
         NSMakePoint(x + 408, y + 2), NSLeftMouseDragged, 0);
     CGEventPost(kCGSessionEventTap, [mouse_drag CGEvent]);
@@ -420,11 +443,15 @@ TEST_F(BridgedNativeWidgetUITest, HitTest) {
         NSMakePoint(x + 408, y + 2), NSLeftMouseUp, 0);
     CGEventPost(kCGSessionEventTap, [mouse_up CGEvent]);
 
-    // The only event observed by us is the original mouse-down. It should not
-    // be reposted.
-    bridge->WaitForShouldRepost();
-    EXPECT_FALSE(bridge->did_repost());
-    EXPECT_FALSE(bridge->IsDraggable());
+    if (using_drag_event_monitor) {
+      // The only event observed by us is the original mouse-down. It should not
+      // be reposted.
+      bridge->WaitForShouldRepost();
+      EXPECT_FALSE(bridge->did_repost());
+      EXPECT_FALSE(bridge->IsDraggable());
+    }
+
+    EXPECT_TRUE([ns_observer wait]);
     EXPECT_EQ(410, [window frame].size.width);
 
     // Origin is unchanged.
@@ -467,10 +494,13 @@ TEST_F(BridgedNativeWidgetUITest, HitTest) {
     CGEventPost(kCGSessionEventTap, [mouse_down CGEvent]);
     WaitForEvent(NSLeftMouseDownMask);
 
+    base::scoped_nsobject<WindowedNSNotificationObserver> ns_observer(
+        [[WindowedNSNotificationObserver alloc]
+            initForNotification:NSWindowDidMoveNotification]);
     NSEvent* mouse_drag = cocoa_test_event_utils::MouseEventAtPointInWindow(
         NSMakePoint(30, 30), NSLeftMouseDragged, window, 0);
     CGEventPost(kCGSessionEventTap, [mouse_drag CGEvent]);
-    WaitForEvent(NSLeftMouseDraggedMask);
+    EXPECT_FALSE([ns_observer wait]);

[karandeepb, 2016/11/08 02:59:03]

This causes a RunLoop to run till the timeout. Can remove this but then we are
not testing anything since it will pass even with HTCAPTION.

[tapted, 2016/11/08 05:03:17]

Hrm. Doesn't that make this effectively ::sleep(test_timeouts::action_timeout())
then?

Does base::RunLoop().RunUntilIdle() work instead?

Or can you just insert an unrelated event into the queue and wait for it to be
processed? (i.e. assume that an event processed after means all events that were
in the queue before it have been processed).

[karandeepb, 2016/11/09 04:44:20]

Yeah it does.
Both of these didn't work. 

However, I am now using the NSWindowWillMoveNotification count. Hence the test
will fail if we were to use HTCAPTION above.

     EXPECT_EQ(120, [window frame].origin.x);
 
     NSEvent* mouse_up = cocoa_test_event_utils::MouseEventAtPointInWindow(