Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2521453002:
[mac] Disable usage of full size content view. (Closed)
Description[mac] Disable usage of full size content view.
Usually AppKit gives the contentView of a framed NSWindow an
NSThemeFrame as its superview. For a long time, Chrome has hacked around
this requirement, so we have more control over the frame layout. We
would instead add our contentView as a sibling of NSThemeFrame. But
AppKit warns, "NSWindow warning: adding an unknown subview" when we do
this.
In r424609 we started using NSFullSizeContentViewWindowMask to avoid the
warning. However, NSThemeFrame uses autolayout for the "traffic
light" buttons.
The prevailing theory is that adding the browser view hierarchy as a
subview of NSThemeFrame (rather than a sibling) we are unexpectedly
increasing the exposure that the browser view layout has to autolayout;
layout constraints and the constraint resolution alogithm. This resulted
in some known performance and layout regressions (fixed in r427290 and
r432024).
There's concern of additional, yet unknown regressions. So we want to
revert to the old ways for m56 so there is time for additional analysis.
BUG=666415, 605219
Committed: https://crrev.com/fd7cfb578fb058ee68c19051fb1ecd564b563f73
Cr-Commit-Position: refs/heads/master@{#433658}
Patch Set 1 #Patch Set 2 : more comment #Messages
Total messages: 18 (12 generated)
The CQ bit was checked by tapted@chromium.org to run a CQ dry run
Description was changed from ========== [mac] Disable usage of full size content view. Usually AppKit gives the contentView of a framed NSWindow an NSThemeFrame as its superview. For a long time, Chrome has hacked around this requirement, so we have more control over the frame layout. We would instead add our contentView as a sibling of NSThemeFrame. But AppKit warns, "NSWindow warning: adding an unknown subview" when we do this. In r424609 we started using NSFullSizeContentViewWindowMask to avoid the warning. However, NSThemeFrame uses autolayout for the the "traffic light" buttons. The prevailing theory is that adding the browser view hierarchy as a subview of NSThemeFrame (rather than a sibling) we are unexpectedly increasing the exposure that the browser view layout has to autolayout; layout constraints and the constraint resolution alogithm. This resulted in some known performance and layout regressions (fixed in r427290 and r432024). There's concern of additional, yet unknown regressions. So we want to revert to the old ways for m56 so there is time for additional analysis. BUG=666415, 605219 ========== to ========== [mac] Disable usage of full size content view. Usually AppKit gives the contentView of a framed NSWindow an NSThemeFrame as its superview. For a long time, Chrome has hacked around this requirement, so we have more control over the frame layout. We would instead add our contentView as a sibling of NSThemeFrame. But AppKit warns, "NSWindow warning: adding an unknown subview" when we do this. In r424609 we started using NSFullSizeContentViewWindowMask to avoid the warning. However, NSThemeFrame uses autolayout for the the "traffic light" buttons. The prevailing theory is that adding the browser view hierarchy as a subview of NSThemeFrame (rather than a sibling) we are unexpectedly increasing the exposure that the browser view layout has to autolayout; layout constraints and the constraint resolution alogithm. This resulted in some known performance and layout regressions (fixed in r427290 and r432024). There's concern of additional, yet unknown regressions. So we want to revert to the old ways for m56 so there is time for additional analysis. BUG=666415, 605219 ==========
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
tapted@chromium.org changed reviewers: + shrike@chromium.org
Hi Jayson - WDYT? (Does this capture the rationale?)
tapted@chromium.org changed reviewers: + erikchen@chromium.org - shrike@chromium.org
+erik On 2016/11/21 04:39:35, tapted wrote: > Hi Jayson^H^H^H^H^H^HErik - WDYT? (Does this capture the rationale?) shrike->cc because OOO
thanks, lgtm
Description was changed from ========== [mac] Disable usage of full size content view. Usually AppKit gives the contentView of a framed NSWindow an NSThemeFrame as its superview. For a long time, Chrome has hacked around this requirement, so we have more control over the frame layout. We would instead add our contentView as a sibling of NSThemeFrame. But AppKit warns, "NSWindow warning: adding an unknown subview" when we do this. In r424609 we started using NSFullSizeContentViewWindowMask to avoid the warning. However, NSThemeFrame uses autolayout for the the "traffic light" buttons. The prevailing theory is that adding the browser view hierarchy as a subview of NSThemeFrame (rather than a sibling) we are unexpectedly increasing the exposure that the browser view layout has to autolayout; layout constraints and the constraint resolution alogithm. This resulted in some known performance and layout regressions (fixed in r427290 and r432024). There's concern of additional, yet unknown regressions. So we want to revert to the old ways for m56 so there is time for additional analysis. BUG=666415, 605219 ========== to ========== [mac] Disable usage of full size content view. Usually AppKit gives the contentView of a framed NSWindow an NSThemeFrame as its superview. For a long time, Chrome has hacked around this requirement, so we have more control over the frame layout. We would instead add our contentView as a sibling of NSThemeFrame. But AppKit warns, "NSWindow warning: adding an unknown subview" when we do this. In r424609 we started using NSFullSizeContentViewWindowMask to avoid the warning. However, NSThemeFrame uses autolayout for the "traffic light" buttons. The prevailing theory is that adding the browser view hierarchy as a subview of NSThemeFrame (rather than a sibling) we are unexpectedly increasing the exposure that the browser view layout has to autolayout; layout constraints and the constraint resolution alogithm. This resulted in some known performance and layout regressions (fixed in r427290 and r432024). There's concern of additional, yet unknown regressions. So we want to revert to the old ways for m56 so there is time for additional analysis. BUG=666415, 605219 ==========
The CQ bit was checked by tapted@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1479764108596900,
"parent_rev": "5a059f63806443f2722dc713ec8da3afe0b3e924", "commit_rev":
"2971f53f276c65dfb20b44c2eca5d3613bfbe9fc"}
Message was sent while issue was closed.
Description was changed from ========== [mac] Disable usage of full size content view. Usually AppKit gives the contentView of a framed NSWindow an NSThemeFrame as its superview. For a long time, Chrome has hacked around this requirement, so we have more control over the frame layout. We would instead add our contentView as a sibling of NSThemeFrame. But AppKit warns, "NSWindow warning: adding an unknown subview" when we do this. In r424609 we started using NSFullSizeContentViewWindowMask to avoid the warning. However, NSThemeFrame uses autolayout for the "traffic light" buttons. The prevailing theory is that adding the browser view hierarchy as a subview of NSThemeFrame (rather than a sibling) we are unexpectedly increasing the exposure that the browser view layout has to autolayout; layout constraints and the constraint resolution alogithm. This resulted in some known performance and layout regressions (fixed in r427290 and r432024). There's concern of additional, yet unknown regressions. So we want to revert to the old ways for m56 so there is time for additional analysis. BUG=666415, 605219 ========== to ========== [mac] Disable usage of full size content view. Usually AppKit gives the contentView of a framed NSWindow an NSThemeFrame as its superview. For a long time, Chrome has hacked around this requirement, so we have more control over the frame layout. We would instead add our contentView as a sibling of NSThemeFrame. But AppKit warns, "NSWindow warning: adding an unknown subview" when we do this. In r424609 we started using NSFullSizeContentViewWindowMask to avoid the warning. However, NSThemeFrame uses autolayout for the "traffic light" buttons. The prevailing theory is that adding the browser view hierarchy as a subview of NSThemeFrame (rather than a sibling) we are unexpectedly increasing the exposure that the browser view layout has to autolayout; layout constraints and the constraint resolution alogithm. This resulted in some known performance and layout regressions (fixed in r427290 and r432024). There's concern of additional, yet unknown regressions. So we want to revert to the old ways for m56 so there is time for additional analysis. BUG=666415, 605219 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== [mac] Disable usage of full size content view. Usually AppKit gives the contentView of a framed NSWindow an NSThemeFrame as its superview. For a long time, Chrome has hacked around this requirement, so we have more control over the frame layout. We would instead add our contentView as a sibling of NSThemeFrame. But AppKit warns, "NSWindow warning: adding an unknown subview" when we do this. In r424609 we started using NSFullSizeContentViewWindowMask to avoid the warning. However, NSThemeFrame uses autolayout for the "traffic light" buttons. The prevailing theory is that adding the browser view hierarchy as a subview of NSThemeFrame (rather than a sibling) we are unexpectedly increasing the exposure that the browser view layout has to autolayout; layout constraints and the constraint resolution alogithm. This resulted in some known performance and layout regressions (fixed in r427290 and r432024). There's concern of additional, yet unknown regressions. So we want to revert to the old ways for m56 so there is time for additional analysis. BUG=666415, 605219 ========== to ========== [mac] Disable usage of full size content view. Usually AppKit gives the contentView of a framed NSWindow an NSThemeFrame as its superview. For a long time, Chrome has hacked around this requirement, so we have more control over the frame layout. We would instead add our contentView as a sibling of NSThemeFrame. But AppKit warns, "NSWindow warning: adding an unknown subview" when we do this. In r424609 we started using NSFullSizeContentViewWindowMask to avoid the warning. However, NSThemeFrame uses autolayout for the "traffic light" buttons. The prevailing theory is that adding the browser view hierarchy as a subview of NSThemeFrame (rather than a sibling) we are unexpectedly increasing the exposure that the browser view layout has to autolayout; layout constraints and the constraint resolution alogithm. This resulted in some known performance and layout regressions (fixed in r427290 and r432024). There's concern of additional, yet unknown regressions. So we want to revert to the old ways for m56 so there is time for additional analysis. BUG=666415, 605219 Committed: https://crrev.com/fd7cfb578fb058ee68c19051fb1ecd564b563f73 Cr-Commit-Position: refs/heads/master@{#433658} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/fd7cfb578fb058ee68c19051fb1ecd564b563f73 Cr-Commit-Position: refs/heads/master@{#433658} |
