From 4226ddf99103e493d7afb23a4c7902ee496108b6 Mon Sep 17 00:00:00 2001
From: Manuel Rego Casasnovas <rego@igalia.com>
Date: Fri, 12 Oct 2018 00:23:12 +0000
Subject: [PATCH] Fix perf regression in
 LayoutBoxModelObject::RelativePositionOffset()

In r597543 we introduced a performance regression
due to the changes in LayoutBoxModelObject::RelativePositionOffset().
One of the main differences is that AvailableHeight|Width()
was called always as part of the method,
while that was not the case before.

This patches moves the calls to AvailableHeight|Width()
to the moment where they are needed, trying to minimize
the impact in performance and come back to previous numbers.

No new tests as it's already covered by existent ones.

BUG=893884,835607

Change-Id: Id8aaba4736a821af9f401492206840c12a2be0e6
Reviewed-on: https://chromium-review.googlesource.com/c/1273117
Commit-Queue: Manuel Rego <rego@igalia.com>
Reviewed-by: Javier Fernandez <jfernandez@igalia.com>
Cr-Commit-Position: refs/heads/master@{#599034}
---
 .../core/layout/layout_box_model_object.cc    | 41 +++++++++++--------
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/third_party/blink/renderer/core/layout/layout_box_model_object.cc b/third_party/blink/renderer/core/layout/layout_box_model_object.cc
index 4b36b3bf0222..1128e3f3cf1c 100644
--- a/third_party/blink/renderer/core/layout/layout_box_model_object.cc
+++ b/third_party/blink/renderer/core/layout/layout_box_model_object.cc
@@ -732,19 +732,15 @@ LayoutSize LayoutBoxModelObject::RelativePositionOffset() const {
   // https://drafts.csswg.org/css-grid/#grid-item-sizing
   base::Optional<LayoutUnit> left;
   base::Optional<LayoutUnit> right;
-  LayoutUnit available_width = containing_block->AvailableWidth();
-  LayoutUnit available_height = containing_block->AvailableHeight();
-  bool has_override_containing_block_content = false;
-  if (HasOverrideContainingBlockContentWidth()) {
-    DCHECK(HasOverrideContainingBlockContentHeight());
-    has_override_containing_block_content = true;
-    available_width = OverrideContainingBlockContentWidth();
-    available_height = OverrideContainingBlockContentHeight();
+  if (!StyleRef().Left().IsAuto() || !StyleRef().Right().IsAuto()) {
+    LayoutUnit available_width = HasOverrideContainingBlockContentWidth()
+                                     ? OverrideContainingBlockContentWidth()
+                                     : containing_block->AvailableWidth();
+    if (!StyleRef().Left().IsAuto())
+      left = ValueForLength(StyleRef().Left(), available_width);
+    if (!StyleRef().Right().IsAuto())
+      right = ValueForLength(StyleRef().Right(), available_width);
   }
-  if (!StyleRef().Left().IsAuto())
-    left = ValueForLength(StyleRef().Left(), available_width);
-  if (!StyleRef().Right().IsAuto())
-    right = ValueForLength(StyleRef().Right(), available_width);
   if (!left && !right) {
     left = LayoutUnit();
     right = LayoutUnit();
@@ -784,19 +780,32 @@ LayoutSize LayoutBoxModelObject::RelativePositionOffset() const {
 
   base::Optional<LayoutUnit> top;
   base::Optional<LayoutUnit> bottom;
+  bool has_override_containing_block_content_height =
+      HasOverrideContainingBlockContentHeight();
   if (!StyleRef().Top().IsAuto() &&
       (!containing_block->HasAutoHeightOrContainingBlockWithAutoHeight() ||
        !StyleRef().Top().IsPercentOrCalc() ||
        containing_block->StretchesToViewport() ||
-       has_override_containing_block_content)) {
-    top = ValueForLength(StyleRef().Top(), available_height);
+       has_override_containing_block_content_height)) {
+    // TODO(rego): The computation of the available height is repeated later for
+    // "bottom". We could refactor this and move it to some common code for both
+    // ifs, however moving it outside of the ifs is not possible as it'd cause
+    // performance regressions (see crbug.com/893884).
+    top = ValueForLength(StyleRef().Top(),
+                         has_override_containing_block_content_height
+                             ? OverrideContainingBlockContentHeight()
+                             : containing_block->AvailableHeight());
   }
   if (!StyleRef().Bottom().IsAuto() &&
       (!containing_block->HasAutoHeightOrContainingBlockWithAutoHeight() ||
        !StyleRef().Bottom().IsPercentOrCalc() ||
        containing_block->StretchesToViewport() ||
-       has_override_containing_block_content)) {
-    bottom = ValueForLength(StyleRef().Bottom(), available_height);
+       has_override_containing_block_content_height)) {
+    // TODO(rego): Check comment above for "top", it applies here too.
+    bottom = ValueForLength(StyleRef().Bottom(),
+                            has_override_containing_block_content_height
+                                ? OverrideContainingBlockContentHeight()
+                                : containing_block->AvailableHeight());
   }
   if (!top && !bottom) {
     top = LayoutUnit();