Issue metadata
Sign in to add a comment
|
Regression: Min/Max/Close buttons are not seen on 'Thanks for feedback window tab-strip.
Reported by
aiman.an...@etouch.net,
Nov 12
|
||||||||||||||||||||||||
Issue descriptionChrome Version: 72.0.3608.0 (Official Build) 13a876533812d5e196bca2b1c60634dc14a79700-refs/branch-heads/3608@{#1} (32/64 Bit) OS: Windows 10 only. Steps to reproduce: 1. Launch chrome, right click on wrench icon and Select ''Report an Issue' from Help option. 2. On Feedback overlay, enter some text and Send Feedback. 3. Observe the tab strip of 'Thanks for feedback window'. Actual Result: Min/Max/Close buttons are not seen on Tab-strip. (Respective action is being performed) Expected Result: Min/Max/Close buttons should be seen on 'Thanks for feedback window tab-strip. This is Regression Issue broken in M-72, and below is the bisect info. Good Build: 72.0.3605.0 (Revision:606282) Bad Build: 72.0.3606.0 (Revision:606693) Narrow Bisect: Change-Log URL: https://chromium.googlesource.com/chromium/src/+log/72.0.3605.0..72.0.3606.0?pretty=fuller&n=10000 Suspect: r606668 ? collinbaker@: 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. Tried performing 'per revision' bisect on multiple Windows and Mac machines but unable to perform the same since getting "RuntimeError: We don't have enough builds to bisect." error. 2. Unable to perform Chromium bisect as 'Report an Issue' feature is not available on chromium. 3. Hence providing suspect manually. 4. Issue is Win 10 specific and is not seen on Win(7,8,8.1), Mac(10.13.1,10.13.6,10.14.2) and Linux(14.04 LTS) OS. Kindly refer the attached screen-cast. Thank You!
,
Nov 13
The bug is still present when I revert my commit. I'm unassigning myself since I suspect my CL is not related.
,
Nov 13
,
Nov 13
This isn't related to the tabstrip. I don't know if the dialog is a constrained dialog, or a bubble.
,
Nov 21
Friendly ping! Could you please provide any update on this issue as it has been marked as a stable blocker. Thank You!
,
Nov 21
Update: Able to reproduce this issue on Windows-10 machine using latest Canary #72.0.3617.0. Attaching screen-cast for the same. Also checked the manual regression range and found the same as mentioned in description of bug i.e. good build: 72.0.3605.0 & bad build: 72.0.3606.0 Thank you.
,
Nov 26
Gentle ping! Could you please provide any update on this issue as it has been marked as a stable blocker. Thank You!
,
Dec 3
Gentle ping! Could you please provide any update on this issue as it has been marked as a stable blocker. Thank You!
,
Dec 10
Hi, Just to update: Retested the above issue using latest canary 73.0.3635.0 on Win 10 OS and issue is still seen. Header for 'Thanks for feedback' window is not seen. Attaching below screen-cast for reference. Thank You!
,
Dec 11
I did a full bisect on this. It looks like https://chromium.googlesource.com/chromium/src/+/eec86ff6c03b228b426a4553c89724d04e0a0f56 is the culprit. I'm assigning to sunnyps@, the author of that commit.
,
Dec 11
cc bsep@ might be related to custom titlebar
,
Dec 11
basically we want the browser to say that the surface is not opaque if we need to blend with OS drawn titlebar and if that doesn't happen it could manifest in this way
,
Dec 11
When custom titlebar is on, we always say the browser is opaque, so I wonder if that's the trouble. See DesktopWindowTreeHostWin::ShouldWindowContentsBeTransparent, which always returns false when ShouldUseNativeFrame is false.
,
Dec 11
First of all, debugging this isn't easy because it requires a src-internals checkout. Plus I couldn't find the code that launches the post feedback window in Chromium or internally. It's probably launched by the feedback extension somehow.
However, after reading the code I think this is the problematic bit:
bool BrowserDesktopWindowTreeHostWin::ShouldWindowContentsBeTransparent()
const {
return !ShouldCustomDrawSystemTitlebar() &&
views::DesktopWindowTreeHostWin::ShouldWindowContentsBeTransparent();
}
https://cs.chromium.org/chromium/src/chrome/browser/ui/views/frame/browser_desktop_window_tree_host_win.cc?rcl=630f1690aac12a8a96b9e4edccae8718834c3a68&l=267
If ShouldCustomDrawSystemTitlebar is true, this will always return false irrespective of the other conditions in BrowserDesktopWindowTreeHostWin::ShouldUseNativeFrame() called by DesktopWindowTreeHostWin::ShouldWindowContentsBeTransparent().
For example this condition should make the window transparent but won't:
// We don't theme popup or app windows, so regardless of whether or not a
// theme is active for normal browser windows, we don't want to use the custom
// frame for popups/apps.
if (!browser_view_->IsBrowserTypeNormal())
return true;
I think it should have an OR instead of an AND:
!ShouldCustomDrawSystemTitlebar() || views::DesktopWindowTreeHostWin::ShouldWindowContentsBeTransparent()
Then the window will always be transparent if we're not custom drawing the titlebar whether by default or for a particular window.
,
Dec 12
I've been trying to debug this. I haven't gotten very far, but I did figure out a couple potentially useful things: - This is a problem for Chrome app windows in general, not just the report issue landing page window (which for some reason is an app window). I can reproduce this in the Postman app and the NaCl dev shell app. - The problem is probably not in BrowserDesktopWindowTreeHostWin, since I'm pretty sure that isn't used for app windows. - The issue is probably in //chrome/browser/ui/views/apps/ or in //ui/views/widget/desktop_aura somewhere. //extensions/browser/app_window/ may be related too.
,
Dec 12
I guess it's not related to custom titlebar after all. sunnyps@: does the fact that it's not restricted to that one surface help debug?
,
Dec 12
> - The problem is probably not in BrowserDesktopWindowTreeHostWin, since I'm pretty sure that isn't used for app windows.
I'll look into this a bit more since I should be able to repro on Chromium with NaCl app windows, but I'm not sure about this part. There are comments in BrowserDesktopWindowTreeHostWin::ShouldUseNativeFrame() which talk about the app window vs normal browser window distinction:
// We don't theme popup or app windows, so regardless of whether or not a
// theme is active for normal browser windows, we don't want to use the custom
// frame for popups/apps.
if (!browser_view_->IsBrowserTypeNormal())
return true;
And I'm pretty sure that every Chrome window has a DesktopWindowTreeHost and I don't know what other implementation of that an app window would use.
,
Dec 14
Issue 915137 has been merged into this issue.
,
Dec 19
Friendly ping! Could you please provide any update on this issue as it has been marked as a stable blocker. Thank You!
,
Dec 21
Fix in review: https://chromium-review.googlesource.com/c/chromium/src/+/1382928 Should land tomorrow hopefully. I'll be OOO from 12/23 so zmo@ please cherry-pick onto M72 once approved (landed and baked in Canary)
,
Dec 22
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5ad4e08d15110e8212fae941d4638e2e4df7e4c1 commit 5ad4e08d15110e8212fae941d4638e2e4df7e4c1 Author: Sunny Sachanandani <sunnyps@chromium.org> Date: Sat Dec 22 00:08:41 2018 Fix app window titlebar blending with direct composition Clear the DWM frame area on WM_ERASEBKGND so that Chrome's client area blends with DWM frame for app windows. Refactor the code to update and clear DWM frame and move it to hwnd message handler so that browser and app windows can share the same code. This mimics existing logic for updating the DWM frame and adds the clear DWM frame behavior to app windows, but one notable change is that it will clear on every WM_ERASEBKGND message, and not just the first one. This shouldn't have a performance impact and seems more correct anyway. Bug: 904322 Change-Id: I70c3ae97a94114dd63110736a35cef4dd887b1aa Reviewed-on: https://chromium-review.googlesource.com/c/1382928 Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: Bret Sepulveda <bsep@chromium.org> Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org> Cr-Commit-Position: refs/heads/master@{#618684} [modify] https://crrev.com/5ad4e08d15110e8212fae941d4638e2e4df7e4c1/chrome/browser/ui/views/apps/app_window_desktop_window_tree_host_win.cc [modify] https://crrev.com/5ad4e08d15110e8212fae941d4638e2e4df7e4c1/chrome/browser/ui/views/apps/app_window_desktop_window_tree_host_win.h [modify] https://crrev.com/5ad4e08d15110e8212fae941d4638e2e4df7e4c1/chrome/browser/ui/views/frame/browser_desktop_window_tree_host_win.cc [modify] https://crrev.com/5ad4e08d15110e8212fae941d4638e2e4df7e4c1/chrome/browser/ui/views/frame/browser_desktop_window_tree_host_win.h [modify] https://crrev.com/5ad4e08d15110e8212fae941d4638e2e4df7e4c1/ui/views/widget/desktop_aura/desktop_window_tree_host_win.cc [modify] https://crrev.com/5ad4e08d15110e8212fae941d4638e2e4df7e4c1/ui/views/widget/desktop_aura/desktop_window_tree_host_win.h [modify] https://crrev.com/5ad4e08d15110e8212fae941d4638e2e4df7e4c1/ui/views/win/hwnd_message_handler.cc [modify] https://crrev.com/5ad4e08d15110e8212fae941d4638e2e4df7e4c1/ui/views/win/hwnd_message_handler.h [modify] https://crrev.com/5ad4e08d15110e8212fae941d4638e2e4df7e4c1/ui/views/win/hwnd_message_handler_delegate.h
,
Dec 22
Requesting merge after this bakes in Canary.
,
Dec 23
This bug requires manual review: M72 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Dec 24
Update: Retested the above issue on Windows (10) using latest Canary build #73.0.3650.0 and issue is fixed. Now, as per commit 5ad4e08d15110e8212fae941d4638e2e4df7e4c1 post feedback url opens in separate tab, hence original issue is fixed. Kindly refer the attached screen-cast. Thank You!
,
Dec 26
Approving merge for M72. Branch:3626
,
Dec 26
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2dd99ab580747edaea6fb0c1ad46526059e900c4 commit 2dd99ab580747edaea6fb0c1ad46526059e900c4 Author: Sunny Sachanandani <sunnyps@chromium.org> Date: Wed Dec 26 20:36:42 2018 Fix app window titlebar blending with direct composition Clear the DWM frame area on WM_ERASEBKGND so that Chrome's client area blends with DWM frame for app windows. Refactor the code to update and clear DWM frame and move it to hwnd message handler so that browser and app windows can share the same code. This mimics existing logic for updating the DWM frame and adds the clear DWM frame behavior to app windows, but one notable change is that it will clear on every WM_ERASEBKGND message, and not just the first one. This shouldn't have a performance impact and seems more correct anyway. Bug: 904322 Change-Id: I70c3ae97a94114dd63110736a35cef4dd887b1aa Reviewed-on: https://chromium-review.googlesource.com/c/1382928 Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: Bret Sepulveda <bsep@chromium.org> Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#618684}(cherry picked from commit 5ad4e08d15110e8212fae941d4638e2e4df7e4c1) Reviewed-on: https://chromium-review.googlesource.com/c/1391319 Reviewed-by: Zhenyao Mo <zmo@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#521} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} [modify] https://crrev.com/2dd99ab580747edaea6fb0c1ad46526059e900c4/chrome/browser/ui/views/apps/app_window_desktop_window_tree_host_win.cc [modify] https://crrev.com/2dd99ab580747edaea6fb0c1ad46526059e900c4/chrome/browser/ui/views/apps/app_window_desktop_window_tree_host_win.h [modify] https://crrev.com/2dd99ab580747edaea6fb0c1ad46526059e900c4/chrome/browser/ui/views/frame/browser_desktop_window_tree_host_win.cc [modify] https://crrev.com/2dd99ab580747edaea6fb0c1ad46526059e900c4/chrome/browser/ui/views/frame/browser_desktop_window_tree_host_win.h [modify] https://crrev.com/2dd99ab580747edaea6fb0c1ad46526059e900c4/ui/views/widget/desktop_aura/desktop_window_tree_host_win.cc [modify] https://crrev.com/2dd99ab580747edaea6fb0c1ad46526059e900c4/ui/views/widget/desktop_aura/desktop_window_tree_host_win.h [modify] https://crrev.com/2dd99ab580747edaea6fb0c1ad46526059e900c4/ui/views/win/hwnd_message_handler.cc [modify] https://crrev.com/2dd99ab580747edaea6fb0c1ad46526059e900c4/ui/views/win/hwnd_message_handler.h [modify] https://crrev.com/2dd99ab580747edaea6fb0c1ad46526059e900c4/ui/views/win/hwnd_message_handler_delegate.h
,
Dec 26
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2dd99ab580747edaea6fb0c1ad46526059e900c4 Commit: 2dd99ab580747edaea6fb0c1ad46526059e900c4 Author: sunnyps@chromium.org Commiter: zmo@chromium.org Date: 2018-12-26 20:36:42 +0000 UTC Fix app window titlebar blending with direct composition Clear the DWM frame area on WM_ERASEBKGND so that Chrome's client area blends with DWM frame for app windows. Refactor the code to update and clear DWM frame and move it to hwnd message handler so that browser and app windows can share the same code. This mimics existing logic for updating the DWM frame and adds the clear DWM frame behavior to app windows, but one notable change is that it will clear on every WM_ERASEBKGND message, and not just the first one. This shouldn't have a performance impact and seems more correct anyway. Bug: 904322 Change-Id: I70c3ae97a94114dd63110736a35cef4dd887b1aa Reviewed-on: https://chromium-review.googlesource.com/c/1382928 Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: Bret Sepulveda <bsep@chromium.org> Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#618684}(cherry picked from commit 5ad4e08d15110e8212fae941d4638e2e4df7e4c1) Reviewed-on: https://chromium-review.googlesource.com/c/1391319 Reviewed-by: Zhenyao Mo <zmo@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#521} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
,
Dec 27
,
Jan 9
Update: Retested the above issue on Windows (10) OS using latest Beta build #72.0.3626.53 and issue is fixed. Now, as per commit 5ad4e08d15110e8212fae941d4638e2e4df7e4c1 post feedback url opens in separate tab, hence original issue is fixed. Kindly refer the attached screen-cast. Thank You!
,
Jan 11
,
Jan 11
,
Jan 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/99f5438aba0fb5e137221ee18f6c6969f4654d0e commit 99f5438aba0fb5e137221ee18f6c6969f4654d0e Author: Zhenyao Mo <zmo@chromium.org> Date: Fri Jan 11 21:50:56 2019 Revert "Fix app window titlebar blending with direct composition" This reverts commit 5ad4e08d15110e8212fae941d4638e2e4df7e4c1. Reason for revert: crbug.com/918461 Original change's description: > Fix app window titlebar blending with direct composition > > Clear the DWM frame area on WM_ERASEBKGND so that Chrome's client area > blends with DWM frame for app windows. Refactor the code to update and > clear DWM frame and move it to hwnd message handler so that browser and > app windows can share the same code. > > This mimics existing logic for updating the DWM frame and adds the clear > DWM frame behavior to app windows, but one notable change is that it > will clear on every WM_ERASEBKGND message, and not just the first one. > This shouldn't have a performance impact and seems more correct anyway. > > Bug: 904322 > Change-Id: I70c3ae97a94114dd63110736a35cef4dd887b1aa > Reviewed-on: https://chromium-review.googlesource.com/c/1382928 > Reviewed-by: Scott Violet <sky@chromium.org> > Reviewed-by: Bret Sepulveda <bsep@chromium.org> > Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org> > Cr-Commit-Position: refs/heads/master@{#618684} TBR=sky@chromium.org,sunnyps@chromium.org,bsep@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 904322 , 918461 Change-Id: I681f37f264884cdfab7425eadfbd6c26bbf2d953 Reviewed-on: https://chromium-review.googlesource.com/c/1407602 Commit-Queue: Zhenyao Mo <zmo@chromium.org> Reviewed-by: Zhenyao Mo <zmo@chromium.org> Cr-Commit-Position: refs/heads/master@{#622168} [modify] https://crrev.com/99f5438aba0fb5e137221ee18f6c6969f4654d0e/chrome/browser/ui/views/apps/app_window_desktop_window_tree_host_win.cc [modify] https://crrev.com/99f5438aba0fb5e137221ee18f6c6969f4654d0e/chrome/browser/ui/views/apps/app_window_desktop_window_tree_host_win.h [modify] https://crrev.com/99f5438aba0fb5e137221ee18f6c6969f4654d0e/chrome/browser/ui/views/frame/browser_desktop_window_tree_host_win.cc [modify] https://crrev.com/99f5438aba0fb5e137221ee18f6c6969f4654d0e/chrome/browser/ui/views/frame/browser_desktop_window_tree_host_win.h [modify] https://crrev.com/99f5438aba0fb5e137221ee18f6c6969f4654d0e/ui/views/widget/desktop_aura/desktop_window_tree_host_win.cc [modify] https://crrev.com/99f5438aba0fb5e137221ee18f6c6969f4654d0e/ui/views/widget/desktop_aura/desktop_window_tree_host_win.h [modify] https://crrev.com/99f5438aba0fb5e137221ee18f6c6969f4654d0e/ui/views/win/hwnd_message_handler.cc [modify] https://crrev.com/99f5438aba0fb5e137221ee18f6c6969f4654d0e/ui/views/win/hwnd_message_handler.h [modify] https://crrev.com/99f5438aba0fb5e137221ee18f6c6969f4654d0e/ui/views/win/hwnd_message_handler_delegate.h
,
Jan 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/507aaf39e1ef42cb9389832582f11fe14f4104ee Commit: 507aaf39e1ef42cb9389832582f11fe14f4104ee Author: zmo@chromium.org Commiter: zmo@chromium.org Date: 2019-01-11 22:00:25 +0000 UTC Revert "Fix app window titlebar blending with direct composition" This reverts commit 2dd99ab580747edaea6fb0c1ad46526059e900c4. Reason for revert: crbug.com/918461 Original change's description: > Fix app window titlebar blending with direct composition > > Clear the DWM frame area on WM_ERASEBKGND so that Chrome's client area > blends with DWM frame for app windows. Refactor the code to update and > clear DWM frame and move it to hwnd message handler so that browser and > app windows can share the same code. > > This mimics existing logic for updating the DWM frame and adds the clear > DWM frame behavior to app windows, but one notable change is that it > will clear on every WM_ERASEBKGND message, and not just the first one. > This shouldn't have a performance impact and seems more correct anyway. > > Bug: 904322 > Change-Id: I70c3ae97a94114dd63110736a35cef4dd887b1aa > Reviewed-on: https://chromium-review.googlesource.com/c/1382928 > Reviewed-by: Scott Violet <sky@chromium.org> > Reviewed-by: Bret Sepulveda <bsep@chromium.org> > Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org> > Cr-Original-Commit-Position: refs/heads/master@{#618684}(cherry picked from commit 5ad4e08d15110e8212fae941d4638e2e4df7e4c1) > Reviewed-on: https://chromium-review.googlesource.com/c/1391319 > Reviewed-by: Zhenyao Mo <zmo@chromium.org> > Cr-Commit-Position: refs/branch-heads/3626@{#521} > Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} TBR=zmo@chromium.org,sunnyps@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 904322 , 918461 Change-Id: If2281410ba24efa0850d7c4116a10adc1473cf5f Reviewed-on: https://chromium-review.googlesource.com/c/1407657 Reviewed-by: Zhenyao Mo <zmo@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#652} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
,
Jan 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/507aaf39e1ef42cb9389832582f11fe14f4104ee commit 507aaf39e1ef42cb9389832582f11fe14f4104ee Author: Zhenyao Mo <zmo@chromium.org> Date: Fri Jan 11 22:00:25 2019 Revert "Fix app window titlebar blending with direct composition" This reverts commit 2dd99ab580747edaea6fb0c1ad46526059e900c4. Reason for revert: crbug.com/918461 Original change's description: > Fix app window titlebar blending with direct composition > > Clear the DWM frame area on WM_ERASEBKGND so that Chrome's client area > blends with DWM frame for app windows. Refactor the code to update and > clear DWM frame and move it to hwnd message handler so that browser and > app windows can share the same code. > > This mimics existing logic for updating the DWM frame and adds the clear > DWM frame behavior to app windows, but one notable change is that it > will clear on every WM_ERASEBKGND message, and not just the first one. > This shouldn't have a performance impact and seems more correct anyway. > > Bug: 904322 > Change-Id: I70c3ae97a94114dd63110736a35cef4dd887b1aa > Reviewed-on: https://chromium-review.googlesource.com/c/1382928 > Reviewed-by: Scott Violet <sky@chromium.org> > Reviewed-by: Bret Sepulveda <bsep@chromium.org> > Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org> > Cr-Original-Commit-Position: refs/heads/master@{#618684}(cherry picked from commit 5ad4e08d15110e8212fae941d4638e2e4df7e4c1) > Reviewed-on: https://chromium-review.googlesource.com/c/1391319 > Reviewed-by: Zhenyao Mo <zmo@chromium.org> > Cr-Commit-Position: refs/branch-heads/3626@{#521} > Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} TBR=zmo@chromium.org,sunnyps@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 904322 , 918461 Change-Id: If2281410ba24efa0850d7c4116a10adc1473cf5f Reviewed-on: https://chromium-review.googlesource.com/c/1407657 Reviewed-by: Zhenyao Mo <zmo@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#652} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} [modify] https://crrev.com/507aaf39e1ef42cb9389832582f11fe14f4104ee/chrome/browser/ui/views/apps/app_window_desktop_window_tree_host_win.cc [modify] https://crrev.com/507aaf39e1ef42cb9389832582f11fe14f4104ee/chrome/browser/ui/views/apps/app_window_desktop_window_tree_host_win.h [modify] https://crrev.com/507aaf39e1ef42cb9389832582f11fe14f4104ee/chrome/browser/ui/views/frame/browser_desktop_window_tree_host_win.cc [modify] https://crrev.com/507aaf39e1ef42cb9389832582f11fe14f4104ee/chrome/browser/ui/views/frame/browser_desktop_window_tree_host_win.h [modify] https://crrev.com/507aaf39e1ef42cb9389832582f11fe14f4104ee/ui/views/widget/desktop_aura/desktop_window_tree_host_win.cc [modify] https://crrev.com/507aaf39e1ef42cb9389832582f11fe14f4104ee/ui/views/widget/desktop_aura/desktop_window_tree_host_win.h [modify] https://crrev.com/507aaf39e1ef42cb9389832582f11fe14f4104ee/ui/views/win/hwnd_message_handler.cc [modify] https://crrev.com/507aaf39e1ef42cb9389832582f11fe14f4104ee/ui/views/win/hwnd_message_handler.h [modify] https://crrev.com/507aaf39e1ef42cb9389832582f11fe14f4104ee/ui/views/win/hwnd_message_handler_delegate.h
,
Jan 14
[bulk update] Just a heads up, M72 stable is about 2 weeks away. This issue is marked as RB-Stable. Please take a look.
,
Jan 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/55e596a4337e58480882e56d5fd31627eb1dd668 commit 55e596a4337e58480882e56d5fd31627eb1dd668 Author: Sunny Sachanandani <sunnyps@chromium.org> Date: Tue Jan 15 23:57:46 2019 Reland "Fix app window titlebar blending with direct composition" This is a reland of 5ad4e08d15110e8212fae941d4638e2e4df7e4c1 The only change is that we clear the DWM frame on the first WM_ERASEBKGND message after frame type changes instead of doing it for every message. This fixes https://crbug.com/918461 Original change's description: > Fix app window titlebar blending with direct composition > > Clear the DWM frame area on WM_ERASEBKGND so that Chrome's client area > blends with DWM frame for app windows. Refactor the code to update and > clear DWM frame and move it to hwnd message handler so that browser and > app windows can share the same code. > > This mimics existing logic for updating the DWM frame and adds the clear > DWM frame behavior to app windows, but one notable change is that it > will clear on every WM_ERASEBKGND message, and not just the first one. > This shouldn't have a performance impact and seems more correct anyway. > > Bug: 904322 > Change-Id: I70c3ae97a94114dd63110736a35cef4dd887b1aa > Reviewed-on: https://chromium-review.googlesource.com/c/1382928 > Reviewed-by: Scott Violet <sky@chromium.org> > Reviewed-by: Bret Sepulveda <bsep@chromium.org> > Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org> > Cr-Commit-Position: refs/heads/master@{#618684} Bug: 904322 , 918461 Change-Id: I032ff424d204cb20d724f6be26b3e6fc8d5d0ec0 Reviewed-on: https://chromium-review.googlesource.com/c/1410388 Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org> Cr-Commit-Position: refs/heads/master@{#622883} [modify] https://crrev.com/55e596a4337e58480882e56d5fd31627eb1dd668/chrome/browser/ui/views/apps/app_window_desktop_window_tree_host_win.cc [modify] https://crrev.com/55e596a4337e58480882e56d5fd31627eb1dd668/chrome/browser/ui/views/apps/app_window_desktop_window_tree_host_win.h [modify] https://crrev.com/55e596a4337e58480882e56d5fd31627eb1dd668/chrome/browser/ui/views/frame/browser_desktop_window_tree_host_win.cc [modify] https://crrev.com/55e596a4337e58480882e56d5fd31627eb1dd668/chrome/browser/ui/views/frame/browser_desktop_window_tree_host_win.h [modify] https://crrev.com/55e596a4337e58480882e56d5fd31627eb1dd668/ui/views/widget/desktop_aura/desktop_window_tree_host_win.cc [modify] https://crrev.com/55e596a4337e58480882e56d5fd31627eb1dd668/ui/views/widget/desktop_aura/desktop_window_tree_host_win.h [modify] https://crrev.com/55e596a4337e58480882e56d5fd31627eb1dd668/ui/views/win/hwnd_message_handler.cc [modify] https://crrev.com/55e596a4337e58480882e56d5fd31627eb1dd668/ui/views/win/hwnd_message_handler.h [modify] https://crrev.com/55e596a4337e58480882e56d5fd31627eb1dd668/ui/views/win/hwnd_message_handler_delegate.h
,
Jan 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/55e596a4337e58480882e56d5fd31627eb1dd668 commit 55e596a4337e58480882e56d5fd31627eb1dd668 Author: Sunny Sachanandani <sunnyps@chromium.org> Date: Tue Jan 15 23:57:46 2019 Reland "Fix app window titlebar blending with direct composition" This is a reland of 5ad4e08d15110e8212fae941d4638e2e4df7e4c1 The only change is that we clear the DWM frame on the first WM_ERASEBKGND message after frame type changes instead of doing it for every message. This fixes https://crbug.com/918461 Original change's description: > Fix app window titlebar blending with direct composition > > Clear the DWM frame area on WM_ERASEBKGND so that Chrome's client area > blends with DWM frame for app windows. Refactor the code to update and > clear DWM frame and move it to hwnd message handler so that browser and > app windows can share the same code. > > This mimics existing logic for updating the DWM frame and adds the clear > DWM frame behavior to app windows, but one notable change is that it > will clear on every WM_ERASEBKGND message, and not just the first one. > This shouldn't have a performance impact and seems more correct anyway. > > Bug: 904322 > Change-Id: I70c3ae97a94114dd63110736a35cef4dd887b1aa > Reviewed-on: https://chromium-review.googlesource.com/c/1382928 > Reviewed-by: Scott Violet <sky@chromium.org> > Reviewed-by: Bret Sepulveda <bsep@chromium.org> > Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org> > Cr-Commit-Position: refs/heads/master@{#618684} Bug: 904322 , 918461 Change-Id: I032ff424d204cb20d724f6be26b3e6fc8d5d0ec0 Reviewed-on: https://chromium-review.googlesource.com/c/1410388 Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org> Cr-Commit-Position: refs/heads/master@{#622883} [modify] https://crrev.com/55e596a4337e58480882e56d5fd31627eb1dd668/chrome/browser/ui/views/apps/app_window_desktop_window_tree_host_win.cc [modify] https://crrev.com/55e596a4337e58480882e56d5fd31627eb1dd668/chrome/browser/ui/views/apps/app_window_desktop_window_tree_host_win.h [modify] https://crrev.com/55e596a4337e58480882e56d5fd31627eb1dd668/chrome/browser/ui/views/frame/browser_desktop_window_tree_host_win.cc [modify] https://crrev.com/55e596a4337e58480882e56d5fd31627eb1dd668/chrome/browser/ui/views/frame/browser_desktop_window_tree_host_win.h [modify] https://crrev.com/55e596a4337e58480882e56d5fd31627eb1dd668/ui/views/widget/desktop_aura/desktop_window_tree_host_win.cc [modify] https://crrev.com/55e596a4337e58480882e56d5fd31627eb1dd668/ui/views/widget/desktop_aura/desktop_window_tree_host_win.h [modify] https://crrev.com/55e596a4337e58480882e56d5fd31627eb1dd668/ui/views/win/hwnd_message_handler.cc [modify] https://crrev.com/55e596a4337e58480882e56d5fd31627eb1dd668/ui/views/win/hwnd_message_handler.h [modify] https://crrev.com/55e596a4337e58480882e56d5fd31627eb1dd668/ui/views/win/hwnd_message_handler_delegate.h
,
Jan 16
(6 days ago)
Update: Retested the above issue on Windows (10) using latest Beta build #72.0.3626.64 and Canary build #73.0.3673.0 and issue is fixed. Now,post feedback url opens in separate tab, hence original issue is fixed. Kindly refer the attached screen-cast. Thank You!
,
Jan 16
(6 days ago)
Requesting merge to M72 for CL in #36 CL is present in Canary 73.0.3673.0 and both issue 918461 (reason for reverting) and this one are verified as fixed. abdulsyed@ please approve this if you think this is important to fix. Note that this affects all NaCl app windows and not just the "Thanks for feedback" page.
,
Jan 16
(6 days ago)
This bug requires manual review: We are only 12 days from stable. Please contact the milestone owner if you have questions. Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jan 16
(6 days ago)
Is this only happening in the Thanks for feedback window. Or are there more important scenarios? What is the fix in #37 compared to what landed earlier?
,
Jan 16
(6 days ago)
I've seen it happen for other NaCl apps like the Google Keep and Postman too. The only change in #37 (#36 is the same thing - a double post by bugdroid) compared to previous is for fixing issue 918461 which reverts a small change in behavior in the CL. From the CL description: "The only change is that we clear the DWM frame on the first WM_ERASEBKGND message after frame type changes instead of doing it for every message. This fixes https://crbug.com/918461 "
,
Jan 18
(5 days ago)
abdulsyed@ ping
,
Jan 18
(4 days ago)
Thanks - approved for m72. branch:3626
,
Jan 18
(4 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fe5f6cceb5028adf1b1cb3e9a586ec7de4741170 commit fe5f6cceb5028adf1b1cb3e9a586ec7de4741170 Author: Sunny Sachanandani <sunnyps@chromium.org> Date: Fri Jan 18 21:42:52 2019 [merge to m72] Reland "Fix app window titlebar blending with direct composition" This is a reland of 5ad4e08d15110e8212fae941d4638e2e4df7e4c1 The only change is that we clear the DWM frame on the first WM_ERASEBKGND message after frame type changes instead of doing it for every message. This fixes https://crbug.com/918461 Original change's description: > Fix app window titlebar blending with direct composition > > Clear the DWM frame area on WM_ERASEBKGND so that Chrome's client area > blends with DWM frame for app windows. Refactor the code to update and > clear DWM frame and move it to hwnd message handler so that browser and > app windows can share the same code. > > This mimics existing logic for updating the DWM frame and adds the clear > DWM frame behavior to app windows, but one notable change is that it > will clear on every WM_ERASEBKGND message, and not just the first one. > This shouldn't have a performance impact and seems more correct anyway. > > Bug: 904322 > Change-Id: I70c3ae97a94114dd63110736a35cef4dd887b1aa > Reviewed-on: https://chromium-review.googlesource.com/c/1382928 > Reviewed-by: Scott Violet <sky@chromium.org> > Reviewed-by: Bret Sepulveda <bsep@chromium.org> > Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org> > Cr-Commit-Position: refs/heads/master@{#618684} Bug: 904322 , 918461 Change-Id: I032ff424d204cb20d724f6be26b3e6fc8d5d0ec0 Reviewed-on: https://chromium-review.googlesource.com/c/1410388 Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#622883}(cherry picked from commit 55e596a4337e58480882e56d5fd31627eb1dd668) Reviewed-on: https://chromium-review.googlesource.com/c/1423113 Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#736} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} [modify] https://crrev.com/fe5f6cceb5028adf1b1cb3e9a586ec7de4741170/chrome/browser/ui/views/apps/app_window_desktop_window_tree_host_win.cc [modify] https://crrev.com/fe5f6cceb5028adf1b1cb3e9a586ec7de4741170/chrome/browser/ui/views/apps/app_window_desktop_window_tree_host_win.h [modify] https://crrev.com/fe5f6cceb5028adf1b1cb3e9a586ec7de4741170/chrome/browser/ui/views/frame/browser_desktop_window_tree_host_win.cc [modify] https://crrev.com/fe5f6cceb5028adf1b1cb3e9a586ec7de4741170/chrome/browser/ui/views/frame/browser_desktop_window_tree_host_win.h [modify] https://crrev.com/fe5f6cceb5028adf1b1cb3e9a586ec7de4741170/ui/views/widget/desktop_aura/desktop_window_tree_host_win.cc [modify] https://crrev.com/fe5f6cceb5028adf1b1cb3e9a586ec7de4741170/ui/views/widget/desktop_aura/desktop_window_tree_host_win.h [modify] https://crrev.com/fe5f6cceb5028adf1b1cb3e9a586ec7de4741170/ui/views/win/hwnd_message_handler.cc [modify] https://crrev.com/fe5f6cceb5028adf1b1cb3e9a586ec7de4741170/ui/views/win/hwnd_message_handler.h [modify] https://crrev.com/fe5f6cceb5028adf1b1cb3e9a586ec7de4741170/ui/views/win/hwnd_message_handler_delegate.h
,
Jan 18
(4 days ago)
,
Jan 18
(4 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fe5f6cceb5028adf1b1cb3e9a586ec7de4741170 Commit: fe5f6cceb5028adf1b1cb3e9a586ec7de4741170 Author: sunnyps@chromium.org Commiter: sunnyps@chromium.org Date: 2019-01-18 21:42:52 +0000 UTC [merge to m72] Reland "Fix app window titlebar blending with direct composition" This is a reland of 5ad4e08d15110e8212fae941d4638e2e4df7e4c1 The only change is that we clear the DWM frame on the first WM_ERASEBKGND message after frame type changes instead of doing it for every message. This fixes https://crbug.com/918461 Original change's description: > Fix app window titlebar blending with direct composition > > Clear the DWM frame area on WM_ERASEBKGND so that Chrome's client area > blends with DWM frame for app windows. Refactor the code to update and > clear DWM frame and move it to hwnd message handler so that browser and > app windows can share the same code. > > This mimics existing logic for updating the DWM frame and adds the clear > DWM frame behavior to app windows, but one notable change is that it > will clear on every WM_ERASEBKGND message, and not just the first one. > This shouldn't have a performance impact and seems more correct anyway. > > Bug: 904322 > Change-Id: I70c3ae97a94114dd63110736a35cef4dd887b1aa > Reviewed-on: https://chromium-review.googlesource.com/c/1382928 > Reviewed-by: Scott Violet <sky@chromium.org> > Reviewed-by: Bret Sepulveda <bsep@chromium.org> > Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org> > Cr-Commit-Position: refs/heads/master@{#618684} Bug: 904322 , 918461 Change-Id: I032ff424d204cb20d724f6be26b3e6fc8d5d0ec0 Reviewed-on: https://chromium-review.googlesource.com/c/1410388 Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#622883}(cherry picked from commit 55e596a4337e58480882e56d5fd31627eb1dd668) Reviewed-on: https://chromium-review.googlesource.com/c/1423113 Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#736} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} |
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by nyerramilli@chromium.org
, Nov 12Labels: ReleaseBlock-Stable