Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1198273003:
Get rid of old flow thread comments and asserts in accumulateOffsetTowardsAncestor(). (Closed)
|
Created:
5 years, 6 months ago by mstensho (USE GERRIT) Modified:
5 years, 6 months ago Reviewers:
chrishtr CC:
blink-reviews, blink-reviews-paint_chromium.org, dshwang, slimming-paint-reviews_chromium.org Base URL:
svn://svn.chromium.org/blink/trunk Target Ref:
refs/remotes/origin/master Project:
blink Visibility:
Public. |
DescriptionGet rid of old flow thread comments and asserts in accumulateOffsetTowardsAncestor().
R=chrishtr@chromium.org
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=197641
Patch Set 1 #
Total comments: 5
Patch Set 2 : rebase master for fun and profit #Messages
Total messages: 22 (9 generated)
Can you explain in more detail why these are useless comments now? I forget the details.
On 2015/06/22 18:04:16, chrishtr wrote: > Can you explain in more detail why these are useless comments now? I forget the > details. Comments at each change. https://codereview.chromium.org/1198273003/diff/1/Source/core/paint/Deprecate... File Source/core/paint/DeprecatedPaintLayer.cpp (left): https://codereview.chromium.org/1198273003/diff/1/Source/core/paint/Deprecate... Source/core/paint/DeprecatedPaintLayer.cpp:1269: // FIXME: Positioning of out-of-flow(fixed, absolute) elements collected in a LayoutFlowThread Comment from the early development stages in WebKit. Addressed in Blink some long time ago, with DeprecatedPaintLayer::visualOffsetFromAncestor() et al. https://codereview.chromium.org/1198273003/diff/1/Source/core/paint/Deprecate... Source/core/paint/DeprecatedPaintLayer.cpp:1271: // If the fixed layoutObject is inside a LayoutFlowThread, we should not compute location using localToAbsolute, This is a comment that survived the https://codereview.chromium.org/616453002 cleanup, but shouldn't have. There used to be a type of flow threads that acted as containers for fixed positioned descendants (something to do with CSS regions), but this is long gone. https://codereview.chromium.org/1198273003/diff/1/Source/core/paint/Deprecate... Source/core/paint/DeprecatedPaintLayer.cpp:1282: // For the fixed positioned elements inside a layout flow thread, we should also skip the code path below Leftovers from https://codereview.chromium.org/616453002 , again. https://codereview.chromium.org/1198273003/diff/1/Source/core/paint/Deprecate... Source/core/paint/DeprecatedPaintLayer.cpp:1335: // LayoutFlowThread is a positioned container, child of LayoutView, positioned at (0,0). CSS regions stuff. Those flow threads used to be direct children of LayoutView, and be a containing block for fixed positioned descendants. https://codereview.chromium.org/1198273003/diff/1/Source/core/paint/Deprecate... Source/core/paint/DeprecatedPaintLayer.cpp:1349: // We should not reach LayoutView layer past the LayoutFlowThread layer for any Nothing incorrect in this assertion as such, but it just feels like a strange place to suddenly start worrying about flow threads. Another relic from the CSS regions days.
The CQ bit was checked by chrishtr@chromium.org
lgtm
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1198273003/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_blink_rel on tryserver.blink (JOB_FAILED, http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/60158)
The CQ bit was checked by mstensho@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1198273003/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_blink_rel on tryserver.blink (JOB_FAILED, http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/60182)
The CQ bit was checked by mstensho@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1198273003/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_blink_rel on tryserver.blink (JOB_FAILED, http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/60241)
The CQ bit was checked by mstensho@opera.com
The patchset sent to the CQ was uploaded after l-g-t-m from chrishtr@chromium.org Link to the patchset: https://codereview.chromium.org/1198273003/#ps20001 (title: "rebase master for fun and profit")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1198273003/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_blink_rel on tryserver.blink (JOB_FAILED, http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/60254)
Message was sent while issue was closed.
Committed patchset #2 (id:20001) manually as 197641 (presubmit successful). |
