Move layouet/LayoutView::setSelection() to editing/LayoutSelection

third_party/WebKit/Source/core/layout/LayoutView.cpp

Index: third_party/WebKit/Source/core/layout/LayoutView.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutView.cpp b/third_party/WebKit/Source/core/layout/LayoutView.cpp
index 3920df6e2ba2183fac103e02d5200c951d9d4680..942e2e5ea4205a121cd72bd6fb985c290b7c06ca 100644
--- a/third_party/WebKit/Source/core/layout/LayoutView.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutView.cpp
@@ -25,6 +25,7 @@
 #include "core/dom/Document.h"
 #include "core/dom/Element.h"
 #include "core/editing/FrameSelection.h"
+#include "core/editing/LayoutSelection.h"
 #include "core/frame/FrameView.h"
 #include "core/frame/LocalFrame.h"
 #include "core/frame/Settings.h"
@@ -87,10 +88,6 @@ class HitTestLatencyRecorder {
 LayoutView::LayoutView(Document* document)
     : LayoutBlockFlow(document),
       frame_view_(document->View()),
-      selection_start_(nullptr),
-      selection_end_(nullptr),
-      selection_start_pos_(-1),
-      selection_end_pos_(-1),
       layout_state_(nullptr),
       layout_quote_head_(nullptr),
       layout_counter_count_(0),
@@ -632,14 +629,15 @@ static LayoutRect SelectionRectForLayoutObject(const LayoutObject* object) {
   return object->SelectionRectInViewCoordinates();
 }
 
-IntRect LayoutView::SelectionBounds() {
+// TODO(yoichio): Move this to LayoutSelection.
+IntRect LayoutSelection::SelectionBounds() {
   // Now create a single bounding box rect that encloses the whole selection.
   LayoutRect sel_rect;
 
   typedef HashSet<const LayoutBlock*> VisitedContainingBlockSet;
   VisitedContainingBlockSet visited_containing_blocks;
 
-  CommitPendingSelection();
+  Commit(*frame_selection_->GetDocument().GetLayoutView());
   LayoutObject* os = selection_start_;
   LayoutObject* stop =
       LayoutObjectAfterPosition(selection_end_, selection_end_pos_);
@@ -667,7 +665,8 @@ IntRect LayoutView::SelectionBounds() {
   return PixelSnappedIntRect(sel_rect);
 }
 
-void LayoutView::InvalidatePaintForSelection() {
+// TODO(yoichio): Move this to LayoutSelection.
+void LayoutSelection::InvalidatePaintForSelection() {
   LayoutObject* end =
       LayoutObjectAfterPosition(selection_end_, selection_end_pos_);
   for (LayoutObject* o = selection_start_; o && o != end;
@@ -707,7 +706,8 @@ static inline LayoutObject* GetNextOrPrevLayoutObjectBasedOnDirection(
   return next;
 }
 
-void LayoutView::SetSelection(
+// TODO(yoichio): Move this to LayoutSelection.
+void LayoutSelection::SetSelection(
     LayoutObject* start,
     int start_pos,
     LayoutObject* end,
@@ -837,7 +837,7 @@ void LayoutView::SetSelection(
                                                   exploring_backwards);
   }
 
-  if (!frame_view_)

[dcheng, 2017/04/11 06:29:14]

I'm not actually sure if there is any time this will ever be true:
- LayoutView is created during Document::Initialize()
- Document::Initialize() is called by LocalDOMWindow::InstallNewDocument()
- LocalDOMWindow::InstallNewDocument() is called when we are committing a
provisional load:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/loader/Do...
- But before we install a new document, we ensure the FrameView is created in
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/loader/Fr...

So probably all these null checks are pointless.

+  if (!frame_selection_->GetDocument().GetLayoutView()->GetFrameView())

[dcheng, 2017/04/11 06:29:15]

I would probably try DCHECKing this. It looks like this is called during layout,
and I wouldn't expect to be updating layout on an inactive Document.

[yoichio, 2017/04/11 07:23:58]

I agree. I would DCHECK in another patch.

     return;
 
   // Have any of the old selected objects changed compared to the new selection?
@@ -882,7 +882,8 @@ void LayoutView::SetSelection(
     i->key->SetShouldInvalidateSelection();
 }
 
-void LayoutView::ClearSelection() {
+// TODO(yoichio): Move this to LayoutSelection.
+void LayoutSelection::ClearSelection() {
   // For querying Layer::compositingState()
   // This is correct, since destroying layout objects needs to cause eager paint
   // invalidations.
@@ -891,6 +892,10 @@ void LayoutView::ClearSelection() {
   SetSelection(0, -1, 0, -1, kPaintInvalidationNewMinusOld);
 }
 
+void LayoutView::ClearSelection() {
+  frame_view_->GetFrame().Selection().ClearLayoutSelection();
+}
+
 bool LayoutView::HasPendingSelection() const {
   return frame_view_->GetFrame().Selection().IsAppearanceDirty();
 }
@@ -901,7 +906,13 @@ void LayoutView::CommitPendingSelection() {
 }
 
 void LayoutView::SelectionStartEnd(int& start_pos, int& end_pos) {
-  CommitPendingSelection();
+  frame_view_->GetFrame().Selection().LayoutSelectionStartEnd(start_pos,
+                                                              end_pos);
+}
+
+// TODO(yoichio): Move this to LayoutSelection.
+void LayoutSelection::SelectionStartEnd(int& start_pos, int& end_pos) {
+  Commit(*frame_selection_->GetDocument().GetLayoutView());
   start_pos = selection_start_pos_;
   end_pos = selection_end_pos_;
 }