Fix preferred logical widths of orthogonal writing modes

third_party/WebKit/Source/core/frame/FrameView.cpp

Index: third_party/WebKit/Source/core/frame/FrameView.cpp
diff --git a/third_party/WebKit/Source/core/frame/FrameView.cpp b/third_party/WebKit/Source/core/frame/FrameView.cpp
index de49f4bed8fe9b65c1d2548956bca675c16783dc..e3075a0daa8b21d7bfcb74c709982fe44b40cc45 100644
--- a/third_party/WebKit/Source/core/frame/FrameView.cpp
+++ b/third_party/WebKit/Source/core/frame/FrameView.cpp
@@ -834,6 +834,9 @@ void FrameView::performLayout(bool inSubtreeLayout)
 
     forceLayoutParentViewIfNeeded();
 
+    if (hasOrthogonalWritingModeRoots())
+        layoutOrthogonalWritingModeRoots();
+
     if (inSubtreeLayout) {
         if (m_analyzer)
             m_analyzer->increment(LayoutAnalyzer::PerformLayoutRootLayoutObjects, m_layoutSubtreeRootList.size());
@@ -1696,6 +1699,36 @@ void FrameView::clearLayoutSubtreeRootsAndMarkContainingBlocks()
     m_layoutSubtreeRootList.clearAndMarkContainingBlocksForLayout();
 }
 
+void FrameView::addOrthogonalWritingModeRoot(LayoutBox& root)
+{
+    m_orthogonalWritingModeRootList.addRoot(root);
+}
+
+void FrameView::removeOrthogonalWritingModeRoot(LayoutBox& root)
+{
+    m_orthogonalWritingModeRootList.removeRoot(root);
+}
+
+bool FrameView::hasOrthogonalWritingModeRoots() const
+{
+    return !m_orthogonalWritingModeRootList.isEmpty();
+}
+
+void FrameView::layoutOrthogonalWritingModeRoots()
+{
+    while (LayoutObject* root = m_orthogonalWritingModeRootList.takeDeepestRoot()) {
+        ASSERT(root->isBox() && toLayoutBox(*root).isOrthogonalWritingModeRoot());
+        if (!root->needsLayout()
+            || root->isOutOfFlowPositioned()
+            || root->isColumnSpanAll()
+            || !root->styleRef().logicalHeight().isIntrinsicOrAuto()) {

[ojan, 2016/01/22 02:22:35]

If it's any of these last three, maybe we just shouldn't put it in
m_orthogonalWritingModeRootList in the first place? 

[kojii, 2016/01/22 02:33:51]

Yeah, thought about it. But then we'll need to check if these properties are
changed and add to/remove from the list. To do so, we'll need to check
writing-mode whenever 'position' or 'height' property were changed.

Given orthogonal is much rarer than changing 'position' or 'height', having
longer list than really necessary looks better to doing such checks. Does this
look reasonable trade-off?

+            continue;
+        }
+        LayoutState layoutState(*root);

[ojan, 2016/01/22 02:22:35]

I'm not 100% sure creating a LayoutState without dealing with any of the
ancestors is correct. I guess that's the fundamental assumption of this approach
though. :) Taking a quick look at the LayoutState code, I don't see a problem,
but Levi and Morten should probably take a close look here.

[kojii, 2016/01/22 02:33:51]

Yeah, wasn't sure either, but reading around and testing some cases seems to
work better than I expected. Thank you for giving your thoughts. I'll ask Levi
more on this point.

+        root->layout();
+    }
+}
+
 void FrameView::scheduleRelayout()
 {
     ASSERT(m_frame->view() == this);