Cleanup ui::PlatformWindowDelegate interface.

Some cleanups for the ui::PlatformWindowDelegate interface
1. Removed device_pixel_ratio parameter from
   OnAcceleratedWidgetAvailable. It wasn't used anywhere and the passed
   value was incorrect for most PlatformWindow implementations.
2. Removed OnAcceleratedWidgetDestroying() - it wasn't used anywhere.
3. Clarified OnAcceleratedWidgetDestroyed() should not to be called
   from destructor. It's only useful on Android. Wayland and scenic
   implementations were calling it incorrectly in destructor. This also
   fixes the crash in the attached bug.

TBR=piman@chromium.org (trivial change in //gpu)

Bug: 866729
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I164888802c65295bd07d9aa8d882332bbacbdbd9
Reviewed-on: https://chromium-review.googlesource.com/1147754
Commit-Queue: Sergey Ulanov <sergeyu@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578888}

components/viz/service/display_embedder/software_output_device_ozone_unittest.cc

@@ -42,11 +42,9 @@ class TestPlatformWindowDelegate : public ui::PlatformWindowDelegate {
   void OnClosed() override {}
   void OnWindowStateChanged(ui::PlatformWindowState new_state) override {}
   void OnLostCapture() override {}
-  void OnAcceleratedWidgetAvailable(gfx::AcceleratedWidget widget,
-                                    float device_pixel_ratio) override {
+  void OnAcceleratedWidgetAvailable(gfx::AcceleratedWidget widget) override {
     widget_ = widget;
   }
-  void OnAcceleratedWidgetDestroying() override {}
   void OnAcceleratedWidgetDestroyed() override {}
   void OnActivationChanged(bool active) override {}
 

gpu/ipc/service/direct_composition_surface_win_unittest.cc

@@ -85,9 +85,7 @@ class TestPlatformDelegate : public ui::PlatformWindowDelegate {
   void OnClosed() override {}
   void OnWindowStateChanged(ui::PlatformWindowState new_state) override {}
   void OnLostCapture() override {}
-  void OnAcceleratedWidgetAvailable(gfx::AcceleratedWidget widget,
-                                    float device_pixel_ratio) override {}
-  void OnAcceleratedWidgetDestroying() override {}
+  void OnAcceleratedWidgetAvailable(gfx::AcceleratedWidget widget) override {}
   void OnAcceleratedWidgetDestroyed() override {}
   void OnActivationChanged(bool active) override {}
 };

ui/aura/mus/window_tree_host_mus.cc

@@ -70,8 +70,7 @@ WindowTreeHostMus::WindowTreeHostMus(WindowTreeHostMusInitParams init_params)
   accelerated_widget =
       static_cast<gfx::AcceleratedWidget>(accelerated_widget_count++);
 #endif
-  OnAcceleratedWidgetAvailable(accelerated_widget,
-                               GetDisplay().device_scale_factor());
+  OnAcceleratedWidgetAvailable(accelerated_widget);
 
   delegate_->OnWindowTreeHostCreated(this);
 

ui/aura/window_tree_host_platform.cc

@@ -86,7 +86,7 @@ WindowTreeHostPlatform::~WindowTreeHostPlatform() {
   DestroyCompositor();
   DestroyDispatcher();
 
-  // |platform_window_| might have already been destroyed by this time.
+  // |platform_window_| may not exist yet.
   if (platform_window_)
     platform_window_->Close();
 }
@@ -236,16 +236,13 @@ void WindowTreeHostPlatform::OnLostCapture() {
 }
 
 void WindowTreeHostPlatform::OnAcceleratedWidgetAvailable(
-    gfx::AcceleratedWidget widget,
-    float device_pixel_ratio) {
+    gfx::AcceleratedWidget widget) {
   widget_ = widget;
   // This may be called before the Compositor has been created.
   if (compositor())
     WindowTreeHost::OnAcceleratedWidgetAvailable();
 }
 
-void WindowTreeHostPlatform::OnAcceleratedWidgetDestroying() {}
-
 void WindowTreeHostPlatform::OnAcceleratedWidgetDestroyed() {
   gfx::AcceleratedWidget widget = compositor()->ReleaseAcceleratedWidget();
   DCHECK_EQ(widget, widget_);

ui/aura/window_tree_host_platform.h

@@ -74,9 +74,7 @@ class AURA_EXPORT WindowTreeHostPlatform : public WindowTreeHost,
   void OnClosed() override;
   void OnWindowStateChanged(ui::PlatformWindowState new_state) override;
   void OnLostCapture() override;
-  void OnAcceleratedWidgetAvailable(gfx::AcceleratedWidget widget,
-                                    float device_pixel_ratio) override;
-  void OnAcceleratedWidgetDestroying() override;
+  void OnAcceleratedWidgetAvailable(gfx::AcceleratedWidget widget) override;
   void OnAcceleratedWidgetDestroyed() override;
   void OnActivationChanged(bool active) override;
 

ui/compositor/test/test_compositor_host_ozone.cc

@@ -41,11 +41,9 @@ class StubPlatformWindowDelegate : public PlatformWindowDelegate {
   void OnClosed() override {}
   void OnWindowStateChanged(PlatformWindowState new_state) override {}
   void OnLostCapture() override {}
-  void OnAcceleratedWidgetAvailable(gfx::AcceleratedWidget widget,
-                                    float device_pixel_ratio) override {
+  void OnAcceleratedWidgetAvailable(gfx::AcceleratedWidget widget) override {
     widget_ = widget;
   }
-  void OnAcceleratedWidgetDestroying() override {}
   void OnAcceleratedWidgetDestroyed() override {
     widget_ = gfx::kNullAcceleratedWidget;
   }

ui/gl/gl_surface_egl_unittest.cc

@@ -59,9 +59,7 @@ class TestPlatformDelegate : public ui::PlatformWindowDelegate {
   void OnClosed() override {}
   void OnWindowStateChanged(ui::PlatformWindowState new_state) override {}
   void OnLostCapture() override {}
-  void OnAcceleratedWidgetAvailable(gfx::AcceleratedWidget widget,
-                                    float device_pixel_ratio) override {}
-  void OnAcceleratedWidgetDestroying() override {}
+  void OnAcceleratedWidgetAvailable(gfx::AcceleratedWidget widget) override {}
   void OnAcceleratedWidgetDestroyed() override {}
   void OnActivationChanged(bool active) override {}
 };

ui/ozone/demo/demo_window.cc

@@ -71,14 +71,11 @@ void DemoWindow::OnWindowStateChanged(PlatformWindowState new_state) {}
 
 void DemoWindow::OnLostCapture() {}
 
-void DemoWindow::OnAcceleratedWidgetAvailable(gfx::AcceleratedWidget widget,
-                                              float device_pixel_ratio) {
+void DemoWindow::OnAcceleratedWidgetAvailable(gfx::AcceleratedWidget widget) {
   DCHECK_NE(widget, gfx::kNullAcceleratedWidget);
   widget_ = widget;
 }
 
-void DemoWindow::OnAcceleratedWidgetDestroying() {}
-
 void DemoWindow::OnAcceleratedWidgetDestroyed() {
   widget_ = gfx::kNullAcceleratedWidget;
 }

ui/ozone/demo/demo_window.h

@@ -40,9 +40,7 @@ class DemoWindow : public PlatformWindowDelegate {
   void OnClosed() override;
   void OnWindowStateChanged(PlatformWindowState new_state) override;
   void OnLostCapture() override;
-  void OnAcceleratedWidgetAvailable(gfx::AcceleratedWidget widget,
-                                    float device_pixel_ratio) override;
-  void OnAcceleratedWidgetDestroying() override;
+  void OnAcceleratedWidgetAvailable(gfx::AcceleratedWidget widget) override;
   void OnAcceleratedWidgetDestroyed() override;
   void OnActivationChanged(bool active) override;
 

ui/ozone/platform/cast/platform_window_cast.cc

@@ -18,7 +18,7 @@ PlatformWindowCast::PlatformWindowCast(PlatformWindowDelegate* delegate,
                                        const gfx::Rect& bounds)
     : StubWindow(delegate, false, bounds) {
   gfx::AcceleratedWidget widget = (bounds.width() << 16) + bounds.height();
-  delegate->OnAcceleratedWidgetAvailable(widget, 1.f);
+  delegate->OnAcceleratedWidgetAvailable(widget);
 
   if (PlatformEventSource::GetInstance())
     PlatformEventSource::GetInstance()->AddPlatformEventDispatcher(this);

ui/ozone/platform/drm/host/drm_window_host.cc

@@ -53,7 +53,7 @@ void DrmWindowHost::Initialize() {
   sender_->AddGpuThreadObserver(this);
   PlatformEventSource::GetInstance()->AddPlatformEventDispatcher(this);
   cursor_->OnWindowAdded(widget_, bounds_, GetCursorConfinedBounds());
-  delegate_->OnAcceleratedWidgetAvailable(widget_, 1.f);
+  delegate_->OnAcceleratedWidgetAvailable(widget_);
 }
 
 gfx::AcceleratedWidget DrmWindowHost::GetAcceleratedWidget() {

ui/ozone/platform/headless/headless_window.cc

@@ -22,7 +22,7 @@ HeadlessWindow::HeadlessWindow(PlatformWindowDelegate* delegate,
 #else
   widget_ = manager_->AddWindow(this);
 #endif
-  delegate->OnAcceleratedWidgetAvailable(widget_, 1.f);
+  delegate->OnAcceleratedWidgetAvailable(widget_);
 }
 
 HeadlessWindow::~HeadlessWindow() {

ui/ozone/platform/magma/magma_window.cc

@@ -18,7 +18,7 @@ MagmaWindow::MagmaWindow(PlatformWindowDelegate* delegate,
                          const gfx::Rect& bounds)
     : StubWindow(delegate, false, bounds), manager_(manager) {
   widget_ = manager_->AddWindow(this);
-  delegate->OnAcceleratedWidgetAvailable(widget_, 1.f);
+  delegate->OnAcceleratedWidgetAvailable(widget_);
 }
 
 MagmaWindow::~MagmaWindow() {

ui/ozone/platform/scenic/scenic_window.cc

@@ -88,21 +88,17 @@ ScenicWindow::ScenicWindow(
 
   // Call Present() to ensure that the scenic session commands are processed,
   // which is necessary to receive metrics event from Scenic.
-  // OnAcceleratedWidgetAvailable() will be called after View metrics are
-  // received.
   scenic_session_.Present();
+
+  delegate_->OnAcceleratedWidgetAvailable(window_id_);
 }
 
 ScenicWindow::~ScenicWindow() {
-  delegate_->OnAcceleratedWidgetDestroying();
-
   scenic_session_.ReleaseResource(node_id_);
   scenic_session_.ReleaseResource(parent_node_id_);
 
   manager_->RemoveWindow(window_id_, this);
   view_.Unbind();
-
-  delegate_->OnAcceleratedWidgetDestroyed();
 }
 
 gfx::Rect ScenicWindow::GetBounds() {
@@ -220,8 +216,6 @@ void ScenicWindow::OnScenicEvents(
           std::max(metrics.metrics.scale_x, metrics.metrics.scale_y);
       if (device_pixel_ratio_ == 0.0) {
         device_pixel_ratio_ = new_device_pixel_ratio;
-        delegate_->OnAcceleratedWidgetAvailable(window_id_,
-                                                device_pixel_ratio_);
         if (!size_dips_.IsEmpty())
           UpdateSize();
       } else if (device_pixel_ratio_ != new_device_pixel_ratio) {

ui/ozone/platform/wayland/wayland_pointer_unittest.cc

@@ -46,7 +46,7 @@ TEST_P(WaylandPointerTest, Leave) {
   MockPlatformWindowDelegate other_delegate;
   WaylandWindow other_window(&other_delegate, connection_.get());
   gfx::AcceleratedWidget other_widget = gfx::kNullAcceleratedWidget;
-  EXPECT_CALL(other_delegate, OnAcceleratedWidgetAvailable(_, _))
+  EXPECT_CALL(other_delegate, OnAcceleratedWidgetAvailable(_))
       .WillOnce(SaveArg<0>(&other_widget));
   PlatformWindowInitProperties properties;
   properties.bounds = gfx::Rect(0, 0, 10, 10);

ui/ozone/platform/wayland/wayland_test.cc

@@ -37,7 +37,7 @@ WaylandTest::~WaylandTest() {}
 void WaylandTest::SetUp() {
   ASSERT_TRUE(server_.Start(GetParam()));
   ASSERT_TRUE(connection_->Initialize());
-  EXPECT_CALL(delegate_, OnAcceleratedWidgetAvailable(_, _))
+  EXPECT_CALL(delegate_, OnAcceleratedWidgetAvailable(_))
       .WillOnce(SaveArg<0>(&widget_));
   PlatformWindowInitProperties properties;
   properties.bounds = gfx::Rect(0, 0, 800, 600);

ui/ozone/platform/wayland/wayland_window.cc

@@ -74,8 +74,6 @@ WaylandWindow::WaylandWindow(PlatformWindowDelegate* delegate,
       state_(PlatformWindowState::PLATFORM_WINDOW_STATE_UNKNOWN) {}
 
 WaylandWindow::~WaylandWindow() {
-  delegate_->OnAcceleratedWidgetDestroying();
-
   PlatformEventSource::GetInstance()->RemovePlatformEventDispatcher(this);
   connection_->RemoveWindow(surface_.id());
 
@@ -84,8 +82,6 @@ WaylandWindow::~WaylandWindow() {
 
   if (has_pointer_focus_)
     connection_->pointer()->reset_window_with_pointer_focus();
-
-  delegate_->OnAcceleratedWidgetDestroyed();
 }
 
 // static
@@ -129,7 +125,7 @@ bool WaylandWindow::Initialize(PlatformWindowInitProperties properties) {
 
   connection_->AddWindow(surface_.id(), this);
   PlatformEventSource::GetInstance()->AddPlatformEventDispatcher(this);
-  delegate_->OnAcceleratedWidgetAvailable(surface_.id(), 1.f);
+  delegate_->OnAcceleratedWidgetAvailable(surface_.id());
 
   return true;
 }

ui/ozone/platform/wayland/wayland_window_unittest.cc

@@ -499,15 +499,11 @@ TEST_P(WaylandWindowTest, OnActivationChanged) {
 }
 
 TEST_P(WaylandWindowTest, OnAcceleratedWidgetDestroy) {
-  EXPECT_CALL(delegate_, OnAcceleratedWidgetDestroying()).Times(1);
-  EXPECT_CALL(delegate_, OnAcceleratedWidgetDestroyed()).Times(1);
   window_.reset();
 }
 
 TEST_P(WaylandWindowTest, CreateAndDestroyMenuWindow) {
   MockPlatformWindowDelegate menu_window_delegate;
-  EXPECT_CALL(menu_window_delegate, OnAcceleratedWidgetDestroying()).Times(1);
-  EXPECT_CALL(menu_window_delegate, OnAcceleratedWidgetDestroyed()).Times(1);
 
   std::unique_ptr<WaylandWindow> menu_window = CreateWaylandWindowWithParams(
       PlatformWindowType::kMenu, widget_, gfx::Rect(0, 0, 10, 10),
@@ -518,8 +514,6 @@ TEST_P(WaylandWindowTest, CreateAndDestroyMenuWindow) {
 
 TEST_P(WaylandWindowTest, CreateAndDestroyMenuWindowWithFocusedParent) {
   MockPlatformWindowDelegate menu_window_delegate;
-  EXPECT_CALL(menu_window_delegate, OnAcceleratedWidgetDestroying()).Times(1);
-  EXPECT_CALL(menu_window_delegate, OnAcceleratedWidgetDestroyed()).Times(1);
 
   // set_pointer_focus(true) requires a WaylandPointer.
   wl_seat_send_capabilities(server_.seat()->resource(),
@@ -538,7 +532,7 @@ TEST_P(WaylandWindowTest, CreateAndDestroyMenuWindowWithFocusedParent) {
 TEST_P(WaylandWindowTest, CreateAndDestroyNestedMenuWindow) {
   MockPlatformWindowDelegate menu_window_delegate;
   gfx::AcceleratedWidget menu_window_widget;
-  EXPECT_CALL(menu_window_delegate, OnAcceleratedWidgetAvailable(_, _))
+  EXPECT_CALL(menu_window_delegate, OnAcceleratedWidgetAvailable(_))
       .WillOnce(SaveArg<0>(&menu_window_widget));
 
   std::unique_ptr<WaylandWindow> menu_window = CreateWaylandWindowWithParams(
@@ -581,7 +575,7 @@ TEST_P(WaylandWindowTest, CanDispatchEventToMenuWindowNonNested) {
 TEST_P(WaylandWindowTest, CanDispatchEventToMenuWindowNested) {
   MockPlatformWindowDelegate menu_window_delegate;
   gfx::AcceleratedWidget menu_window_widget;
-  EXPECT_CALL(menu_window_delegate, OnAcceleratedWidgetAvailable(_, _))
+  EXPECT_CALL(menu_window_delegate, OnAcceleratedWidgetAvailable(_))
       .WillOnce(SaveArg<0>(&menu_window_widget));
 
   std::unique_ptr<WaylandWindow> menu_window = CreateWaylandWindowWithParams(

ui/ozone/platform/x11/x11_window_ozone_unittest.cc

@@ -55,7 +55,7 @@ class X11WindowOzoneTest : public testing::Test {
       MockPlatformWindowDelegate* delegate,
       const gfx::Rect& bounds,
       gfx::AcceleratedWidget* widget) {
-    EXPECT_CALL(*delegate, OnAcceleratedWidgetAvailable(_, _))
+    EXPECT_CALL(*delegate, OnAcceleratedWidgetAvailable(_))
         .WillOnce(StoreWidget(widget));
     auto window = std::make_unique<X11WindowOzone>(window_manager_.get(),
                                                    delegate, bounds);

ui/ozone/test/mock_platform_window_delegate.h

@@ -23,9 +23,8 @@ class MockPlatformWindowDelegate : public PlatformWindowDelegate {
   MOCK_METHOD0(OnClosed, void());
   MOCK_METHOD1(OnWindowStateChanged, void(PlatformWindowState new_state));
   MOCK_METHOD0(OnLostCapture, void());
-  MOCK_METHOD2(OnAcceleratedWidgetAvailable,
-               void(gfx::AcceleratedWidget widget, float device_pixel_ratio));
-  MOCK_METHOD0(OnAcceleratedWidgetDestroying, void());
+  MOCK_METHOD1(OnAcceleratedWidgetAvailable,
+               void(gfx::AcceleratedWidget widget));
   MOCK_METHOD0(OnAcceleratedWidgetDestroyed, void());
   MOCK_METHOD1(OnActivationChanged, void(bool active));
 

ui/platform_window/android/platform_window_android.cc

@@ -80,7 +80,7 @@ void PlatformWindowAndroid::SurfaceCreated(
     base::android::ScopedJavaLocalFrame scoped_local_reference_frame(env);
     window_ = ANativeWindow_fromSurface(env, jsurface);
   }
-  delegate()->OnAcceleratedWidgetAvailable(window_, device_pixel_ratio);
+  delegate()->OnAcceleratedWidgetAvailable(window_);
 }
 
 void PlatformWindowAndroid::SurfaceDestroyed(JNIEnv* env,

ui/platform_window/platform_window_delegate.h

@@ -43,15 +43,11 @@ class PlatformWindowDelegate {
 
   virtual void OnLostCapture() = 0;
 
-  virtual void OnAcceleratedWidgetAvailable(gfx::AcceleratedWidget widget,
-                                            float device_pixel_ratio) = 0;
-
-  // Notifies the delegate its the last chance to manipulate the native
-  // widget before destruction.
-  virtual void OnAcceleratedWidgetDestroying() = 0;
+  virtual void OnAcceleratedWidgetAvailable(gfx::AcceleratedWidget widget) = 0;
 
   // Notifies the delegate that the widget cannot be used anymore until
   // a new widget is made available through OnAcceleratedWidgetAvailable().
+  // Must not be called when the PlatformWindow is being destroyed.
   virtual void OnAcceleratedWidgetDestroyed() = 0;
 
   virtual void OnActivationChanged(bool active) = 0;

ui/platform_window/stub/stub_window.cc

@@ -15,17 +15,14 @@ StubWindow::StubWindow(PlatformWindowDelegate* delegate,
     : delegate_(delegate), bounds_(bounds) {
   DCHECK(delegate);
   if (use_default_accelerated_widget)
-    delegate_->OnAcceleratedWidgetAvailable(gfx::kNullAcceleratedWidget, 1.f);
+    delegate_->OnAcceleratedWidgetAvailable(gfx::kNullAcceleratedWidget);
 }
 
-StubWindow::~StubWindow() {
-}
+StubWindow::~StubWindow() {}
 
-void StubWindow::Show() {
-}
+void StubWindow::Show() {}
 
-void StubWindow::Hide() {
-}
+void StubWindow::Hide() {}
 
 void StubWindow::Close() {
   delegate_->OnClosed();
@@ -47,40 +44,31 @@ gfx::Rect StubWindow::GetBounds() {
 
 void StubWindow::SetTitle(const base::string16& title) {}
 
-void StubWindow::SetCapture() {
-}
+void StubWindow::SetCapture() {}
 
-void StubWindow::ReleaseCapture() {
-}
+void StubWindow::ReleaseCapture() {}
 
 bool StubWindow::HasCapture() const {
   return false;
 }
 
-void StubWindow::ToggleFullscreen() {
-}
+void StubWindow::ToggleFullscreen() {}
 
-void StubWindow::Maximize() {
-}
+void StubWindow::Maximize() {}
 
-void StubWindow::Minimize() {
-}
+void StubWindow::Minimize() {}
 
-void StubWindow::Restore() {
-}
+void StubWindow::Restore() {}
 
 PlatformWindowState StubWindow::GetPlatformWindowState() const {
   return PlatformWindowState::PLATFORM_WINDOW_STATE_UNKNOWN;
 }
 
-void StubWindow::SetCursor(PlatformCursor cursor) {
-}
+void StubWindow::SetCursor(PlatformCursor cursor) {}
 
-void StubWindow::MoveCursorTo(const gfx::Point& location) {
-}
+void StubWindow::MoveCursorTo(const gfx::Point& location) {}
 
-void StubWindow::ConfineCursorToBounds(const gfx::Rect& bounds) {
-}
+void StubWindow::ConfineCursorToBounds(const gfx::Rect& bounds) {}
 
 PlatformImeController* StubWindow::GetPlatformImeController() {
   return nullptr;

ui/platform_window/win/win_window.cc

@@ -178,8 +178,7 @@ void WinWindow::OnClose() {
 }
 
 LRESULT WinWindow::OnCreate(CREATESTRUCT* create_struct) {
-  // TODO(sky): provide real scale factor.
-  delegate_->OnAcceleratedWidgetAvailable(hwnd(), 1.f);
+  delegate_->OnAcceleratedWidgetAvailable(hwnd());
   return 0;
 }
 

ui/platform_window/x11/x11_window_base.cc

@@ -138,8 +138,7 @@ void X11WindowBase::Create() {
   size_hints.win_gravity = StaticGravity;
   XSetWMNormalHints(xdisplay_, xwindow_, &size_hints);
 
-  // TODO(sky): provide real scale factor.
-  delegate_->OnAcceleratedWidgetAvailable(xwindow_, 1.f);
+  delegate_->OnAcceleratedWidgetAvailable(xwindow_);
 }
 
 void X11WindowBase::Show() {

ui/views/widget/desktop_aura/desktop_window_tree_host_platform.cc

@@ -470,10 +470,6 @@ void DesktopWindowTreeHostPlatform::OnCloseRequest() {
   GetWidget()->Close();
 }
 
-void DesktopWindowTreeHostPlatform::OnAcceleratedWidgetDestroying() {
-  native_widget_delegate_->OnNativeWidgetDestroying();
-}
-
 void DesktopWindowTreeHostPlatform::OnActivationChanged(bool active) {
   is_active_ = active;
   aura::WindowTreeHostPlatform::OnActivationChanged(active);

ui/views/widget/desktop_aura/desktop_window_tree_host_platform.h

@@ -93,7 +93,6 @@ class VIEWS_EXPORT DesktopWindowTreeHostPlatform
   void OnClosed() override;
   void OnWindowStateChanged(ui::PlatformWindowState new_state) override;
   void OnCloseRequest() override;
-  void OnAcceleratedWidgetDestroying() override;
   void OnActivationChanged(bool active) override;
 
  private: