MacViews: Implement BridgedNativeWidget::OnDeviceScaleFactorChanged function.

ui/views/widget/widget_unittest.cc

Index: ui/views/widget/widget_unittest.cc
diff --git a/ui/views/widget/widget_unittest.cc b/ui/views/widget/widget_unittest.cc
index 5bb82e6943d7910cdb4259df78913c24d080e51e..8def9d499b68bab5c5734b571b0efa67b523ccd5 100644
--- a/ui/views/widget/widget_unittest.cc
+++ b/ui/views/widget/widget_unittest.cc
@@ -3398,5 +3398,47 @@ TEST_F(WidgetTest, AlwaysOnTop) {
   widget->CloseNow();
 }
 
+namespace {
+
+class ScaleFactorView : public View {
+ public:
+  ScaleFactorView() = default;
+
+  // Overridden from ui::LayerDelegate:
+  void OnDeviceScaleFactorChanged(float device_scale_factor) override {
+    last_scale_factor_ = device_scale_factor;
+    View::OnDeviceScaleFactorChanged(device_scale_factor);
+  }
+
+  float last_scale_factor() const { return last_scale_factor_; };
+
+ private:
+  float last_scale_factor_ = 0.f;
+
+  DISALLOW_COPY_AND_ASSIGN(ScaleFactorView);
+};
+
+struct WidgetCloser {
+  inline void operator()(Widget* widget) const { widget->CloseNow(); }
+};
+
+}
+
+TEST_F(WidgetTest, OnDeviceScaleFactorChanged) {

[tapted, 2015/12/21 07:20:16]

nit: comment describing the test like
// Ensure scale factor changes are propagated from the native Widget.

[Andrey Kraynov, 2015/12/21 09:47:19]

Done.

+  // Automatically close the widget, but not delete it.
+  scoped_ptr<Widget, WidgetCloser> widget(CreateTopLevelPlatformWidget());

[tapted, 2015/12/21 07:20:16]

ooh neat. we should do this more. WidgetCloser probably belongs in widget_test.h
but that's a good follow-up. (Perhaps the CreateFooWidget(..) methods should
even return a scoped_ptr..)

[Andrey Kraynov, 2015/12/21 09:47:19]

IMHO, this line can be much better with implemented make_scoped_ptr(ptr,
deleter) function, like
auto scoped = make_scoped_ptr(CreateFooWidget(), [](Widget* w){ w->CloseNow();
});

A colleague of mine already proposed a CL with that function,
https://codereview.chromium.org/1036133003/
How do you think, can it be usefull?

[tapted, 2015/12/21 22:54:11]

For this example, I think it's more useful if CreateTopLevelPlatformWidget()
returned the scoped_ptr, in which case it would be a stretch to use a lambda
since you need a return type. Then the line here would just be

auto widget = CreateTopLevelPlatformWidget();

But there may be cases where returning a scoped_ptr in specific tests breaks
down. E.g. those testing other ways to close, so it would need the right balance
to ensure those tests don't lose readability.

+  ScaleFactorView* view = new ScaleFactorView;
+  widget->GetRootView()->AddChildView(view);
+  float scale_factor = widget->GetLayer()->device_scale_factor();

[tapted, 2015/12/21 07:23:49]

heh, I guess we should check that this isn't 0 as well, or everything could be
"working"

[Andrey Kraynov, 2015/12/21 09:47:19]

Done.

+
+  // Adding the view should notify the view about the initial scale factor.

[tapted, 2015/12/21 07:20:16]

I guess

  // For views that are not layer-backed, adding the view won't notify the view
  // about the initial scale factor. Fake it.
  widget->GetLayer()->OnDeviceScaleFactorChanged(scale_factor);

[Andrey Kraynov, 2015/12/21 09:47:19]

Unfortunately, ui::Layer class compares scale factor value with cached value and
returns.
https://code.google.com/p/chromium/codesearch#chromium/src/ui/compositor/laye...
I used a direct call to ScaleFactorView::OnDeviceScaleFactorChanged function.
After that EXPECT looks strange. How do you think, is it OK?

[tapted, 2015/12/21 22:54:11]

yep - I think that's fine. Just takes "faking it" to another level :)

+  EXPECT_EQ(scale_factor, view->last_scale_factor());
+
+  // Changes should be propagated.
+  scale_factor *= 2.0f;
+  widget->GetLayer()->OnDeviceScaleFactorChanged(scale_factor);
+  EXPECT_EQ(scale_factor, view->last_scale_factor());
+}
+
 }  // namespace test
 }  // namespace views