New issue
Advanced search Search tips

Issue 897662 link

Starred by 7 users

Issue metadata

Status: Verified
Owner:
Closed: Nov 14
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression: Unable to Min/Max/Close browser via tap-touch.

Reported by aiman.an...@etouch.net, Oct 22

Issue description

Chrome Version: 72.0.3588.0 (Official Build) Revision 56533f367ba451d6545ab0045e8e11f288b36326-refs/branch-heads/3588@{#1} (32/64-bit).

OS: Windows 10 (Touch Device) only.

Steps to reproduce:
1. Launch chrome, Try to tap-touch on Minimize/Maximize/Close button in Chrome tab strip.
2. Observe .

Actual Result: Unable to Min/Max/Close browser via tap-touch.  
Expected Result: Should be able to Min/Max/Close browser via tap-touch. 

This is Regression Issue broken in M-72, and below is the bisect info.
Good Build: 72.0.3584.0 (Revision:600617)
Bad Build: 72.0.3585.0 (Revision:601013)

Narrow Bisect:

CHANGE-LOG URL:

https://chromium.googlesource.com/chromium/src/+log/f17b24b1e2bb0488361cce031afd8ede9ff41455..1c621415aa17d8f1d3de440798ad19714b07df8c?pretty=fuller&n=10000

Suspecting: r600658 or r600662 ?

tasak/sunnyps@: Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner.

Note:
1. Issue is Win 10 (Touch Device) specific and is not seen on Win(7,8,8.1,10), Mac(10.12.6,10.13.1,10.13.6,10.14.1) and Linux(14.04 LTS) OS.
2. Unable to provide 'per-revision' bisect as it shows "RuntimeError : We don't have enough builds to bisect..." error message for above range. 
3. Hence provided suspect through 'Chromium bisect'.

Kindly refer the attached screen-cast.

Thank You!
 
 
Actual Result.mp4
905 KB View Download
Expected Result.mp4
1.2 MB View Download
Labels: ReleaseBlock-Stable
As this is a recent regression adding release blocker label for this issue.Please reduce priority or remove if not the case.

Thank You!
Cc: brucedaw...@chromium.org zmo@chromium.org chaopeng@chromium.org
Can you attach chrome://gpu from the affected device?

Might be the same root cause as  issue 871257  which is that Windows passes touch events to the intermediate D3D window from GPU process instead of browser's window.

My change makes it more likely that direct composition is not used which should prevent the above kind of bug, but maybe not using direct composition causes this bug?
Owner: sunn...@chromium.org
It's probably not tasak's change.
Hi,

w.r.t. comment #2 attaching chrome://gpu details from Win 10 Touch Device.

Kindly refer.

Thank You!
CHROME-GPU-897662.png
1.6 MB View Download
According to chrome://gpu, direct composition is not being used.

I reproduced this on a Lenovo Yoga Tab by disabling direct composition with --disable-direct-composition-layers.  The minimize/maximize/close buttons do not respond to touch, but double tapping on the title bar maximizes and restores, and the rest of Chrome UI is responsive to touch.  With direct composition enabled, the minimize/maximize/close buttons work as well.

With direct composition, Spy++ thinks the intermediate D3D window is over buttons, but without direct composition it's the Chrome browser parent window (Chrome_WidgetWin_1).  So it looks like the D3D window causes touch events on the buttons to be dropped (like the other bug), but that makes the buttons do the right thing (via default message handler probably?).

So yes, my CL caused this regression because it disabled direct composition in many cases, but we don't want to revert it because we'd be using direct composition where we don't need to, and that's a riskier code path IMO.
Cc: bsep@chromium.org
I found an interesting post while reading the product forum for the other bug:

"ruifung said:
Based on what is said in https://bugs.chromium.org/p/chromium/issues/detail?id=871257

I worked around it by starting chrome with  --disable-d3d11, which appears to fix the issue for me. 
While this does result in the minimize/maximize/close buttons no longer accepting touch input, I worked around it by setting
#windows10-custom-titlebar in chrome://flags to disabled.

I'm using a Surface Go 128GB. Chrome 70"

+bsep@ - looks like custom title bar makes window controls not work with touch when direct composition is disabled
I'm seeing interesting results on a Lenovo Yoga Touch 710:

1) Default (direct composition enabled): window buttons work on touch
2) --disable-direct-composition-layers: window buttons do not work at all on touch (don't change visual state)
3) chrome://flags set windows10-custom-titlebar to Disabled: window buttons do not work but change visual state on touch
4) chrome://flags as above and --disable-direct-composition-layers: window buttons work on touch

So it looks like window buttons work on touch = direct composition xnor custom titlebar
From Ross about spy++ issue. 

> I debugged that too… I see spy++ calls WindowFromPoint, and that function is returning
> the legacy window. Spy++ must be doing > something else after the fact, but I can’t tell
> what. Certainly misleading…
>
> > Hi Ross, 
> >
> > We have one related issue. When we debugging this issue, we use spy++ find tool to 
> > find the Chrome window, it select the D3D window, but spy++ shows D3D window is below 
> > legacy Chrome window. Is it expected?

MSDN only mention the spy++ shows window by z-order.
https://docs.microsoft.com/en-us/visualstudio/debugger/windows-view?view=vs-2017

So if you seeing the order is:

+ Toplevel: class=Chrome_WidgetWin_1
+- Child 1: Chrome Legacy Window
+- Child 2: Intermediate D3D Window or no D3D window

Then the z-order is correct.
I've seen a few of these go by. I have  bug 894023  for "caption buttons don't work if hardware acceleration is off" which I haven't had time to look at.

