[HiC] Store IsInCanvasSubtree rather than IsCanvasOrInCanvasSubtree [chromium/src : main]
Stephen Chenney (Gerrit)
Stephen Chenney added 1 comment![]( "Open in Gerrit")
I believe this improves efficiency of the canvas and canvas subtree checks.
Related details
- Philip Rogers
- Stefan Zager
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Philip Rogers (Gerrit)
Philip Rogers added 6 comments![]( "Open in Gerrit")
// Like `IsCanvasOrInCanvasSubtree()`, but includes <canvas> elements.Typo: IsCanvasOrInCanvasSubtree -> IsInCanvasSubtree.
bool IsParentOrOwnerCanvasOrInCanvasSubtree() const;WDYT of renaming this "ComputeIsInCanvasSubtree"?
if (IsA<HTMLCanvasElement>(element)) {Can you move the `DCHECK(element.IsCanvasOrInCanvasSubtree() == value);` check from line 4307 to line 4301? Now that `IsCanvasOrInCanvasSubtree` returns true for HTMLCanvasElements, setting the value to true on line 4305 means that the DCHECK is always true. Moving the DCHECK up above this lets VerifySubtreeIsInCanvas verify the flag on canvas elements.
if (!value && IsA<HTMLCanvasElement>(*this)) {WDYT of:
```
if (value_changed && !value && IsA<HTMLCanvasElement>(*this)) {
// When a canvas element transitions from inside a canvas subtree to
// outside, reset value to true for child propagation.
value = true;
}
```
// in the FlatTreeTraversal::ParentElement call inside SetIsInCanvasSubtree.Should "SetIsInCanvasSubtree" be "IsParentOrOwnerCanvasOrInCanvasSubtree"?
Related details
- Stefan Zager
- Stephen Chenney
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Stephen Chenney (Gerrit)
Stephen Chenney voted and added 6 comments![]( "Open in Gerrit")
Votes added by Stephen Chenney
| Commit-Queue | +1 |
6 comments
// Like `IsCanvasOrInCanvasSubtree()`, but includes <canvas> elements.Stephen ChenneyTypo: IsCanvasOrInCanvasSubtree -> IsInCanvasSubtree.
Done
WDYT of renaming this "ComputeIsInCanvasSubtree"?
Actually what I had first, so I'[ll go back to that.
Can you move the `DCHECK(element.IsCanvasOrInCanvasSubtree() == value);` check from line 4307 to line 4301? Now that `IsCanvasOrInCanvasSubtree` returns true for HTMLCanvasElements, setting the value to true on line 4305 means that the DCHECK is always true. Moving the DCHECK up above this lets VerifySubtreeIsInCanvas verify the flag on canvas elements.
Yes, good point, but slightly more complex. Done.
WDYT of:
```
if (value_changed && !value && IsA<HTMLCanvasElement>(*this)) {
// When a canvas element transitions from inside a canvas subtree to
// outside, reset value to true for child propagation.
value = true;
}
```
Yes, in a better way with no need for value_changed.
// in the FlatTreeTraversal::ParentElement call inside SetIsInCanvasSubtree.Should "SetIsInCanvasSubtree" be "IsParentOrOwnerCanvasOrInCanvasSubtree"?
- Philip Rogers
- Stefan Zager
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Stephen Chenney (Gerrit)
Stephen Chenney voted and added 1 comment![]( "Open in Gerrit")
Votes added by Stephen Chenney
| Auto-Submit | +1 |
| Commit-Queue | +1 |
1 comment
Related details
- Philip Rogers
- Stefan Zager
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |