Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2930963002:
[LayoutNG] Refactor of NGLineBreaker. (Closed)
|
Created:
3 years, 6 months ago by ikilpatrick Modified:
3 years, 6 months ago CC:
atotic+reviews_chromium.org, blink-reviews, blink-reviews-layout_chromium.org, cbiesinger, chromium-reviews, dgrogan+ng_chromium.org, eae+blinkwatch, glebl+reviews_chromium.org, jchaffraix+rendering, lchoi+reviews_chromium.org, leviw+renderwatch, ojan+watch_chromium.org, pdr+renderingwatchlist_chromium.org, szager+layoutwatch_chromium.org, zoltan1 Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
Description[LayoutNG] Refactor of NGLineBreaker.
This patch does a few things because I got slightly carried away:
- Removes the fact that NGLineBreaker takens an NGInlineAlgorithm as an
argument.
- Has better logic around resolving the BfcOffset of the container and
positioning floats. We were previously doing this incorrectly.
Specifically resolving too early if we just had floats.
- Removes the available_width_ class member and makes it a stack
allocated WTF::Optional for clarity now.
- Tries to simplify a few other things.
BUG=635619
Review-Url: https://codereview.chromium.org/2930963002
Cr-Commit-Position: refs/heads/master@{#479016}
Committed: https://chromium.googlesource.com/chromium/src/+/f723897c7994e4ceee81dba57a6a3b94bc974a05
Patch Set 1 #
Total comments: 11
Patch Set 2 : comments. #Patch Set 3 : rebase + move function #Patch Set 4 : rebase #Messages
Total messages: 37 (23 generated)
Description was changed from ========== [LayoutNG] Refactor of NGLineBreaker. This patch does a few things because I got slightly carried away: - Removes the fact that NGLineBreaker takens an NGInlineAlgorithm as an argument. - Has better logic around resolving the BfcOffset of the container and positioning floats. We were previously doing this incorrectly. Specifically resolving too early if we just had floats. - Removes the available_width_ class member and makes it a stack allocated WTF::Optional for clarity now. - Tries to simplify a few other things. BUG=635619 ========== to ========== [LayoutNG] Refactor of NGLineBreaker. This patch does a few things because I got slightly carried away: - Removes the fact that NGLineBreaker takens an NGInlineAlgorithm as an argument. - Has better logic around resolving the BfcOffset of the container and positioning floats. We were previously doing this incorrectly. Specifically resolving too early if we just had floats. - Removes the available_width_ class member and makes it a stack allocated WTF::Optional for clarity now. - Tries to simplify a few other things. BUG=635619 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_layout_ng ==========
ikilpatrick@chromium.org changed reviewers: + cbiesinger@chromium.org, kojii@chromium.org
This fixes a bunch of tests which I'll update the testexpectations in a bit. A few other meta-thoughts while working on this patch: - Should NGLineBreaker & NGInlineLayoutAlgorithm be switch to using inline_size instead of width? - Considering adding a new NGBfcOffset type specifically for block layout. - Considering adding terminology for offset relative to the direct parent as "local_offset" instead of just "offset".
The CQ bit was checked by ikilpatrick@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...
https://codereview.chromium.org/2930963002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/layout/ng/inline/ng_line_breaker.cc (right): https://codereview.chromium.org/2930963002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/layout/ng/inline/ng_line_breaker.cc:130: // DO NOT SUBMIT: maybe kControl as well? kojii - halp! :D
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
Thank you for working on this. I guess I need to study a bit more to discuss, sorry about that. Replies from my current knowledge. Replies are inline, but for your comments: > - Should NGLineBreaker & NGInlineLayoutAlgorithm be switch to using inline_size instead of width? NGLineBreaker is probably ok, it's almost all about inline direciton, but NGInlineLayoutAlgorithm needs to deal with several types of different height that "block-size" doesn't work. "line-height" isn't exactly the same as "block-size", and it has "text-height" too. When we have "line-height", using "line-width" looks more natural to me than using "inline_size". Also inline is strongly directed by baseline; we use "text-top" (different from block-start), "line-left" (different from inline-start), etc., terminologies we can't map to logical box terminologies. On the other hand, I understand "line-width" is probably the only case where we have corresponding box terminologies, and "inline_size" is easier for box developers. So while using "inline_size" in inline algorithms looks a mix of different types of terminologies to me, I can live with it, but probably not for other inline terminologies since they loose precise meanings. Is the mix ok for you? > - Considering adding a new NGBfcOffset type specifically for block layout. ahh...can't answer. I really have to understand how bfc works... > - Considering adding terminology for offset relative to the direct parent as "local_offset" instead of just "offset". My mild preference is "offset" being "offset_to_xxx". Is "offset_to_parent" too long? https://codereview.chromium.org/2930963002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.cc (right): https://codereview.chromium.org/2930963002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.cc:387: {border_and_padding_.inline_start, content_size_}); Minor, but I've been wondering each child algorithm to handle container border and padding maybe redundant. Is it hard for parent block layout to handle in a unified way? Or maybe offset the anonymous container box by border_and_padding? Does it require special code only for inline? Wanted to experiment but my knowledge on block layout code prevented me doing so... https://codereview.chromium.org/2930963002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/layout/ng/inline/ng_inline_node.cc (right): https://codereview.chromium.org/2930963002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/layout/ng/inline/ng_inline_node.cc:378: // NGInlineLayoutAlgorithm. I think NGLineBreaker is better fit, and I'm hoping to do so sometime. MinMax is all about inline size, and line breaker knows how to compute without doing all layout computations. Also if we introduce different line breakers, it may produce different results. https://codereview.chromium.org/2930963002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/layout/ng/inline/ng_line_breaker.cc (right): https://codereview.chromium.org/2930963002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/layout/ng/inline/ng_line_breaker.cc:130: // DO NOT SUBMIT: maybe kControl as well? On 2017/06/08 at 20:53:18, ikilpatrick wrote: > kojii - halp! :D Hmm...my vague understanding of what "resolve the BFC offset" means makes me hard to answer, but your comments reminds me that I dealt with a magical criteria to determine whether a line has a height or not. It's not defined anywhere, but all browsers give a height to a line box if there were non-zero inline size. See tests: http://jsbin.com/lotiwen/edit?html,output Does this look related? I'll try to learn your snippets in hangout today a bit more to discuss better. From the code, probably "resolve the BFC offset" means to position pending floats. But I wonder, we could position anyway, and if the line ended up with zero-height, the floats will be taken to the next line anyway, no? Sorry, you might be surprised how poorly I understand how these work, both in our code or in spec.
The CQ bit was checked by ikilpatrick@chromium.org to run a CQ dry run
On 2017/06/09 13:32:18, kojii wrote: > Thank you for working on this. I guess I need to study a bit more to discuss, > sorry about that. Replies from my current knowledge. > > Replies are inline, but for your comments: > > > - Should NGLineBreaker & NGInlineLayoutAlgorithm be switch to using > inline_size instead of width? > > NGLineBreaker is probably ok, it's almost all about inline direciton, but > NGInlineLayoutAlgorithm needs to deal with several types of different height > that "block-size" doesn't work. "line-height" isn't exactly the same as > "block-size", and it has "text-height" too. When we have "line-height", using > "line-width" looks more natural to me than using "inline_size". > > Also inline is strongly directed by baseline; we use "text-top" (different from > block-start), "line-left" (different from inline-start), etc., terminologies we > can't map to logical box terminologies. > > On the other hand, I understand "line-width" is probably the only case where we > have corresponding box terminologies, and "inline_size" is easier for box > developers. So while using "inline_size" in inline algorithms looks a mix of > different types of terminologies to me, I can live with it, but probably not for > other inline terminologies since they loose precise meanings. > > Is the mix ok for you? > Yeah that sounds ok. > > - Considering adding a new NGBfcOffset type specifically for block layout. > > ahh...can't answer. I really have to understand how bfc works... Np. That was a mental note to myself. :) > > - Considering adding terminology for offset relative to the direct parent as > "local_offset" instead of just "offset". > > My mild preference is "offset" being "offset_to_xxx". Is "offset_to_parent" too > long? I actually like that, I'll send some cleanup patches that use that terminology if other folks are ok with it. I basically want to ban using "offset" as its vauge to want it is relative towards. > https://codereview.chromium.org/2930963002/diff/1/third_party/WebKit/Source/c... > File > third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.cc > (right): > > https://codereview.chromium.org/2930963002/diff/1/third_party/WebKit/Source/c... > third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.cc:387: > {border_and_padding_.inline_start, content_size_}); > Minor, but I've been wondering each child algorithm to handle container border > and padding maybe redundant. Is it hard for parent block layout to handle in a > unified way? > > Or maybe offset the anonymous container box by border_and_padding? Does it > require special code only for inline? Wanted to experiment but my knowledge on > block layout code prevented me doing so... > > https://codereview.chromium.org/2930963002/diff/1/third_party/WebKit/Source/c... > File third_party/WebKit/Source/core/layout/ng/inline/ng_inline_node.cc (right): > > https://codereview.chromium.org/2930963002/diff/1/third_party/WebKit/Source/c... > third_party/WebKit/Source/core/layout/ng/inline/ng_inline_node.cc:378: // > NGInlineLayoutAlgorithm. > I think NGLineBreaker is better fit, and I'm hoping to do so sometime. MinMax is > all about inline size, and line breaker knows how to compute without doing all > layout computations. Also if we introduce different line breakers, it may > produce different results. > > https://codereview.chromium.org/2930963002/diff/1/third_party/WebKit/Source/c... > File third_party/WebKit/Source/core/layout/ng/inline/ng_line_breaker.cc (right): > > https://codereview.chromium.org/2930963002/diff/1/third_party/WebKit/Source/c... > third_party/WebKit/Source/core/layout/ng/inline/ng_line_breaker.cc:130: // DO > NOT SUBMIT: maybe kControl as well? > On 2017/06/08 at 20:53:18, ikilpatrick wrote: > > kojii - halp! :D > > Hmm...my vague understanding of what "resolve the BFC offset" means makes me > hard to answer, but your comments reminds me that I dealt with a magical > criteria to determine whether a line has a height or not. It's not defined > anywhere, but all browsers give a height to a line box if there were non-zero > inline size. See tests: > http://jsbin.com/lotiwen/edit?html,output > Does this look related? > > I'll try to learn your snippets in hangout today a bit more to discuss better. > From the code, probably "resolve the BFC offset" means to position pending > floats. But I wonder, we could position anyway, and if the line ended up with > zero-height, the floats will be taken to the next line anyway, no? > > Sorry, you might be surprised how poorly I understand how these work, both in > our code or in spec.
https://codereview.chromium.org/2930963002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.cc (right): https://codereview.chromium.org/2930963002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.cc:387: {border_and_padding_.inline_start, content_size_}); On 2017/06/09 13:32:18, kojii wrote: > Minor, but I've been wondering each child algorithm to handle container border > and padding maybe redundant. Is it hard for parent block layout to handle in a > unified way? > > Or maybe offset the anonymous container box by border_and_padding? Does it > require special code only for inline? Wanted to experiment but my knowledge on > block layout code prevented me doing so... That might work, I can add a TODO to investigate this for the anon wrapper? My only concern is that we'd need to take this into account for the when resolving the bfc offset, but that might be ok. https://codereview.chromium.org/2930963002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/layout/ng/inline/ng_inline_node.cc (right): https://codereview.chromium.org/2930963002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/layout/ng/inline/ng_inline_node.cc:378: // NGInlineLayoutAlgorithm. On 2017/06/09 13:32:18, kojii wrote: > I think NGLineBreaker is better fit, and I'm hoping to do so sometime. MinMax is > all about inline size, and line breaker knows how to compute without doing all > layout computations. Also if we introduce different line breakers, it may > produce different results. Ok, my only concern is that it'd keep the same structure as the block layout algorithm. E.g. it'd be nice if the NGLineNode didn't know anything about the line breaker. The algorithm, can use the line breaker directly - but this is hidden from the NGInlineNode. https://codereview.chromium.org/2930963002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/layout/ng/inline/ng_line_breaker.cc (right): https://codereview.chromium.org/2930963002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/layout/ng/inline/ng_line_breaker.cc:130: // DO NOT SUBMIT: maybe kControl as well? On 2017/06/09 13:32:18, kojii wrote: > On 2017/06/08 at 20:53:18, ikilpatrick wrote: > > kojii - halp! :D > > Hmm...my vague understanding of what "resolve the BFC offset" means makes me > hard to answer, but your comments reminds me that I dealt with a magical > criteria to determine whether a line has a height or not. It's not defined > anywhere, but all browsers give a height to a line box if there were non-zero > inline size. See tests: > http://jsbin.com/lotiwen/edit?html,output > Does this look related? > > I'll try to learn your snippets in hangout today a bit more to discuss better. > From the code, probably "resolve the BFC offset" means to position pending > floats. But I wonder, we could position anyway, and if the line ended up with > zero-height, the floats will be taken to the next line anyway, no? > > Sorry, you might be surprised how poorly I understand how these work, both in > our code or in spec. The key is this test here: https://www.software.hixie.ch/utilities/js/live-dom-viewer/?%3C!DOCTYPE%20htm... If you change 
 to 	 or <br> they all have the same behaviour. This behaviour is that they margins do *not* collapse through the kControl object. I.e. if you remove the "
" the height of the box is: max(margin#zero1, margin#zero2) + height(END); if you have the "
" (or other control) character the height is: margin#zero1 + height(control)=0 + margin#zero2 + height(END). Added kControl to the list. Basically here the control character forces us to "resolve" the container within the block formatting context.
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: Try jobs failed on following builders: linux_layout_tests_layout_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_layout_te...)
https://codereview.chromium.org/2930963002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/layout/ng/inline/ng_inline_node.cc (right): https://codereview.chromium.org/2930963002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/layout/ng/inline/ng_inline_node.cc:378: // NGInlineLayoutAlgorithm. On 2017/06/09 at 17:04:07, ikilpatrick wrote: > Ok, my only concern is that it'd keep the same structure as the block layout algorithm. E.g. it'd be nice if the NGLineNode didn't know anything about the line breaker. The algorithm, can use the line breaker directly - but this is hidden from the NGInlineNode. I'm good to always go through algorithm. I found out that inline direction measuring/positioning and block direction measuring/positioning is so much different in inline layout that these days. And since they're quite independent to each other that NGLineBreaker to own inline direction helps separating concerns. Atm, I think the separation is good, so the core logic will be in NGLineBreaker, but entry can be in algorithm. I hope you're good with the separation? https://codereview.chromium.org/2930963002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/layout/ng/inline/ng_line_breaker.cc (right): https://codereview.chromium.org/2930963002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/layout/ng/inline/ng_line_breaker.cc:130: // DO NOT SUBMIT: maybe kControl as well? This test helps. Modified a bit: https://www.software.hixie.ch/utilities/js/live-dom-viewer/?%3C!DOCTYPE%20htm... It looks like margin collapsing uses the same criteria; in other words, it's a "line box with height" that prevents margin collapsing. We already have some logic to determine inline box creation in NGInlineLayoutAlgorithm::PlaceItems and NGInlineBoxState. As I commented in the doc CL, I'll see if we can separate floats to line breaker and PlaceItems.
https://codereview.chromium.org/2930963002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/layout/ng/inline/ng_inline_node.cc (right): https://codereview.chromium.org/2930963002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/layout/ng/inline/ng_inline_node.cc:378: // NGInlineLayoutAlgorithm. On 2017/06/10 05:47:53, kojii wrote: > On 2017/06/09 at 17:04:07, ikilpatrick wrote: > > Ok, my only concern is that it'd keep the same structure as the block layout > algorithm. E.g. it'd be nice if the NGLineNode didn't know anything about the > line breaker. The algorithm, can use the line breaker directly - but this is > hidden from the NGInlineNode. > > I'm good to always go through algorithm. I found out that inline direction > measuring/positioning and block direction measuring/positioning is so much > different in inline layout that these days. And since they're quite independent > to each other that NGLineBreaker to own inline direction helps separating > concerns. > > Atm, I think the separation is good, so the core logic will be in NGLineBreaker, > but entry can be in algorithm. I hope you're good with the separation? Yup that sounds good. https://codereview.chromium.org/2930963002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/layout/ng/inline/ng_line_breaker.cc (right): https://codereview.chromium.org/2930963002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/layout/ng/inline/ng_line_breaker.cc:130: // DO NOT SUBMIT: maybe kControl as well? On 2017/06/10 05:47:53, kojii wrote: > This test helps. Modified a bit: > https://www.software.hixie.ch/utilities/js/live-dom-viewer/?%3C!DOCTYPE%20htm... > > It looks like margin collapsing uses the same criteria; in other words, it's a > "line box with height" that prevents margin collapsing. > > We already have some logic to determine inline box creation in > NGInlineLayoutAlgorithm::PlaceItems and NGInlineBoxState. As I commented in the > doc CL, I'll see if we can separate floats to line breaker and PlaceItems. Yeah so the problem is that we need to resolve the BFC offset before we are able to position floats and recalculate the available width. E.g. Without the BFC offset we can't determine where floats we encounter on the line will go as clearance, top-alignment, etc may effect where they are positioned with the BFC. I commented on your patch with a couple of simple examples, but this is why I moved the resolving and line float handling in here. The problem with the: if (!Items().IsEmpty()) { /* resolve bfc offset */ } ... check inside the layout algorithm is that if you have the following: <span><span><float></float></span></span> Here the number of items == 5, but we aren't allowed to resolve the BFC offset. We can't do it after before we need the BFC offset to determine the available width :).
On 2017/06/10 at 18:00:25, ikilpatrick wrote: > We can't do it after before we need the BFC offset to determine the available width :). I'm not sure if you agree with my view in the other CL, but regardless this is really required to do here or this is an optimization, I'm fine to try to determine the linebox creation in NGLineBreaker. Optimization is good to have ;) As commented before, I haven't understood all the criteria yet, but doing more testing: http://output.jsbin.com/lotiwen I can't find any spec saying this but impls are surprisingly interoperable! I'll send you a CL to try to determine this in NGLineBreaker, so that you can use the function.
The CQ bit was checked by ikilpatrick@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...
ptal, move resolving bfc offset into function.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_layout_tests_layout_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does not exist (misspelled?))
lgtm, thank you for going through detailed explanation, that really helped me to understand better.
Some of my CLs may have conflicted, sorry if so. I realized I should have landed after this, after they landed.
Some of my CLs may have conflicted, sorry if so. I realized I should have landed after this, after they landed.
The CQ bit was checked by ikilpatrick@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: Try jobs failed on following builders: linux_layout_tests_layout_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_layout_te...)
Description was changed from ========== [LayoutNG] Refactor of NGLineBreaker. This patch does a few things because I got slightly carried away: - Removes the fact that NGLineBreaker takens an NGInlineAlgorithm as an argument. - Has better logic around resolving the BfcOffset of the container and positioning floats. We were previously doing this incorrectly. Specifically resolving too early if we just had floats. - Removes the available_width_ class member and makes it a stack allocated WTF::Optional for clarity now. - Tries to simplify a few other things. BUG=635619 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_layout_ng ========== to ========== [LayoutNG] Refactor of NGLineBreaker. This patch does a few things because I got slightly carried away: - Removes the fact that NGLineBreaker takens an NGInlineAlgorithm as an argument. - Has better logic around resolving the BfcOffset of the container and positioning floats. We were previously doing this incorrectly. Specifically resolving too early if we just had floats. - Removes the available_width_ class member and makes it a stack allocated WTF::Optional for clarity now. - Tries to simplify a few other things. BUG=635619 ==========
The CQ bit was checked by ikilpatrick@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from kojii@chromium.org Link to the patchset: https://codereview.chromium.org/2930963002/#ps60001 (title: "rebase")
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": 60001, "attempt_start_ts": 1497364932108790,
"parent_rev": "8cc019c673743e9a1a48c8a62145b1456a31e7c1", "commit_rev":
"f723897c7994e4ceee81dba57a6a3b94bc974a05"}
Message was sent while issue was closed.
Description was changed from ========== [LayoutNG] Refactor of NGLineBreaker. This patch does a few things because I got slightly carried away: - Removes the fact that NGLineBreaker takens an NGInlineAlgorithm as an argument. - Has better logic around resolving the BfcOffset of the container and positioning floats. We were previously doing this incorrectly. Specifically resolving too early if we just had floats. - Removes the available_width_ class member and makes it a stack allocated WTF::Optional for clarity now. - Tries to simplify a few other things. BUG=635619 ========== to ========== [LayoutNG] Refactor of NGLineBreaker. This patch does a few things because I got slightly carried away: - Removes the fact that NGLineBreaker takens an NGInlineAlgorithm as an argument. - Has better logic around resolving the BfcOffset of the container and positioning floats. We were previously doing this incorrectly. Specifically resolving too early if we just had floats. - Removes the available_width_ class member and makes it a stack allocated WTF::Optional for clarity now. - Tries to simplify a few other things. BUG=635619 Review-Url: https://codereview.chromium.org/2930963002 Cr-Commit-Position: refs/heads/master@{#479016} Committed: https://chromium.googlesource.com/chromium/src/+/f723897c7994e4ceee81dba57a6a... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://chromium.googlesource.com/chromium/src/+/f723897c7994e4ceee81dba57a6a... |
