[webkit-reviews] review canceled: [Bug 190492] [css-grid] Fix percentages in relative offsets for grid items : [Attachment 352088] Patch

[bugzilla-daemon at webkit.org]

Manuel Rego Casasnovas <rego at igalia.com> has canceled Manuel Rego Casasnovas
<rego at igalia.com>'s request for review:
Bug 190492: [css-grid] Fix percentages in relative offsets for grid items
https://bugs.webkit.org/show_bug.cgi?id=190492

Attachment 352088: Patch

https://bugs.webkit.org/attachment.cgi?id=352088&action=review




--- Comment #4 from Manuel Rego Casasnovas <rego at igalia.com> ---
Comment on attachment 352088
  --> https://bugs.webkit.org/attachment.cgi?id=352088
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=352088&action=review

>>> Source/WebCore/rendering/RenderBoxModelObject.cpp:426
>>> +	 bool hasOverrideContainingBlockContent =
hasOverrideContainingBlockContentHeight();
>> 
>> You're unconditionally adding a hashmap query here. I think it's better to
have the call inside the if so we can shortcircuit it if any of the previous
conditions is already true.
>> 
>> On the other hand it might be the case that we need to call it twice in the
worst case...
> 
> We should get rid of the hashmap to manage the RenderBox override size, as we
do in Blink. 
> 
> About this specific case, I wouldn't complicate the code because of
performance, since we'll probably get rid of the hashmap in the near future.

We're getting some issues in Blink, so until we stablize the code there I don't
think it makes sense to merge this on WebKit.