FramedBrowserWindowTest.DoesHideTitle, BrowserTest.GetSizeForNewRenderView fails on Mac 10.11 tests |
|||||||||||||
Issue descriptionFirst appeared in https://build.chromium.org/p/chromium.mac/builders/Mac10.11%20Tests/builds/2868 ===================================================== [ RUN ] BrowserTest.GetSizeForNewRenderView 2016-10-11 22:07:33.582 browser_tests[56567:626527] Unable to simultaneously satisfy constraints: ( "<NSAutoresizingMaskLayoutConstraint:0x7f978d3fb140 h=-&- v=&-- V:[BookmarkBarToolbarView:0x7f978d4060a0(0)]>", "<NSAutoresizingMaskLayoutConstraint:0x7f978d3fabf0 h=-&- v=-&- V:|-(6)-[BookmarkBarView:0x7f978b578440] (Names: '|':BookmarkBarToolbarView:0x7f978d4060a0 )>", "<NSAutoresizingMaskLayoutConstraint:0x7f978d3faba0 h=-&- v=-&- V:[BookmarkBarView:0x7f978b578440]-(6)-| (Names: '|':BookmarkBarToolbarView:0x7f978d4060a0 )>" ) Will attempt to recover by breaking constraint <NSAutoresizingMaskLayoutConstraint:0x7f978d3fabf0 h=-&- v=-&- V:|-(6)-[BookmarkBarView:0x7f978b578440] (Names: '|':BookmarkBarToolbarView:0x7f978d4060a0 )> Set the NSUserDefault NSConstraintBasedLayoutVisualizeMutuallyExclusiveConstraints to YES to have -[NSWindow visualizeConstraints:] automatically called when this happens. And/or, break on objc_exception_throw to catch this in the debugger. ../../chrome/browser/ui/browser_browsertest.cc:2609: Failure Value of: rwhv_commit_size0 Actual: 980x523 Expected: rwhv_create_size0 Which is: 980x563 ../../chrome/browser/ui/browser_browsertest.cc:2616: Failure Value of: web_contents->GetRenderWidgetHostView()->GetViewBounds().size() Actual: 980x563 Expected: rwhv_commit_size0 Which is: 980x523 [56567:93255:1011/220733:WARNING:embedded_test_server.cc(201)] Request not handled. Returning 404: /favicon.ico [56567:95491:1011/220733:WARNING:embedded_test_server.cc(201)] Request not handled. Returning 404: /favicon.ico [ FAILED ] BrowserTest.GetSizeForNewRenderView, where TypeParam = and GetParam() = (2849 ms) ===================================================== FindIt found https://codereview.chromium.org/2404783002 as the suspects. This is believable, as it's a Mac-specific CL dealing with window sizes. Apparently this CL was also addressed by the previous sheriff, and a fix was landed afterwards - https://codereview.chromium.org/2416513002/ Therefore, I'm not reverting in order not to bring this to an inconsistent state. I'll disable the tests instead.
,
Oct 12 2016
Two more failures appeared on the same bot: FramedBrowserWindowTest.WindowWidgetLocation FramedBrowserWindowTest.DoesHideTitle ===================================================== [ RUN ] FramedBrowserWindowTest.WindowWidgetLocation ../../chrome/browser/ui/cocoa/framed_browser_window_unittest.mm:166: Failure Value of: NSMaxY(windowBounds) - kFramedWindowButtonsWithTabStripOffsetFromTop Actual: 589 Expected: NSMaxY(closeBoxFrame) Which is: 19 ../../chrome/browser/ui/cocoa/framed_browser_window_unittest.mm:168: Failure Value of: kFramedWindowButtonsWithTabStripOffsetFromLeft Actual: 11 Expected: NSMinX(closeBoxFrame) Which is: 7 ../../chrome/browser/ui/cocoa/framed_browser_window_unittest.mm:175: Failure Value of: NSMaxY(windowBounds) - kFramedWindowButtonsWithTabStripOffsetFromTop Actual: 589 Expected: NSMaxY(miniaturizeFrame) Which is: 19 [ FAILED ] FramedBrowserWindowTest.WindowWidgetLocation (28 ms) ===================================================== [ RUN ] FramedBrowserWindowTest.DoesHideTitle ../../chrome/browser/ui/cocoa/framed_browser_window_unittest.mm:101: Failure Value of: [emptyTitleData isEqualToData:hiddenTitleData] Actual: false Expected: true [ FAILED ] FramedBrowserWindowTest.DoesHideTitle (24 ms) ===================================================== FindIt suspects the abovementioned fix https://codereview.chromium.org/2416513002/. Perhaps I should have reverted both CLs instead... Eugene, PTAL ASAP!
,
Oct 12 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ede24242bd8b521e122812c3c703020840eac153 commit ede24242bd8b521e122812c3c703020840eac153 Author: msramek <msramek@chromium.org> Date: Wed Oct 12 13:17:28 2016 Disable BrowserTest.GetSizeForNewRenderView on Mac Broken in https://codereview.chromium.org/2404783002. TBR=sky@chromium.org BUG= 655112 Review-Url: https://codereview.chromium.org/2412653004 Cr-Commit-Position: refs/heads/master@{#424724} [modify] https://crrev.com/ede24242bd8b521e122812c3c703020840eac153/chrome/browser/ui/browser_browsertest.cc
,
Oct 12 2016
...so let me disable the other two tests as well.
,
Oct 12 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cd7d6f600a2b4d2095fe7bd6d94af29289075c8b commit cd7d6f600a2b4d2095fe7bd6d94af29289075c8b Author: msramek <msramek@chromium.org> Date: Wed Oct 12 13:28:01 2016 Disable two cocoa FramedBrowserWindowTest tests. The following tests: FramedBrowserWindowTest.WindowWidgetLocation FramedBrowserWindowTest.DoesHideTitle were broken by https://codereview.chromium.org/2416513002/. TBR=eugenebut@chromium.org,tapted@chromium.org BUG= 655112 Review-Url: https://codereview.chromium.org/2414673002 Cr-Commit-Position: refs/heads/master@{#424725} [modify] https://crrev.com/cd7d6f600a2b4d2095fe7bd6d94af29289075c8b/chrome/browser/ui/cocoa/framed_browser_window_unittest.mm
,
Oct 12 2016
CL which broke these tests has landed behind runtime flags. So if we won't find a quick fix then we can set that flag to false until we find a solution.
,
Oct 13 2016
FramedBrowserWindowTest.WindowWidgetLocation fix on review: https://codereview.chromium.org/2418703002/#
,
Oct 14 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0bd2a26b4420c4d4abb0f1101111e16c8f65c91c commit 0bd2a26b4420c4d4abb0f1101111e16c8f65c91c Author: eugenebut <eugenebut@chromium.org> Date: Fri Oct 14 00:10:40 2016 [mac] Fixed FramedBrowserWindowTest.WindowWidgetLocation. - call [window_ layoutIfNeeded] to update buttons position, which is controlled by layout constraints - use window coordinates for verifying button locations BUG= 655112 , 605219 TBR=tapted@chromium.org Review-Url: https://codereview.chromium.org/2418703002 Cr-Commit-Position: refs/heads/master@{#425211} [modify] https://crrev.com/0bd2a26b4420c4d4abb0f1101111e16c8f65c91c/chrome/browser/ui/cocoa/framed_browser_window_unittest.mm
,
Oct 14 2016
FramedBrowserWindowTest.DoesHideTitle failure is caused by cl/2404783002 but it's a test problem rather than regression in functionality. Title is successfully hidden, but for some reason the test can not draw the window. CGWindowListCreate returns test window which size is 0:0, but I don't know why window with full contents mask behaves differently.
,
Oct 14 2016
BrowserTest.GetSizeForNewRenderView works fine if we don't set NSFullSizeContentViewWindowMask by changing |shouldUseFullSizeContentView| to be always |false|: https://cs.chromium.org/chromium/src/chrome/browser/ui/cocoa/framed_browser_window.mm?q=bool+shouldUseFullSizeContentView%5C+%3D&sq=package:chromium&dr=C&l=108 This can be an actual regression caused by this new window mask.
,
Oct 14 2016
Trent, could you please find a new owner for this bug as I'm switching back to iOS.
,
Oct 20 2016
Ugh. autolayout. Let's first decode the message
2016-10-11 22:07:33.582 browser_tests[56567:626527] Unable to simultaneously satisfy constraints:
(
"<NSAutoresizingMaskLayoutConstraint:0x7f978d3fb140 h=-&- v=&-- V:[BookmarkBarToolbarView:0x7f978d4060a0(0)]>",
"<NSAutoresizingMaskLayoutConstraint:0x7f978d3fabf0 h=-&- v=-&- V:|-(6)-[BookmarkBarView:0x7f978b578440] (Names: '|':BookmarkBarToolbarView:0x7f978d4060a0 )>",
"<NSAutoresizingMaskLayoutConstraint:0x7f978d3faba0 h=-&- v=-&- V:[BookmarkBarView:0x7f978b578440]-(6)-| (Names: '|':BookmarkBarToolbarView:0x7f978d4060a0 )>"
)
Will attempt to recover by breaking constraint
<NSAutoresizingMaskLayoutConstraint:0x7f978d3fabf0 h=-&- v=-&- V:|-(6)-[BookmarkBarView:0x7f978b578440] (Names: '|':BookmarkBarToolbarView:0x7f978d4060a0 )>
- First, note BookmarkBarView is a subview of BookmarkBarToolbarView
- NSAutoresizingMaskLayoutConstraint means these are coming from the view property `translatesAutoresizingMaskIntoConstraints:YES`
- "h=-&- v=&--" is the autoresizing mask. It means left-margin, right-margin, bottom-margin and height are fixed; width and top-margin are flexible.
- "h=-&- v=-&-" means all margins are fixed, width/height is flexible.
- V:[BookmarkBarToolbarView:0x7f978d4060a0(0)]> means BookmarkBarToolbarView wants a fixed height of 0
- V:|-(6)-[BookmarkBarView:0x7f978b578440] means BookmarkBarView wants a fixed 6-DIP padding above it
- V:[BookmarkBarView:0x7f978b578440]-(6)-| means BookmarkBarView wants a fixed 6-DIP padding below it
To hold that padding BookmarkBarToolbarView needs to be at least 12 DIP tall, but we just set the height to 0.
This is happening in the test because:
- It sets the bookmark to "detached" mode
- It starts on chrome://ntp -- "detached" bookmarks bar is visible
- Navigates away from NTP - "detached" bookmarks bar should go away
- Unlike the "attached" bookmarks bar, a detached on has a padding of 6 DIP above/below it while on the NTP.
So that "6" is probably
// The amount of space between the inner bookmark bar and the outer toolbar on
// new tab pages.
const int kNTPBookmarkBarPadding =
(chrome::kNTPBookmarkBarHeight -
(chrome::kMinimumBookmarkBarHeight + kVisualHeightOffset)) /
2;
= (40 - (26 + 2)) / 2
= 6
The stack where this happens is
frame #12: 0x00007fff9525e90d AppKit`-[NSView setFrame:] + 476
::-[BrowserWindowController resizeView:newHeight:](self=0x0000000149c04330, _cmd="resizeView:newHeight:", view=0x000000014fe70b80, height=0) + 2435 at browser_window_controller.mm:956
::-[AnimatableView setHeight:](self=0x000000014fe70b80, _cmd="setHeight:", newHeight=0) + 85 at animatable_view.mm:70
::-[BookmarkBarController showBookmarkBarWithAnimation:](self=0x000000014fe64090, _cmd="showBookmarkBarWithAnimation:", animate=NO) + 167 at bookmark_bar_controller.mm:1029
::-[BookmarkBarController updateVisibility](self=0x000000014fe64090, _cmd="updateVisibility") + 45 at bookmark_bar_controller.mm:594
::-[BookmarkBarController finalizeState](self=0x000000014fe64090, _cmd="finalizeState") + 144 at bookmark_bar_controller.mm:1574
::-[BookmarkBarController moveToState:withAnimation:](self=0x000000014fe64090, _cmd="moveToState:withAnimation:", nextState=HIDDEN, animate=NO) + 521 at bookmark_bar_controller.mm:1549
::-[BookmarkBarController updateState:changeType:](self=0x000000014fe64090, _cmd="updateState:changeType:", newState=HIDDEN, changeType=DONT_ANIMATE_STATE_CHANGE) + 98 at bookmark_bar_controller.mm:1557
BrowserWindowCocoa::BookmarkBarStateChanged(this=0x0000000149e14b20, change_type=DONT_ANIMATE_STATE_CHANGE) + 80 at browser_window_cocoa.mm:325
Browser::UpdateBookmarkBarState(this=0x00000001499930b0, reason=BOOKMARK_BAR_STATE_CHANGE_TAB_STATE) + 1998 at browser.cc:2485
Browser::DidNavigateMainFramePostCommit(this=0x00000001499930b0, web_contents=0x000000014b826e00) + 103 at browser.cc:1647
...
frame #61: 0x00000001044368c7 browser_tests`ui_test_utils::NavigateToURL(browser=0x00000001499930b0, url=0x00007fff5fbf9348) + 39 at ui_test_utils.cc:169
frame #62: 0x0000000101eb1a85 browser_tests`BrowserTest_DISABLED_GetSizeForNewRenderView_Test::RunTestOnMainThread(this=0x0000000149fa5400) + 4325 at browser_browsertest.cc:2598
kNTPBookmarkBarPadding is applied in -[BookmarkBarController layoutSubviews]:
// Add padding to the detached bookmark bar.
// The state of our morph (if any); 1 is total bubble, 0 is the regular bar.
CGFloat morph = [self detachedMorphProgress];
CGFloat padding = 0;
padding = bookmarks::kNTPBookmarkBarPadding;
buttonViewFrame =
NSInsetRect(buttonViewFrame, morph * padding, morph * padding);
[buttonView_ setFrame:buttonViewFrame];
But layoutSubviews is called at the end of showBookmarkBarWithAnimation; the height of the superview is set first, to an "invalid" value since the subviews haven't been updated yet.
Long story short, we can make this warning go away by changing the autoresize mask of BookmarkBarView to allow flexible padding. But that doesn't fix the test failure.
So I guess that's just a big ol' red herring. Fine. Let's fix it anyway - https://codereview.chromium.org/2435593008
,
Oct 20 2016
So for this bit:
../../chrome/browser/ui/browser_browsertest.cc:2615: Failure
Value of: rwhv_commit_size0
Actual: 925x899
Expected: rwhv_create_size0
Which is: 925x939
../../chrome/browser/ui/browser_browsertest.cc:2622: Failure
Value of: web_contents->GetRenderWidgetHostView()->GetViewBounds().size()
Actual: 925x939
Expected: rwhv_commit_size0
Which is: 925x899
it looks like CreateRenderWidgetHostViewForRenderManager is creating the webcontents of the correct size -- -[RenderWidgetHostViewCocoa setFrameSize:] is called with {925, 939} at this point. But then something in autolayout diddles with the frame size and sizes it to {925, 899}:
17 AppKit 0x00007fff952d35d8 -[NSView layoutSubtreeIfNeeded] + 950
18 AppKit 0x00007fff952f2f21 -[NSWindow(NSConstraintBasedLayout) _layoutViewTree] + 82
19 AppKit 0x00007fff953654ff -[NSWindow(NSConstraintBasedLayout) layoutIfNeeded] + 244
20 AppKit 0x00007fff959fc155 ___NSWindowGetDisplayCycleObserver_block_invoke6358 + 218
21 AppKit 0x00007fff953774f8 __37+[NSDisplayCycle currentDisplayCycle]_block_invoke + 719
22 QuartzCore 0x00007fff94e22f71 _ZN2CA11Transaction19run_commit_handlersE18CATransactionPhase + 85
23 QuartzCore 0x00007fff94e2242c _ZN2CA7Context18commit_transactionEPNS_11TransactionE + 160
24 QuartzCore 0x00007fff94e220ec _ZN2CA11Transaction6commitEv + 508
25 QuartzCore 0x00007fff94e2d977 _ZN2CA11Transaction17observer_callbackEP19__CFRunLoopObservermPv + 71
26 CoreFoundation 0x00007fff8d28f067 __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__ + 23
27 CoreFoundation 0x00007fff8d28efd7 __CFRunLoopDoObservers + 391
28 CoreFoundation 0x00007fff8d26def8 CFRunLoopRunSpecific + 328
29 HIToolbox 0x00007fff8ce87935 RunCurrentEventLoopInMode + 235
30 HIToolbox 0x00007fff8ce87677 ReceiveNextEventCommon + 184
31 HIToolbox 0x00007fff8ce875af _BlockUntilNextEventMatchingListInModeWithFilter + 71
32 AppKit 0x00007fff9521fdf6 _DPSNextEvent + 1067
33 AppKit 0x00007fff9521f226 -[NSApplication _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 454
34 AppKit 0x00007fff95213d80 -[NSApplication run] + 682
There's nothing form Chrome higher up in the stack. It's as though by using NSFullSizeContentMask we're opting in to some extra cycles from CoreAnimation to perform autolayout. Gross.
We probably need to do something to the view hierarchy before calling CreateRenderWidgetHostViewForRenderManager.
Still digging... I'll compare this stacks with what happens with the NSFullSizeContentMask disabled.
,
Oct 21 2016
OK - I think I have a fix for this in https://codereview.chromium.org/2442573003/ It's basically an alternate way to fix Issue 264207 which avoids having the renderer participate in these extra autolayouts that CoreAnimation is triggering while we wait for a commit.
,
Oct 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e6144c977967c4ebbf5dedf8706e4ed5aca65124 commit e6144c977967c4ebbf5dedf8706e4ed5aca65124 Author: tapted <tapted@chromium.org> Date: Sat Oct 22 01:19:39 2016 Cocoa: Fix a constraint violation when navigating away from a detached bookmark bar. On 10.11+, when navigating away from the NTP with the bookmarks bar "detached", AppKit currently complains about a constraint violation when setting the bookmarks bar height to zero. It seems to occur because the subview previously asked for 6 DIP of padding on the top and bottom which won't fit in a height of 0 DIP. The subview height is set to 0 DIP shortly after. BookmarkBarView currently says it has a flexible height and fixed padding. Change it to a fixed height and flexible padding to stop AppKit complaining. While investigating, improve the encapsulation of -layoutSubviews slightly. BrowserWindowController called it after setting the view's frame manually and unnecessarily tweaking (a different set of) layout constraints. DCHECK instead. BUG= 655112 Review-Url: https://chromiumcodereview.appspot.com/2435593008 Cr-Commit-Position: refs/heads/master@{#426944} [modify] https://crrev.com/e6144c977967c4ebbf5dedf8706e4ed5aca65124/chrome/app/nibs/BookmarkBar.xib [modify] https://crrev.com/e6144c977967c4ebbf5dedf8706e4ed5aca65124/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.h [modify] https://crrev.com/e6144c977967c4ebbf5dedf8706e4ed5aca65124/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.mm [modify] https://crrev.com/e6144c977967c4ebbf5dedf8706e4ed5aca65124/chrome/browser/ui/cocoa/browser_window_controller_private.mm
,
Oct 25 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ba268401a975c321f1e15ae3a4776fd3c60e268e commit ba268401a975c321f1e15ae3a4776fd3c60e268e Author: tapted <tapted@chromium.org> Date: Tue Oct 25 06:41:26 2016 Mac: Don't allow RenderWidgetHostViewCocoa to participate in autolayout. WebContentsViewCocoa has a hook to resize all subviews to match its size. The problem is that the size of a RenderWidgetHostViewCocoa is allowed to get out of sync with its superview while waiting for a WebContents paint to be committed. If an autolayout is triggered while waiting for that commit, the WebContents thinks it's been resized and spawns a new paint. Since r424609, for 10.11+, Chrome stopped replacing the NSThemeFrame (which AppKit does not support) and instead started using NSFullSizeContentViewWindowMask. This seems to opt the window into additional autolayout triggers, coming from CoreAnimation. This can engage the code to "re-sync" the sizes of the RenderWidgetHostViewCocoa and WebContentsViewCocoa when it wasn't done previously. To fix, "re-sync" sizes in an override of -setFrameSize: rather than -resizeSubviewsWithOldSize:. This ensures a re-sync only occurs when the size of the WebContentsViewCocoa changes. BUG= 655112 , 655665 , 264207 TBR=sky@chromium.org Review-Url: https://codereview.chromium.org/2442573003 Cr-Commit-Position: refs/heads/master@{#427290} [modify] https://crrev.com/ba268401a975c321f1e15ae3a4776fd3c60e268e/chrome/browser/ui/browser_browsertest.cc [modify] https://crrev.com/ba268401a975c321f1e15ae3a4776fd3c60e268e/content/browser/web_contents/web_contents_view_mac.mm
,
Oct 25 2016
GetSizeForNewRenderView is happy in https://build.chromium.org/p/chromium.mac/builders/Mac10.11%20Tests/builds/2959 so.. barring any further regressions, this should be fixed (\o/).
,
Oct 25 2016
ohhh, I think there's still FramedBrowserWindowTest.DoesHideTitle to fix. Doh.
,
Jan 27 2017
I'm not currently looking at this. It should be addressed if we try to fix Issue 605219 again. CL to re-enable FramedBrowserWindowTest.DoesHideTitle is in https://codereview.chromium.org/2659623003
,
Jan 31 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9423656f22bc945169a6337b4e2001ec67ba9646 commit 9423656f22bc945169a6337b4e2001ec67ba9646 Author: tapted <tapted@chromium.org> Date: Tue Jan 31 16:55:50 2017 Re-enable FramedBrowserWindowTest.DoesHideTitle This started failing on 10.11 after r424609 landed, which was a change to start using NSFullSizeContentViewWindowMask for the browser window. However, that's been disabled since r433658, so this test should keep running. BUG= 655112 Review-Url: https://codereview.chromium.org/2659623003 Cr-Commit-Position: refs/heads/master@{#447247} [modify] https://crrev.com/9423656f22bc945169a6337b4e2001ec67ba9646/chrome/browser/ui/cocoa/framed_browser_window_unittest.mm
,
Jan 31 2017
,
Jan 31 2017
,
Feb 23 2017
,
Mar 17 2017
I just verified that this last test failure is tied to NSFullSizeContentViewWindowMask, not Auto Layout.
,
Jul 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/45f7994fbcf0e40e02e4c4823a07b9b6ef28a325 commit 45f7994fbcf0e40e02e4c4823a07b9b6ef28a325 Author: Sidney San Martín <sdy@chromium.org> Date: Tue Jul 11 07:05:57 2017 Enable ShouldUseFullSizeContentView(). Also fixes the known issues with enabling it: - crbug/665787: The tab background view was added in front of the content and blocked interaction with the top of the scroll bar when the toolbar was hidden. It's now inserted behind the content, like before. - crbug/655112: FramedBrowserWindowTest.DoesHideTitle took screenshots of the frame view, which gets drawn using a different (layer-backed?) path when NSFullSizeContentViewWindowMask is on. The test now calls -[CATransaction flush] before taking each screenshot, and uses a different API to screenshot the whole window instead of trying to grab just the frame view by accessing the content view's superview. Bug: 605219 , 655112 , 665787 , 730679 , 734713 , 737206 Change-Id: Id78c21aff6748e02fd4b3e461c77ef9f7454d744 Reviewed-on: https://chromium-review.googlesource.com/566030 Commit-Queue: Sidney San Martin <sdy@chromium.org> Reviewed-by: Trent Apted <tapted@chromium.org> Cr-Commit-Position: refs/heads/master@{#485544} [modify] https://crrev.com/45f7994fbcf0e40e02e4c4823a07b9b6ef28a325/chrome/browser/ui/cocoa/browser_window_layout.mm [modify] https://crrev.com/45f7994fbcf0e40e02e4c4823a07b9b6ef28a325/chrome/browser/ui/cocoa/framed_browser_window_unittest.mm [modify] https://crrev.com/45f7994fbcf0e40e02e4c4823a07b9b6ef28a325/chrome/browser/ui/cocoa/tabs/tab_window_controller.mm
,
Jul 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/685e99063e6bac1d4417829ec3b6abdcb918caf6 commit 685e99063e6bac1d4417829ec3b6abdcb918caf6 Author: Keishi Hattori <keishi@chromium.org> Date: Tue Jul 11 08:55:01 2017 Revert "Enable ShouldUseFullSizeContentView()." This reverts commit 45f7994fbcf0e40e02e4c4823a07b9b6ef28a325. Reason for revert: Broke BrowserWindowControllerTest.UsesAutoLayout Original change's description: > Enable ShouldUseFullSizeContentView(). > > Also fixes the known issues with enabling it: > > - crbug/665787: The tab background view was added in front of the content and > blocked interaction with the top of the scroll bar when the toolbar was > hidden. It's now inserted behind the content, like before. > > - crbug/655112: FramedBrowserWindowTest.DoesHideTitle took screenshots of the > frame view, which gets drawn using a different (layer-backed?) path when > NSFullSizeContentViewWindowMask is on. The test now calls -[CATransaction > flush] before taking each screenshot, and uses a different API to screenshot > the whole window instead of trying to grab just the frame view by accessing > the content view's superview. > > Bug: 605219 , 655112 , 665787 , 730679 , 734713 , 737206 > Change-Id: Id78c21aff6748e02fd4b3e461c77ef9f7454d744 > Reviewed-on: https://chromium-review.googlesource.com/566030 > Commit-Queue: Sidney San Martin <sdy@chromium.org> > Reviewed-by: Trent Apted <tapted@chromium.org> > Cr-Commit-Position: refs/heads/master@{#485544} TBR=tapted@chromium.org,shrike@chromium.org,sdy@chromium.org Change-Id: I00b73bc5d2d5607736ddc81b9ae81ec65527ebad No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 605219 , 655112 , 665787 , 730679 , 734713 , 737206 Reviewed-on: https://chromium-review.googlesource.com/566898 Reviewed-by: Keishi Hattori <keishi@chromium.org> Commit-Queue: Keishi Hattori <keishi@chromium.org> Cr-Commit-Position: refs/heads/master@{#485564} [modify] https://crrev.com/685e99063e6bac1d4417829ec3b6abdcb918caf6/chrome/browser/ui/cocoa/browser_window_layout.mm [modify] https://crrev.com/685e99063e6bac1d4417829ec3b6abdcb918caf6/chrome/browser/ui/cocoa/framed_browser_window_unittest.mm [modify] https://crrev.com/685e99063e6bac1d4417829ec3b6abdcb918caf6/chrome/browser/ui/cocoa/tabs/tab_window_controller.mm
,
Jul 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a20a025fb10592190b5b3bc90fd627c45c76676a commit a20a025fb10592190b5b3bc90fd627c45c76676a Author: Sidney San Martín <sdy@chromium.org> Date: Tue Jul 11 18:39:06 2017 Enable ShouldUseFullSizeContentView(). Also fixes the known issues with enabling it: - crbug/665787: The tab background view was added in front of the content and blocked interaction with the top of the scroll bar when the toolbar was hidden. It's now inserted behind the content, like before. - crbug/655112: FramedBrowserWindowTest.DoesHideTitle took screenshots of the frame view, which gets drawn using a different (layer-backed?) path when NSFullSizeContentViewWindowMask is on. The test now calls -[CATransaction flush] before taking each screenshot, and uses a different API to screenshot the whole window instead of trying to grab just the frame view by accessing the content view's superview. This is a re-land of r485544 (reverted in r485564) with a test fix for BrowserWindowControllerTest.UsesAutoLayout. Bug: 605219 , 655112 , 665787 , 730679 , 734713 , 737206 Change-Id: I59c03e90b30613285df89a1b3694f36de3f1a7d8 Reviewed-on: https://chromium-review.googlesource.com/567205 Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org> Commit-Queue: Sidney San Martin <sdy@chromium.org> Cr-Commit-Position: refs/heads/master@{#485682} [modify] https://crrev.com/a20a025fb10592190b5b3bc90fd627c45c76676a/chrome/browser/ui/cocoa/browser_window_controller_unittest.mm [modify] https://crrev.com/a20a025fb10592190b5b3bc90fd627c45c76676a/chrome/browser/ui/cocoa/browser_window_layout.mm [modify] https://crrev.com/a20a025fb10592190b5b3bc90fd627c45c76676a/chrome/browser/ui/cocoa/framed_browser_window_unittest.mm [modify] https://crrev.com/a20a025fb10592190b5b3bc90fd627c45c76676a/chrome/browser/ui/cocoa/tabs/tab_window_controller.mm
,
Jul 12 2017
|
|||||||||||||
►
Sign in to add a comment |
|||||||||||||
Comment 1 by msramek@chromium.org
, Oct 12 2016