[css-grid] Fix simplified layout of positioned grid items

third_party/WebKit/Source/core/layout/LayoutBlock.h

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2007 David Smith (catfish.man@gmail.com)
  * Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010 Apple Inc.
  *               All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
@@ skipping 340 matching lines @@
   void dirtyForLayoutFromPercentageHeightDescendants(SubtreeLayoutScope&);
 
   void layout() override;
 
   enum PositionedLayoutBehavior {
     DefaultLayout,
     LayoutOnlyFixedPositionedObjects,
     ForcedLayoutAfterContainingBlockMoved
   };
 
-  void layoutPositionedObjects(bool relayoutChildren,
-                               PositionedLayoutBehavior = DefaultLayout);
+  virtual void layoutPositionedObjects(

[mstensho (USE GERRIT), 2016/11/04 08:49:12]

This shouldn't be necessary.

[Manuel Rego, 2016/11/04 09:29:17]

Mmmm, we need that LayoutGrid::layoutPositionedObjects() is called
from LayoutBlock::simplifiedLayout().
If we don't mark the function as virtual, this won't happen, only
LayoutBlock::layoutPositionedObjects() will be called
(and the positioned objects won't be properly placed inside the grid).

Am I missing something?

[mstensho (USE GERRIT), 2016/11/04 09:47:34]

Duh, no. *I* was missing something. Sorry. This needs to remain virtual.

+      bool relayoutChildren,
+      PositionedLayoutBehavior = DefaultLayout);
   void markFixedPositionObjectForLayoutIfNeeded(LayoutObject* child,
                                                 SubtreeLayoutScope&);
 
   LayoutUnit marginIntrinsicLogicalWidthForChild(const LayoutBox& child) const;
 
   int beforeMarginInLineDirection(LineDirectionMode) const;
 
   void paint(const PaintInfo&, const LayoutPoint&) const override;
 
  public:
@@ skipping 41 matching lines @@
   void styleDidChange(StyleDifference, const ComputedStyle* oldStyle) override;
   void updateFromStyle() override;
 
   // Returns true if non-visible overflow should be respected. Otherwise
   // hasOverflowClip() will be false and we won't create scrollable area for
   // this object even if overflow is non-visible.
   virtual bool allowsOverflowClip() const;
 
   virtual bool hasLineIfEmpty() const;
 
-  bool simplifiedLayout();
+  virtual bool simplifiedLayout();

[mstensho (USE GERRIT), 2016/11/04 08:49:12]

Ditto.

[Manuel Rego, 2016/11/04 09:29:17]

True for this case, actually as @cbiesinger suggested
I'm no longer overriding this function in the new version of the patch.

   virtual void simplifiedNormalFlowLayout();
 
  public:
   virtual void computeOverflow(LayoutUnit oldClientAfterEdge, bool = false);
 
  protected:
   virtual void addOverflowFromChildren();
   void addOverflowFromPositionedObjects();
   void addOverflowFromBlockChildren();
   void addVisualOverflowFromTheme();
@@ skipping 130 matching lines @@
   // TODO(lunalu): Temporary in order to ensure compatibility with existing
   // layout test results.
   virtual void adjustChildDebugRect(LayoutRect&) const {}
 };
 
 DEFINE_LAYOUT_OBJECT_TYPE_CASTS(LayoutBlock, isLayoutBlock());
 
 }  // namespace blink
 
 #endif  // LayoutBlock_h