Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2423263002:
Create FloatingObject from NG Fragment in old Layout tree (Closed)
|
Created:
4 years, 2 months ago by Gleb Lanbin Modified:
4 years, 2 months ago CC:
chromium-reviews, ojan+watch_chromium.org, szager+layoutwatch_chromium.org, glebl+reviews_chromium.org, zoltan1, blink-reviews-layout_chromium.org, pdr+renderingwatchlist_chromium.org, eae+blinkwatch, leviw+renderwatch, jchaffraix+rendering, blink-reviews Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionCreate FloatingObject from NG Fragment in old Layout tree
This will create FloatingObjects from fragments in old Layout tree. FloatingObjects are required by BlockFlowPainter that uses them to paint floats.
BUG=635619
Committed: https://crrev.com/bf964eb2871efa5f52041b13c3f9a7dfa4b93a24
Cr-Commit-Position: refs/heads/master@{#425896}
Patch Set 1 #
Total comments: 2
Patch Set 2 : add DCHECK(layout_box_->parent()) #Messages
Total messages: 18 (10 generated)
glebl@chromium.org changed reviewers: + cbiesinger@chromium.org, ikilpatrick@chromium.org
https://codereview.chromium.org/2423263002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/layout/ng/ng_box.cc (right): https://codereview.chromium.org/2423263002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/layout/ng/ng_box.cc:112: if (layout_box_->isFloating() && parent->isLayoutBlockFlow()) { You're accessing layout_box_ without a nullcheck here. Probably the first nullcheck in this function should just be an early return. Why do you need the parent argument instead of using layout_box_->parent()?
https://codereview.chromium.org/2423263002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/layout/ng/ng_box.cc (right): https://codereview.chromium.org/2423263002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/layout/ng/ng_box.cc:112: if (layout_box_->isFloating() && parent->isLayoutBlockFlow()) { On 2016/10/17 21:00:34, cbiesinger wrote: > You're accessing layout_box_ without a nullcheck here. > > Probably the first nullcheck in this function should just be an early return. > > Why do you need the parent argument instead of using layout_box_->parent()? Done.
lgtm
lgtm
The CQ bit was checked by glebl@chromium.org to run a CQ dry run
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 glebl@chromium.org
The CQ bit was checked by glebl@chromium.org to run a CQ dry run
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.
The CQ bit was checked by glebl@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Create FloatingObject from NG Fragment in old Layout tree This will create FloatingObjects from fragments in old Layout tree. FloatingObjects are required by BlockFlowPainter that uses them to paint floats. BUG=635619 ========== to ========== Create FloatingObject from NG Fragment in old Layout tree This will create FloatingObjects from fragments in old Layout tree. FloatingObjects are required by BlockFlowPainter that uses them to paint floats. BUG=635619 Committed: https://crrev.com/bf964eb2871efa5f52041b13c3f9a7dfa4b93a24 Cr-Commit-Position: refs/heads/master@{#425896} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/bf964eb2871efa5f52041b13c3f9a7dfa4b93a24 Cr-Commit-Position: refs/heads/master@{#425896} |