I also have a smattering of "caption buttons don't work at all" bugs for older hardware (e.g. bug 845832). I'm guessing their touch drivers are weird, but I haven't dug into it that much. Those are mostly for Dell Inspiron 15R though.

Is this one of those issues? If so, this doesn't need to be RBS.
Looks like a duplicate of those other issues.  Disabling hardware acceleration has the side-effect of disabling direct composition which is what actually breaks the window buttons.  We disabled direct composition in many cases (non Intel Skylake+ devices) so more users will see this bug now so it might be higher priority.
I see. I'll bump up 894023 on my todo list then, or feel free to investigate yourself. Let me know if you have any questions. The buttons are supposed to work in all cases of course, but custom titlebar is rarely off on Windows 10 and touch devices are rare on Windows 7.
We probably want to dupe the other way tho, since there's more info here.
Can you provide a few pointers for how we handle window button interactions?  Where are the message handlers for the browser parent (Chrome_WidgetWin_1) and child (Chrom Legacy Window) windows?
Labels: Hotlist-DesktopUIConsider
Labels: Group-Window_Frame
We have a fix for the touch scrolling issue (crrev.com/c/1299342) but it makes this bug happen 100% of the time.
Owner: bsep@chromium.org
-> bsep@ since this is a known UI bug
Labels: -Hotlist-DesktopUIConsider Hotlist-DesktopUITriaged

Comment 20 Deleted

Could you please provide any update on this issue as it has been marked as a stable blocker.

Thank You!

@Gentle Ping! Could you please provide any update on this issue as it has been marked as a stable blocker.

Thank You!
Haven't started yet, been too busy. Hopefully will soon
 Issue 894023  has been merged into this issue.
Status: Started (was: Assigned)
I looked at this today. I tried to reproduce the bug in both the simulator and on a Surfacebook:
* Building ToT always reproduces, regardless of whether I pass in --disable-direct-composition-layers or --enable-direct-composition-layers, or toggle hardware acceleration.
* Canary seems to reproduce inconsistently... sometimes it works and sometimes it doesn't. It sends to reproduce more often than not, but it certainly was confusing.
* Stable always works. I ran a bisect and it came up with r600662 as above, which we already knew about.

I attached Spyxx and it claims that it's sending WM_POINTERDOWN to Stable, but WM_NCPOINTERDOWN to Canary. So we did something that makes Windows think we can handle WM_NCPOINTERDOWN, but we don't. Honestly, we SHOULD be handling it, but no one has had time to implement it.

I'll play around with it more tomorrow, but we may have to revert sunnyps@'s patch until we get around to doing Windows 10 touch properly... I'll also check and make sure bug 894032 is actually a dupe; I'm not convinced it is, since the direct composition flags above didn't seem to do anything.
FWIW that CL has been effectively reverted on ToT with https://chromium-review.googlesource.com/c/chromium/src/+/1318808

We started using WS_EX_TRANSPARENT for the GPU process child window which makes Windows pass input events to sibling/parent windows.  So now one of the browser windows will be processing the event which is where the bug is.

We'd have to revert using that window style, but we did that because touch scrolling was completely broken without that (Windows wouldn't pass touch messages to the browser window).  We decided that the min/max/close button bug is less severe than touch scrolling being broken.
Okay, so it sounds like we need to roll forward. Hopefully it won't be too much work to support WM_NCPOINTER* events.
Project Member

Comment 28 by bugdroid1@chromium.org, Nov 14

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

commit fbdbb023f2619c745abcd8bcab3039688b8eded5
Author: Bret Sepulveda <bsep@chromium.org>
Date: Wed Nov 14 20:25:58 2018

Add support for WM_NCPOINTER* messages.

A recent change caused the caption buttons to stop responding to touch.
When I investigated, the change caused Windows to send WM_NCPOINTERDOWN
and WM_NCPOINTERUP in that area (which we don't handle), where
previously it was sending WM_POINTERDOWN and WM_POINTERUP.

Since the change in question is fixing a different critical issue, this
patch implements handling for WM_NCPOINTERDOWN, WM_NCPOINTERUP, and
WM_NCPOINTERUPDATE, by delegating to the existing touch event handler.

This patch also modifies the workaround frin crrev.com/c/1260507, since
the type of messages being translated into ui::ET_TOUCH_RELEASED events
is now different.

Bug:  897662 
Change-Id: I6fd052211b91a9a78834573deac830883972092b
Reviewed-on: https://chromium-review.googlesource.com/c/1330838
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608108}
[modify] https://crrev.com/fbdbb023f2619c745abcd8bcab3039688b8eded5/ui/views/win/hwnd_message_handler.cc
[modify] https://crrev.com/fbdbb023f2619c745abcd8bcab3039688b8eded5/ui/views/win/hwnd_message_handler.h
[modify] https://crrev.com/fbdbb023f2619c745abcd8bcab3039688b8eded5/ui/views/win/pen_event_processor.cc

Status: Fixed (was: Started)
Labels: TE-Verified-M72 TE-Verified-72.0.3611.0
Hi,

Retested the above issue on Win 10(Touch Device) using latest canary #72.0.3611.0 and issue is fixed.

Now, able to Min/Max/Close browser via Tap-touch.

Please refer the attached screen-cast for reference.

Thank You!
Canary Behaviour.mp4
971 KB View Download
Labels: Hotlist-DesktopUIChecked
Status: Verified (was: Fixed)
***UI Mass Triage***

Sign in to add a comment