Partly revert https://codereview.chromium.org/1860273003/ where paint follows

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

Index: third_party/WebKit/Source/core/frame/LocalFrame.cpp
diff --git a/third_party/WebKit/Source/core/frame/LocalFrame.cpp b/third_party/WebKit/Source/core/frame/LocalFrame.cpp
index 80e692a96d2c29a743fcd76d8e388a8890f506ce..e623431220d76a02f98b328256f74b9d0e71bba9 100644
--- a/third_party/WebKit/Source/core/frame/LocalFrame.cpp
+++ b/third_party/WebKit/Source/core/frame/LocalFrame.cpp
@@ -619,7 +619,8 @@ double LocalFrame::devicePixelRatio() const
 
 PassOwnPtr<DragImage> LocalFrame::nodeImage(Node& node)
 {
-    m_view->updateLifecycleToCompositingCleanPlusScrolling();
+    // TODO(wangxianzhu): Don't need synchronized painting.

[pdr., 2016/04/13 22:11:02]

Nit: Don't -> Doesn't.

I'm learning about this code for the first time so forgive some silly questions.

Is there any way we can enforce this comment with an assert instead of a
comment? For example, in the stacktrace in 602961 it looks like we are painting
but we've skipped the paint invalidation step. In FrameView::paintContents we
have an assert that the lifecycle >= DocumentLifecycle::CompositingClean...
should that assert lifecycle >= PaintInvalidationClean with a comment about why?

A second issue is that the root cause seems to be that we are updating collapsed
borders during paint invalidation. Would it be practical to just fix that issue
(maybe by moving recalcCollapsedBordersIfNeeded into
LayoutTable::collapsedBorders?). If not, WDYT about adding something like
LayoutView::updateLifecycleToPaintInvalidationClean() with comments about why
it's necessary?

How important was Chris's original patch? It seems to have raised several issues
(2 of the linked bugs are P1)... should we just fully roll it out? It's not
clear to me why some of the calls to
updateLifecycleToCompositingCleanPlusScrolling are still safe while the ones in
this patch aren't.

[Xianzhu, 2016/04/13 22:51:45]

Done.
Good idea to update/add the asserts which ensures the least lifecycle
requirement. The asserts won't replace this comment which reminds us the extra
thing (synchronized painting) than needed. Will update/add the asserts in a
separate CL because I'm afraid
there might be something unexpected.
We must call recalcCollapsedBordersIfNeeded() during or before paint
invalidation to detect changed cells and invalidate them.
LayoutTable::collapsedBorders() is called from paint code which is too late
(which was a paint invalidation during painting bug).

Because we'll also need paint property update before painting for spv2, and will
integrate paint invalidation into pre-painting tree walk, updating collapsed
borders during paint invalidaiton (to be pre-painting tree walk) seems a
non-issue.

Though the bug is about collapsed borders, it's not the only reason that we
require paint invalidation before painting. We also need paint invalidation to
update some status/flags on LayoutObjects and PaintLayers in order to do correct
painting.
Whether its safe depends what is needed to be ready by the following code. At
this place, the following code does painting, so the document lifecycle should
be ready for painting. They other places are safe because they only need
compositing update ready.

+    m_view->updateAllLifecyclePhases();
     LayoutObject* layoutObject = node.layoutObject();
     if (!layoutObject)
         return nullptr;
@@ -645,7 +646,8 @@ PassOwnPtr<DragImage> LocalFrame::dragImageForSelection(float opacity)
     if (!selection().isRange())
         return nullptr;
 
-    m_view->updateLifecycleToCompositingCleanPlusScrolling();
+    // TODO(wangxianzhu): Don't need synchronized painting.
+    m_view->updateAllLifecyclePhases();
     ASSERT(document()->isActive());
 
     FloatRect paintingRect = FloatRect(selection().bounds());