Algorithm for deciding if a frame's selection should be hidden

third_party/WebKit/Source/core/editing/FrameSelection.cpp

Index: third_party/WebKit/Source/core/editing/FrameSelection.cpp
diff --git a/third_party/WebKit/Source/core/editing/FrameSelection.cpp b/third_party/WebKit/Source/core/editing/FrameSelection.cpp
index 3ee3ee041c1788952e17959615677739200b0219..cf1aed35a230d735765b27a2b3bda6a1a51ce51e 100644
--- a/third_party/WebKit/Source/core/editing/FrameSelection.cpp
+++ b/third_party/WebKit/Source/core/editing/FrameSelection.cpp
@@ -250,7 +250,7 @@ void FrameSelection::DidSetSelectionDeprecated(SetSelectionOptions options,
   }
 
   frame_caret_->StopCaretBlinkTimer();
-  UpdateAppearance(LayoutSelection::PaintHint::kPaint);
+  UpdateAppearance();
 
   // Always clear the x position used for vertical arrow navigation.
   // It will be restored by the vertical arrow navigation code if necessary.
@@ -335,26 +335,7 @@ void FrameSelection::DidChangeFocus() {
   // Hits in
   // virtual/gpu/compositedscrolling/scrollbars/scrollbar-miss-mousemove-disabled.html
   DisableCompositingQueryAsserts disabler;
-
-  // No focused element means document root has focus.
-  const Element* const focus = GetDocument().FocusedElement()
-                                   ? GetDocument().FocusedElement()
-                                   : GetDocument().documentElement();
-
-  // Protection against LayoutTests/editing/selection/selection-crash.html
-  if (!focus) {
-    frame_caret_->ScheduleVisualUpdateForPaintInvalidationIfNeeded();
-    text_control_focused_ = false;
-    return;
-  }
-
-  // Hide the selection when focus goes away from a text-field and into
-  // something that is not a text-field. When focus enters another text-field we
-  // do not need to update appearance; the appearance is updated when the new
-  // selection is set.
-  if (text_control_focused_ && !focus->IsTextControl())
-    UpdateAppearance(LayoutSelection::PaintHint::kHide);
-  text_control_focused_ = focus->IsTextControl();
+  UpdateAppearance();
 }
 
 static DispatchEventResult DispatchSelectStart(
@@ -444,6 +425,60 @@ void FrameSelection::Clear() {
   SetSelection(SelectionInDOMTree());
 }
 
+bool FrameSelection::SelectionHasFocus() const {
+  const Node* current = ComputeVisibleSelectionInDOMTreeDeprecated()

[yosin_UTC9, 2017/05/09 01:29:37]

I prefer to use |ComputeVisibleSelectionInDOMTree()| even if we have bug of
tracking this.
I don't want to see increasing of deprecated functions into new code.

If there are lots of call sites to make layout clean before calling this
functions,
we can update layout in this function with

  // TODO(editing-dev): Hoist updateStyleAndLayoutIgnorePendingStylesheets
  // to caller. See http://crbug.com/590369 for more details.
  GetDocument().UpdateStyleAndLayoutIgnorePendingStylesheets();

[hugoh_UTC2, 2017/05/09 08:11:23]

Are you OK with hitting a DCHECK during unit testing?
See
https://codereview.chromium.org/2841093002/diff/220001/third_party/WebKit/Sou...
.
I mean, if it happens during unit testing, it may also happen during real
usage...

I guess the safest is to add
GetDocument().UpdateStyleAndLayoutIgnorePendingStylesheets(); here then?

[hugoh_UTC2, 2017/05/09 09:22:45]

Done in PS12.

+                            .Start()
+                            .ComputeContainerNode();
+  if (!current)
+    return false;
+
+  // No focused element means document root has focus.
+  const Element* const focused_element = GetDocument().FocusedElement()
+                                             ? GetDocument().FocusedElement()
+                                             : GetDocument().documentElement();
+  if (!focused_element)
+    return false;
+
+  if (focused_element->IsTextControl())
+    return focused_element->ContainsIncludingHostElements(*current);
+
+  bool has_editable_style = HasEditableStyle(*current);
+  do {
+    // If the selection is within an editable sub tree and that sub tree
+    // doesn't have focus, the selection doesn't have focus either.
+    if (has_editable_style && !HasEditableStyle(*current))
+      return false;
+
+    // Selection has focus if its sub tree has focus.
+    if (current == focused_element)
+      return true;
+    current = current->ParentOrShadowHostNode();
+  } while (current);
+
+  return false;
+}
+
+bool FrameSelection::IsHidden() const {
+  if (SelectionHasFocus())
+    return false;
+
+  const Node* start = ComputeVisibleSelectionInDOMTreeDeprecated()

[yosin_UTC9, 2017/05/09 01:29:37]

We can use |ComputeVisibleSelectionInDOMTree()| since |SelectionHasFocus()|
updates layout.

[hugoh_UTC2, 2017/05/09 08:11:23]

Ah yes!

[hugoh_UTC2, 2017/05/09 09:22:45]

Done in PS12.

+                          .Start()
+                          .ComputeContainerNode();
+  if (!start)
+    return true;
+
+  // The selection doesn't have focus, so hide everything but range selections.
+  if (!GetSelectionInDOMTree().IsRange())
+    return true;
+
+  // Here we know we have an unfocused range selection. Let's say that
+  // selection resides inside a text control. Since the selection doesn't have
+  // focus neither does the text control. Meaning, if the selection indeed
+  // resides inside a text control, it should be hidden.
+  return EnclosingTextControl(start);
+}
+
 void FrameSelection::DocumentAttached(Document* document) {
   DCHECK(document);
   use_secure_keyboard_entry_when_active_ = false;
@@ -796,15 +831,13 @@ void FrameSelection::CommitAppearanceIfNeeded() {
 }
 
 void FrameSelection::DidLayout() {
-  // Upon relayout, a hidden selection must be kept hidden and a visible
-  // selection must be kept visible.
-  UpdateAppearance(LayoutSelection::PaintHint::kKeep);
+  UpdateAppearance();
 }
 
-void FrameSelection::UpdateAppearance(LayoutSelection::PaintHint hint) {
+void FrameSelection::UpdateAppearance() {
   DCHECK(!frame_->ContentLayoutItem().IsNull());
   frame_caret_->ScheduleVisualUpdateForPaintInvalidationIfNeeded();
-  layout_selection_->SetHasPendingSelection(hint);
+  layout_selection_->SetHasPendingSelection();
 }
 
 void FrameSelection::NotifyLayoutObjectOfSelectionChange(
@@ -993,7 +1026,7 @@ void FrameSelection::RevealSelection(const ScrollAlignment& alignment,
       document_loader->GetInitialScrollState().was_scrolled_by_user = true;
     if (start.AnchorNode()->GetLayoutObject()->ScrollRectToVisible(
             rect, alignment, alignment))
-      UpdateAppearance(LayoutSelection::PaintHint::kPaint);
+      UpdateAppearance();
   }
 }