Hide editing commands from DragController

Hide editing commands from DragController

BUG=n/a
TEST=n/a; no behavior change

Committed: https://crrev.com/cc78b117c95380fc74c66c33192b923c310af622
Cr-Commit-Position: refs/heads/master@{#371720}

[Xiaocheng, 2016-01-27 02:02:42]

xiaochengh@chromium.org changed reviewers:
+ yosin@chromium.org

[Xiaocheng, 2016-01-27 02:02:42]

PTAL.

Yesterday we also talked about forcing editing/commands to be hidden by
modifying the DEP files. However, editing/commands/UndoStack.h is also used
outside editing/, so it can't be done now.

[yosin_UTC9, 2016-01-27 02:23:02]

yosin@chromium.org changed reviewers:
+ tkent@chromium.org

[yosin_UTC9, 2016-01-27 02:23:03]

lgtm w/ nits for TODO comments

Thanks for quick work!

+tkent@ for core/page changes.

How about introducing EditingOption class which hold:
 - smartReplacment
 - smartReplace
 - matchStyle
 - smartInsert
 - smartDelete
 - plainText

Note: Pick them up from Editor.h

as replacement bool flags?

https://codereview.chromium.org/1634603003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/editing/Editor.cpp (right):

https://codereview.chromium.org/1634603003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/Editor.cpp:529: void
Editor::replaceSelectionAfterDragging(PassRefPtrWillBeRawPtr<DocumentFragment>
fragment, bool smartReplace, bool plainText)
Please add TODO comment to replace |bool| parameters by |enum|.

https://codereview.chromium.org/1634603003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/Editor.cpp:540: void
Editor::moveSelectionAfterDragging(PassRefPtrWillBeRawPtr<DocumentFragment>
fragment, const Position& pos, bool smartInsert, bool smartDelete)
Please add TODO comment to replace |bool| parameters by |enum|.

https://codereview.chromium.org/1634603003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/editing/Editor.h (right):

https://codereview.chromium.org/1634603003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/Editor.h:225: void
replaceSelectionAfterDragging(PassRefPtrWillBeRawPtr<DocumentFragment>, bool
smartReplace, bool plainText);
Please add TODO comment to replace |bool| parameters by |enum|.

https://codereview.chromium.org/1634603003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/page/DragController.cpp (right):

https://codereview.chromium.org/1634603003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/page/DragController.cpp:518:
m_documentUnderMouse->frame()->editor().replaceSelectionAfterDragging(createFragmentFromText(EphemeralRange(range.get()),
text), false, true);
nit: please use const local variable to name parameters, e.g.

const bool smartInsert = false;
const bool smartDelete = true;

[Xiaocheng, 2016-01-27 03:16:03]

Revised accordingly. PTAL.

https://codereview.chromium.org/1634603003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/editing/Editor.cpp (right):

https://codereview.chromium.org/1634603003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/Editor.cpp:529: void
Editor::replaceSelectionAfterDragging(PassRefPtrWillBeRawPtr<DocumentFragment>
fragment, bool smartReplace, bool plainText)
On 2016/01/27 at 02:23:03, Yosi_UTC9 wrote:
> Please add TODO comment to replace |bool| parameters by |enum|.

Added in Editor.h.

https://codereview.chromium.org/1634603003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/editing/Editor.h (right):

https://codereview.chromium.org/1634603003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/Editor.h:225: void
replaceSelectionAfterDragging(PassRefPtrWillBeRawPtr<DocumentFragment>, bool
smartReplace, bool plainText);
On 2016/01/27 at 02:23:03, Yosi_UTC9 wrote:
> Please add TODO comment to replace |bool| parameters by |enum|.

Done.

https://codereview.chromium.org/1634603003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/page/DragController.cpp (right):

https://codereview.chromium.org/1634603003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/page/DragController.cpp:518:
m_documentUnderMouse->frame()->editor().replaceSelectionAfterDragging(createFragmentFromText(EphemeralRange(range.get()),
text), false, true);
On 2016/01/27 at 02:23:03, Yosi_UTC9 wrote:
> nit: please use const local variable to name parameters, e.g.
> 
> const bool smartInsert = false;
> const bool smartDelete = true;

Done.

[tkent, 2016-01-27 03:17:25]

lgtm

[Xiaocheng, 2016-01-27 03:17:34]

The CQ bit was checked by xiaochengh@chromium.org

[Xiaocheng, 2016-01-27 03:17:34]

The patchset sent to the CQ was uploaded after l-g-t-m from yosin@chromium.org
Link to the patchset: https://codereview.chromium.org/1634603003/#ps20001
(title: " ")

[commit-bot: I haz the power, 2016-01-27 03:18:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1634603003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1634603003/20001

[yosin_UTC9, 2016-01-27 03:48:27]

On 2016/01/27 at 02:02:42, xiaochengh wrote:
> PTAL.
> 
> Yesterday we also talked about forcing editing/commands to be hidden by
modifying the DEP files. However, editing/commands/UndoStack.h is also used
outside editing/, so it can't be done now.

I see Page owns UndoStack and it is used in FrameLoader::dispatchUnloadEvent()
as follows:

void FrameLoader::dispatchUnloadEvent() {
 .... snip ...
 if (Page* page = m_frame->page())
   page->undoStack().didUnloadFrame(*m_frame);
}

It seems it is time to move UndoStack to per-frame instead of one global
UndoStack.
OOPIF, out-of-process iframe, team may or may not do this.

[commit-bot: I haz the power, 2016-01-27 04:22:03]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-01-27 04:23:36]

Description was changed from

==========
Hide editing commands from DragController

BUG=n/a
TEST=n/a; no behavior change
==========

to

==========
Hide editing commands from DragController

BUG=n/a
TEST=n/a; no behavior change

Committed: https://crrev.com/cc78b117c95380fc74c66c33192b923c310af622
Cr-Commit-Position: refs/heads/master@{#371720}
==========

[commit-bot: I haz the power, 2016-01-27 04:23:36]

Patchset 2 (id:??) landed as
https://crrev.com/cc78b117c95380fc74c66c33192b923c310af622
Cr-Commit-Position: refs/heads/master@{#371720}