Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(123)

Issue 11273107: Fix BrowserFrameWin artifact on opaque to glass frame changes. (Closed)

Created:
8 years, 1 month ago by msw
Modified:
8 years, 1 month ago
CC:
chromium-reviews, tfarina
Visibility:
Public.

Description

Fix BrowserFrameWin artifact on opaque to glass frame changes. The problem when switching from an opaque frame to a glass frame: Calling UpdateDWMFrame() first uses stale glass inset values. (from the old OpaqueBrowserFrameView's tabstrip bounds, yielding 45px on top) HandleFrameChanged() then sets the new GlassBrowserFrameView with a larger top inset. (but doesn't update the dwm glass inset until the next UpdateDWMFrame, for 49px on top) Swapping the call order works without any apparent regressions. (this puts the new frame in place before updating the dwm glass insets) See http://crrev.com/29164 for a contradictory change that may be outdated. Also, use the more convenient IsFullscreen() call, no behavior change. BUG=156982 TEST=No black bar switching from theme->glass chrome on Windows; no regressions. R=ben@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=165265

Patch Set 1 #

Patch Set 2 : Cleanup. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+3 lines, -4 lines) Patch
M chrome/browser/ui/views/frame/browser_frame_win.cc View 1 2 chunks +3 lines, -4 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
msw
Hey Ben, please take a look; thanks!
8 years, 1 month ago (2012-10-31 19:57:27 UTC) #1
Ben Goodger (Google)
lgtm
8 years, 1 month ago (2012-10-31 19:59:48 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/msw@chromium.org/11273107/5001
8 years, 1 month ago (2012-10-31 20:03:17 UTC) #3
commit-bot: I haz the power
Step "update" is always a major failure. Look at the try server FAQ for more ...
8 years, 1 month ago (2012-10-31 21:51:12 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/msw@chromium.org/11273107/5001
8 years, 1 month ago (2012-10-31 21:51:46 UTC) #5
commit-bot: I haz the power
Step "update" is always a major failure. Look at the try server FAQ for more ...
8 years, 1 month ago (2012-10-31 23:43:14 UTC) #6
msw
8 years, 1 month ago (2012-10-31 23:55:30 UTC) #7
On 2012/10/31 23:43:14, I haz the power (commit-bot) wrote:
> Step "update" is always a major failure.
> Look at the try server FAQ for more details.

The linux_chromeos trybot fails update for myself and others:
http://build.chromium.org/p/tryserver.chromium/builders/linux_chromeos/builds...
http://build.chromium.org/p/tryserver.chromium/builders/linux_chromeos/builds...
All other bots passed, and this CL is Win-specific; landing manually.

Powered by Google App Engine
This is Rietveld 408576698