New issue
Advanced search Search tips

Issue 674003 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Dec 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

Ozone AuraMusClient doesn't send out a visibility change during Widget::Close()?

Project Member Reported by tapted@chromium.org, Dec 14 2016

Issue description

Encountered this while investigating  Issue 673991  . Mus currently behaves differently to everything else.

To reproduce, have a View that overrides View::VisibilityChanged(), then Close() the Widget it's in.

Mus doesn't notify the view of a visibility change when Close() calls Hide() (per the contract of Widget::Close(), i.e.

  // Hides the widget then closes it after a return to the message loop.
  virtual void Close();

See https://codereview.chromium.org/2577593002/#ps1

And bot failure on linux_chromium_chromeos_ozone_rel_ng:

Retrying 1 test (retry #2)
[13931:13932:1213/185443.255989:435210663:ERROR:kill_posix.cc(84)] Unable to terminate process group 17994: No such process
[ RUN      ] ViewTest.DestroyLayerInClose
[17994:17998:1213/185442.950624:434905317:ERROR:entry.cc(167)] Entry::Deserialize: dictionary has no interface_provider_specs key
[17994:17998:1213/185442.952824:434907506:INFO:child_process_host.cc(241)] Launched child process pid=18002, instance=, name=test_wm, user_id=505C0EE9-3013-43C0-82B0-A84F50CF8D84
[17994:17998:1213/185442.953845:434908524:INFO:child_process_host.cc(241)] Launched child process pid=18003, instance=, name=ui, user_id=0a6ff6e8-3fb0-4b72-ba2a-60f732e78027
[17994:17998:1213/185442.965404:434920078:INFO:child_process_host.cc(241)] Launched child process pid=18012, instance=, name=tracing, user_id=984a787f-14ba-4b98-a6ee-be2f6cbc8426
[17994:17998:1213/185442.966820:434921498:INFO:child_process_host.cc(241)] Launched child process pid=18013, instance=, name=test_ime_driver, user_id=0a6ff6e8-3fb0-4b72-ba2a-60f732e78027
[18002:18002:1213/185443.177362:435132106:ERROR:screen_base.cc(42)] Not implemented reached in virtual display::Display display::ScreenBase::GetDisplayNearestWindow(gfx::NativeView) const
[17994:17994:1213/185443.178897:435133569:ERROR:screen_base.cc(42)] Not implemented reached in virtual display::Display display::ScreenBase::GetDisplayNearestWindow(gfx::NativeView) const
../../ui/views/view_unittest.cc:4663: Failure
Value of: view->layer()
  Actual: true
Expected: false
../../ui/views/view_unittest.cc:4666: Failure
Value of: view->was_hidden()
  Actual: false
Expected: true
[18002:18002:1213/185443.189853:435144526:ERROR:layer_tree_host_impl.cc(2181)] Forcing zero-copy tile initialization as worker context is missing
[  FAILED  ] ViewTest.DestroyLayerInClose (250 ms)


 

Comment 1 by tapted@chromium.org, Dec 14 2016

Summary: Ozone AuraMusClient doesn't send out a visibility change during Widget::Close()? (was: NativeWidgetMus doesn't send out a visibility change during Widget::Close()?)
So actually behaviour was different just on Ozone when IsAuraMusClient was true:

  widget->Close();
  if (IsAuraMusClient()) {
    // Mus on Ozone doesn't send the visibility change during Close().
    // See  http://crbug.com/674003 .
    EXPECT_TRUE(view.layer());
    EXPECT_FALSE(view.was_hidden());
  } else {
    EXPECT_FALSE(view.layer());
    // Ensure the layer went away via VisibilityChanged().
    EXPECT_TRUE(view.was_hidden());
  }

From https://codereview.chromium.org/2577593002/
Cc: sky@chromium.org
Labels: Proj-Mustash-Aura-Views
sky@: could you triage?
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 14 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c8701123a47788c2dcd15f957ac02492d1138fa4

commit c8701123a47788c2dcd15f957ac02492d1138fa4
Author: tapted <tapted@chromium.org>
Date: Wed Dec 14 22:34:28 2016

MacViews: Be robust against Views manipulating Layers during Widget::Close().

An InkDropHostView may manipulate layers in response the Widget::Hide().
A Hide() occurs synchronously in Widget::Close(), followed by tear down
that occurs asynchronously. Mac's BridgedNativeWidget (and
aura::Window::~Window()) suppress repaints during a close.
BridgedNativeWidget does this by clearing out the compositor. We
shouldn't try to subsequently recreate the compositor (there's a DCHECK
for that).

Manipulating Layers may call Widget::ReorderNativeViews. And
NativeWidgetMac::ReorderNativeViews() currently calls
BridgedNativeWidget::SetRootView() (which updates the compositor). It's
done this since before ReorderChildViews() was implemented, but it's no
longer necessary. It should always be a no-op: the RootView can't
change.

BUG= 673991 ,  674003 

Review-Url: https://codereview.chromium.org/2577593002
Cr-Commit-Position: refs/heads/master@{#438653}

[modify] https://crrev.com/c8701123a47788c2dcd15f957ac02492d1138fa4/ui/views/cocoa/bridged_native_widget.mm
[modify] https://crrev.com/c8701123a47788c2dcd15f957ac02492d1138fa4/ui/views/view_unittest.cc
[modify] https://crrev.com/c8701123a47788c2dcd15f957ac02492d1138fa4/ui/views/widget/native_widget_mac.mm

Comment 4 by sky@chromium.org, Dec 15 2016

Cc: -sky@chromium.org
Labels: OS-Chrome
Owner: sky@chromium.org
Status: Started (was: Untriaged)
Project Member

Comment 5 by bugdroid1@chromium.org, Dec 15 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/88dc5d7572db0090dbb1b14442da7d2c33efb18c

commit 88dc5d7572db0090dbb1b14442da7d2c33efb18c
Author: sky <sky@chromium.org>
Date: Thu Dec 15 18:51:11 2016

Fixes visibility bugs in DesktopWindowTreeHostMus

Close() needs to hide right away.
And the delegate wasn't notified of show/hide.

BUG= 674003 
TEST=covered by test
R=erg@chromium.org

Review-Url: https://codereview.chromium.org/2582523003
Cr-Commit-Position: refs/heads/master@{#438882}

[modify] https://crrev.com/88dc5d7572db0090dbb1b14442da7d2c33efb18c/ui/views/mus/desktop_window_tree_host_mus.cc
[modify] https://crrev.com/88dc5d7572db0090dbb1b14442da7d2c33efb18c/ui/views/view_unittest.cc

Comment 6 by sky@chromium.org, Dec 15 2016

Status: Fixed (was: Started)

Comment 7 by dchan@google.com, Mar 4 2017

Labels: VerifyIn-58

Comment 8 by dchan@google.com, Apr 17 2017

Labels: VerifyIn-59

Comment 9 by dchan@google.com, May 30 2017

Labels: VerifyIn-60
Labels: VerifyIn-61

Comment 11 by dchan@chromium.org, Oct 14 2017

Status: Archived (was: Fixed)
Components: -Internals>MUS Internals>Services>WindowService

Sign in to add a comment