Select All should show handles if they were already present

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

 /*
  * Copyright (C) 2004, 2008, 2009, 2010 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 895 matching lines @@
   if (selectStartTarget) {
     const Document& expectedDocument = document();
     if (selectStartTarget->dispatchEvent(Event::createCancelableBubble(
             EventTypeNames::selectstart)) != DispatchEventResult::NotCanceled)
       return;
     // |root| may be detached due to selectstart event.
     if (!root->isConnected() || expectedDocument != root->document())
       return;
   }
 
-  setSelection(SelectionInDOMTree::Builder().selectAllChildren(*root).build());
+  SetSelectionOptions options = CloseTyping | ClearTypingStyle;

[yosin_UTC9, 2017/01/19 08:31:03]

I prefer

const SetSelectionOptions options = isHandleVisible()
  ? CloseTypeing | ClearTyping | HandleVisible
  : CloseTypeing | ClearTyping;

[amaralp, 2017/01/20 01:22:09]

This has been removed.

+  if (isHandleVisible())

[yosin_UTC9, 2017/01/19 08:31:02]

When do we not show handle if |isHandleVisbile()|?
Can we have a option |HandleVisibleIfHahdleVisible| or another better name?
Checking |isHandleVisible()| and turn on option is very error prone.

[yosin_UTC9, 2017/01/19 09:36:52]

[1] is another example of error prone regarding handle visibility.

[1] https://codereview.chromium.org/2370663002

[amaralp, 2017/01/20 01:22:09]

A few cases I can think of:
  1) Javascript selection
  2) Tapping to clear a handle
  3) Typing should clear the handle.
Yep, |PreserveHandle| has been added.
Not sure what you mean by this. Have my new changes addressed this?

I think this change would also make changwan@'s CL simpler.

+    options |= HandleVisible;
+
+  setSelection(SelectionInDOMTree::Builder().selectAllChildren(*root).build(),
+               options);
   selectFrameElementInParentIfFullySelected();
   notifyLayoutObjectOfSelectionChange(UserTriggered);
 }
 
 bool FrameSelection::setSelectedRange(const EphemeralRange& range,
                                       TextAffinity affinity,
                                       SelectionDirectionalMode directional,
                                       SetSelectionOptions options) {
   if (range.isNull())
     return false;
@@ skipping 494 matching lines @@
 }
 
 void showTree(const blink::FrameSelection* sel) {
   if (sel)
     sel->showTreeForThis();
   else
     LOG(INFO) << "Cannot showTree for <null> FrameSelection.";
 }
 
 #endif