Code should never branch on childNeedsDistributionRecalc |
|||||
Issue descriptionHacks were added to not hit asserts: ex. https://codereview.chromium.org/845453006 See https://bugs.chromium.org/p/chromium/issues/detail?id=450115 but we should never branch on this bit since it causes correctness bugs. It's like branching on childNeedsStyleRecalc. All things that check this bit that are not *inside* updateDistribution or setting the bits themselves are bugs.
,
Feb 6 2017
,
Feb 6 2017
,
Feb 6 2017
kojii@, could you take a look? I think you have a lot of contexts than me about this.
,
Feb 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/884c1d0ef960ac2bfcc0839b9767fad82f6ef668 commit 884c1d0ef960ac2bfcc0839b9767fad82f6ef668 Author: kojii <kojii@chromium.org> Date: Thu Feb 09 12:25:17 2017 Remove branch on childNeedsDistributionRecalc This patch moves a branch on childNeedsDistributionRecalc() in Text::textLayoutObjectIsNeeded() to shouldUpdateLayoutByReattaching() and adds comments so that it is clearer that the branch is used for the optimization purpose. BUG= 673967 Review-Url: https://codereview.chromium.org/2680463003 Cr-Commit-Position: refs/heads/master@{#449266} [add] https://crrev.com/884c1d0ef960ac2bfcc0839b9767fad82f6ef668/third_party/WebKit/LayoutTests/shadow-dom/crashes/character-data-set-data-crash.html [modify] https://crrev.com/884c1d0ef960ac2bfcc0839b9767fad82f6ef668/third_party/WebKit/Source/core/dom/Text.cpp
,
Feb 9 2017
The code still branch on childNeedsDistributionRecalc(), but the purpose is more limited only for the intended code path, and better clarified, with multiple reviewers. If this new usage doesn't still look good and should be eliminated, please leave comments and I'll investigate further. |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by dominicc@chromium.org
, Jan 19 2017Owner: hayato@chromium.org
Status: Assigned (was: Untriaged)