Move UndoStack from Page to Editor (issue 2110543008 by xiaochengh@chromium.org)

[xiaoc...@chromium.org]

Reviewers: Yosi_UTC9
CL: https://codereview.chromium.org/2110543008/

Message:
PTAL. Thanks.

I also have some questions about the usage of the undo stack after its frame is
unloaded. Please refer to the TODOs I added in the CL.

Description:
Move UndoStack from Page to Editor

Blink currently maintains per-page undo stacks, leading to:
- Security risks. A frame can directly manipulate content of another frame
by running document.execCommand('undo'), allowing Javascript to bypass
frame and even origin boundaries.
- Inconsistent behaviors. Without OOPIF, all changes in a page can be undone
by repeatedly invoking keyboard undo (CTRL+Z); With OOPIF, only those changes
in the focused frame and its same-origin frames can be undone.

Redos have analogous defects.

This patch changes UndoStack from per-page to per-frame, so that undos and
redos are consistently resolved by the frame where script is run or which
gets focused.

BUG=349272,549334

Base URL: https://chromium.googlesource.com/chromium/src.git@master

Affected files (+26, -52 lines):
M third_party/WebKit/LayoutTests/editing/undo/undo-iframe-location-change.html
M third_party/WebKit/Source/core/editing/Editor.h
M third_party/WebKit/Source/core/editing/Editor.cpp
M third_party/WebKit/Source/core/editing/commands/UndoStack.h
M third_party/WebKit/Source/core/editing/commands/UndoStack.cpp
M third_party/WebKit/Source/core/loader/FrameLoader.cpp
M third_party/WebKit/Source/core/page/Page.h
M third_party/WebKit/Source/core/page/Page.cpp


Index: third_party/WebKit/LayoutTests/editing/undo/undo-iframe-location-change.html
diff --git a/third_party/WebKit/LayoutTests/editing/undo/undo-iframe-location-change.html b/third_party/WebKit/LayoutTests/editing/undo/undo-iframe-location-change.html
index 6b7aa2c2a280a7ec6ddfa73c5aaf1b622fb18015..f2c9888eb69f13fbbb816fde0743128fa5c31491 100644
--- a/third_party/WebKit/LayoutTests/editing/undo/undo-iframe-location-change.html
+++ b/third_party/WebKit/LayoutTests/editing/undo/undo-iframe-location-change.html
@@ -12,7 +12,7 @@ function log(message) {
function part1() {
frames['iframe'].document.body.focus();

- // Hack to perform the editing command. Should be able to
+ // Hack to perform the editing command. Should not be able to
// call execCommand on the main document.
frames['iframe'].document.execCommand('InsertText', false, 'c');

@@ -23,8 +23,8 @@ function part1() {
return;
}

- if (!document.queryCommandEnabled('Undo')) {
- log("FAIL: Undo is not enabled after text insertion.");
+ if (document.queryCommandEnabled('Undo')) {
+ log("FAIL: Undo is enabled in main document after text insertion in subframe.");
if (window.testRunner)
testRunner.notifyDone();
return;
@@ -40,7 +40,7 @@ function part2() {
if (!document.queryCommandEnabled('Undo'))
log("Success");
else
- log("Failure, Undo was still enabled after the location changed (but at least we didn't crash!)");
+ log("Failure, Undo is enabled after the location changed (but at least we didn't crash!)");

if (window.testRunner)
testRunner.notifyDone();
Index: third_party/WebKit/Source/core/editing/Editor.cpp
diff --git a/third_party/WebKit/Source/core/editing/Editor.cpp b/third_party/WebKit/Source/core/editing/Editor.cpp
index 74818e022a20ffb40cba3ae648826a4ca11c544f..405ebf9a0d9366af7b89895cb1c4d9966faab026 100644
--- a/third_party/WebKit/Source/core/editing/Editor.cpp
+++ b/third_party/WebKit/Source/core/editing/Editor.cpp
@@ -206,11 +206,9 @@ EditorClient& Editor::client() const
return emptyEditorClient();
}

-UndoStack* Editor::undoStack() const
+UndoStack& Editor::undoStack() const
{
- if (Page* page = frame().page())
- return &page->undoStack();
- return 0;
+ return *m_undoStack;
}

bool Editor::handleTextEvent(TextEvent* event)
@@ -776,8 +774,7 @@ void Editor::appliedEditing(CompositeEditCommand* cmd)
// Only register a new undo command if the command passed in is
// different from the last command
m_lastEditCommand = cmd;
- if (UndoStack* undoStack = this->undoStack())
- undoStack->registerUndoStep(m_lastEditCommand->ensureComposition());
+ m_undoStack->registerUndoStep(m_lastEditCommand->ensureComposition());
}

respondToChangedContents(newSelection);
@@ -797,8 +794,7 @@ void Editor::unappliedEditing(EditCommandComposition* cmd)
changeSelectionAfterCommand(newSelection, FrameSelection::CloseTyping | FrameSelection::ClearTypingStyle);

m_lastEditCommand = nullptr;
- if (UndoStack* undoStack = this->undoStack())
- undoStack->registerRedoStep(cmd);
+ m_undoStack->registerRedoStep(cmd);
respondToChangedContents(newSelection);
}

@@ -814,8 +810,7 @@ void Editor::reappliedEditing(EditCommandComposition* cmd)
changeSelectionAfterCommand(newSelection, FrameSelection::CloseTyping | FrameSelection::ClearTypingStyle);

m_lastEditCommand = nullptr;
- if (UndoStack* undoStack = this->undoStack())
- undoStack->registerUndoStep(cmd);
+ m_undoStack->registerUndoStep(cmd);
respondToChangedContents(newSelection);
}

@@ -826,6 +821,7 @@ Editor* Editor::create(LocalFrame& frame)

Editor::Editor(LocalFrame& frame)
: m_frame(&frame)
+ , m_undoStack(UndoStack::create())
, m_preventRevealSelection(0)
, m_shouldStartNewKillRingSequence(false)
// This is off by default, since most editors want this behavior (this matches IE but not FF).
@@ -1060,28 +1056,22 @@ void Editor::copyImage(const HitTestResult& result)

bool Editor::canUndo()
{
- if (UndoStack* undoStack = this->undoStack())
- return undoStack->canUndo();
- return false;
+ return m_undoStack->canUndo();
}

void Editor::undo()
{
- if (UndoStack* undoStack = this->undoStack())
- undoStack->undo();
+ m_undoStack->undo();
}

bool Editor::canRedo()
{
- if (UndoStack* undoStack = this->undoStack())
- return undoStack->canRedo();
- return false;
+ return m_undoStack->canRedo();
}

void Editor::redo()
{
- if (UndoStack* undoStack = this->undoStack())
- undoStack->redo();
+ m_undoStack->redo();
}

void Editor::setBaseWritingDirection(WritingDirection direction)
@@ -1434,6 +1424,7 @@ DEFINE_TRACE(Editor)
{
visitor->trace(m_frame);
visitor->trace(m_lastEditCommand);
+ visitor->trace(m_undoStack);
visitor->trace(m_mark);
}

Index: third_party/WebKit/Source/core/editing/Editor.h
diff --git a/third_party/WebKit/Source/core/editing/Editor.h b/third_party/WebKit/Source/core/editing/Editor.h
index e2bd766a26d295c6f3010f3a02cfe89e87d24b9d..a80bae98d764c9001859b9fedf02156bde765477 100644
--- a/third_party/WebKit/Source/core/editing/Editor.h
+++ b/third_party/WebKit/Source/core/editing/Editor.h
@@ -233,6 +233,8 @@ public:
EditorParagraphSeparator defaultParagraphSeparator() const { return m_defaultParagraphSeparator; }
void setDefaultParagraphSeparator(EditorParagraphSeparator separator) { m_defaultParagraphSeparator = separator; }

+ UndoStack& undoStack() const;
+
static void tidyUpHTMLStructure(Document&);

class RevealSelectionScope {
@@ -253,6 +255,7 @@ public:
private:
Member<LocalFrame> m_frame;
Member<CompositeEditCommand> m_lastEditCommand;
+ Member<UndoStack> m_undoStack;
int m_preventRevealSelection;
bool m_shouldStartNewKillRingSequence;
bool m_shouldStyleWithCSS;
@@ -273,8 +276,6 @@ private:
bool canDeleteRange(const EphemeralRange&) const;
bool shouldDeleteRange(const EphemeralRange&) const;

- UndoStack* undoStack() const;
-
bool tryDHTMLCopy();
bool tryDHTMLCut();
bool tryDHTMLPaste(PasteMode);
Index: third_party/WebKit/Source/core/editing/commands/UndoStack.cpp
diff --git a/third_party/WebKit/Source/core/editing/commands/UndoStack.cpp b/third_party/WebKit/Source/core/editing/commands/UndoStack.cpp
index 778e50f556ffbc4e5bc36735d705130aae9856a5..7c50a2942978072a03f64afddd4fc47f45682075 100644
--- a/third_party/WebKit/Source/core/editing/commands/UndoStack.cpp
+++ b/third_party/WebKit/Source/core/editing/commands/UndoStack.cpp
@@ -63,23 +63,12 @@ void UndoStack::registerRedoStep(UndoStep* step)
m_redoStack.append(step);
}

-void UndoStack::didUnloadFrame(const LocalFrame& frame)
+void UndoStack::didUnloadFrame()
{
+ // TODO(xiaochengh): What's the use of this scope? Can we remove it?
EventDispatchForbiddenScope assertNoEventDispatch;
- filterOutUndoSteps(m_undoStack, frame);
- filterOutUndoSteps(m_redoStack, frame);
-}
-
-void UndoStack::filterOutUndoSteps(UndoStepStack& stack, const LocalFrame& frame)
-{
- UndoStepStack newStack;
- while (!stack.isEmpty()) {
- UndoStep* step = stack.first().get();
- if (!step->belongsTo(frame))
- newStack.append(step);
- stack.removeFirst();
- }
- stack.swap(newStack);
+ m_undoStack.clear();
+ m_redoStack.clear();
}

bool UndoStack::canUndo() const
Index: third_party/WebKit/Source/core/editing/commands/UndoStack.h
diff --git a/third_party/WebKit/Source/core/editing/commands/UndoStack.h b/third_party/WebKit/Source/core/editing/commands/UndoStack.h
index 39615d15d3bac4c46217529dc1f7faf8130ace83..e1d61e2aa977205caaafcb046e7c1de71851661b 100644
--- a/third_party/WebKit/Source/core/editing/commands/UndoStack.h
+++ b/third_party/WebKit/Source/core/editing/commands/UndoStack.h
@@ -47,7 +47,7 @@ public:

void registerUndoStep(UndoStep*);
void registerRedoStep(UndoStep*);
- void didUnloadFrame(const LocalFrame&);
+ void didUnloadFrame();
bool canUndo() const;
bool canRedo() const;
void undo();
@@ -60,8 +60,6 @@ private:

typedef HeapDeque<Member<UndoStep>> UndoStepStack;

- void filterOutUndoSteps(UndoStepStack&, const LocalFrame&);
-
bool m_inRedo;
UndoStepStack m_undoStack;
UndoStepStack m_redoStack;
Index: third_party/WebKit/Source/core/loader/FrameLoader.cpp
diff --git a/third_party/WebKit/Source/core/loader/FrameLoader.cpp b/third_party/WebKit/Source/core/loader/FrameLoader.cpp
index 2e480c0c4290bfb72f1189103f626fb446adbe82..74ae88074df1dffb232994aa6cb00a9805cd96b8 100644
--- a/third_party/WebKit/Source/core/loader/FrameLoader.cpp
+++ b/third_party/WebKit/Source/core/loader/FrameLoader.cpp
@@ -270,8 +270,9 @@ void FrameLoader::dispatchUnloadEvent()
if (m_frame->document() && !SVGImage::isInSVGImage(m_frame->document()))
m_frame->document()->dispatchUnloadEvents();

- if (Page* page = m_frame->page())
- page->undoStack().didUnloadFrame(*m_frame);
+ // TODO(xiaochengh): can the undo stack be used again after frame unloading?
+ // If not, we can simply remove |UndoStack::didUnloadFrame()|.
+ m_frame->editor().undoStack().didUnloadFrame();
}

void FrameLoader::didExplicitOpen()
Index: third_party/WebKit/Source/core/page/Page.cpp
diff --git a/third_party/WebKit/Source/core/page/Page.cpp b/third_party/WebKit/Source/core/page/Page.cpp
index 270e298c1c35d0db22488e97117b180cc0310be5..ff71fae0c65ade2aa799053f8718ba2b8a3e55f3 100644
--- a/third_party/WebKit/Source/core/page/Page.cpp
+++ b/third_party/WebKit/Source/core/page/Page.cpp
@@ -24,7 +24,6 @@
#include "core/dom/StyleChangeReason.h"
#include "core/dom/VisitedLinkState.h"
#include "core/editing/DragCaretController.h"
-#include "core/editing/commands/UndoStack.h"
#include "core/editing/markers/DocumentMarkerController.h"
#include "core/events/Event.h"
#include "core/fetch/ResourceFetcher.h"
@@ -116,7 +115,6 @@ Page::Page(PageClients& pageClients)
, m_focusController(FocusController::create(this))
, m_contextMenuController(ContextMenuController::create(this, pageClients.contextMenuClient))
, m_pointerLockController(PointerLockController::create(this))
- , m_undoStack(UndoStack::create())
, m_mainFrame(nullptr)
, m_editorClient(pageClients.editorClient)
, m_spellCheckerClient(pageClients.spellCheckerClient)
@@ -474,7 +472,6 @@ DEFINE_TRACE(Page)
visitor->trace(m_contextMenuController);
visitor->trace(m_pointerLockController);
visitor->trace(m_scrollingCoordinator);
- visitor->trace(m_undoStack);
visitor->trace(m_mainFrame);
visitor->trace(m_validationMessageClient);
visitor->trace(m_frameHost);
Index: third_party/WebKit/Source/core/page/Page.h
diff --git a/third_party/WebKit/Source/core/page/Page.h b/third_party/WebKit/Source/core/page/Page.h
index b56d35aa02a9b090247719f2c2a1777ceb2fc547..c22f2b0c1d501f9d371a2141cd52ad7041e6c97c 100644
--- a/third_party/WebKit/Source/core/page/Page.h
+++ b/third_party/WebKit/Source/core/page/Page.h
@@ -62,7 +62,6 @@ class PointerLockController;
class ScrollingCoordinator;
class Settings;
class SpellCheckerClient;
-class UndoStack;
class ValidationMessageClient;
class WebLayerTreeView;

@@ -124,7 +123,6 @@ public:

EditorClient& editorClient() const { return *m_editorClient; }
SpellCheckerClient& spellCheckerClient() const { return *m_spellCheckerClient; }
- UndoStack& undoStack() const { return *m_undoStack; }

void setMainFrame(Frame*);
Frame* mainFrame() const { return m_mainFrame; }
@@ -226,7 +224,6 @@ private:
const Member<ContextMenuController> m_contextMenuController;
const Member<PointerLockController> m_pointerLockController;
Member<ScrollingCoordinator> m_scrollingCoordinator;
- const Member<UndoStack> m_undoStack;

// Typically, the main frame and Page should both be owned by the embedder,
// which must call Page::willBeDestroyed() prior to destroying Page. This

[yosin@chromium.org via codereview.chromium.org]

https://codereview.chromium.org/2110543008/diff/20001/third_party/WebKit/LayoutTests/editing/undo/undo-iframe-location-change.html
File
third_party/WebKit/LayoutTests/editing/undo/undo-iframe-location-change.html
(right):

https://codereview.chromium.org/2110543008/diff/20001/third_party/WebKit/LayoutTests/editing/undo/undo-iframe-location-change.html#newcode12
third_party/WebKit/LayoutTests/editing/undo/undo-iframe-location-change.html:12:
function part1() {
Optional: Could you convert this test to use w3c test harness to get rid
of undo-iframe-location-change-expected.html and upstream to w3c test
repository.
Example: http://crrev.com/2010333003

https://codereview.chromium.org/2110543008/diff/20001/third_party/WebKit/LayoutTests/editing/undo/undo-iframe-location-change.html#newcode15
third_party/WebKit/LayoutTests/editing/undo/undo-iframe-location-change.html:15:

// Hack to perform the editing command. Should not be able to

How about just say "Call execCommand for subframe document rather than
main window"?
Or better English statement. ;-)

https://codereview.chromium.org/2110543008/diff/20001/third_party/WebKit/Source/core/editing/commands/UndoStack.cpp
File third_party/WebKit/Source/core/editing/commands/UndoStack.cpp
(right):

https://codereview.chromium.org/2110543008/diff/20001/third_party/WebKit/Source/core/editing/commands/UndoStack.cpp#newcode68
third_party/WebKit/Source/core/editing/commands/UndoStack.cpp:68: //

TODO(xiaochengh): What's the use of this scope? Can we remove it?

This event scope to prevent JavaScript event handlers to destruct
|frame|.
Since JavaScript is never executed during |m_{undo,redo}Stack.clear()|,
we can remove this |EventDispatchForbiddenScope|.

https://codereview.chromium.org/2110543008/diff/20001/third_party/WebKit/Source/core/loader/FrameLoader.cpp
File third_party/WebKit/Source/core/loader/FrameLoader.cpp (right):

https://codereview.chromium.org/2110543008/diff/20001/third_party/WebKit/Source/core/loader/FrameLoader.cpp#newcode273
third_party/WebKit/Source/core/loader/FrameLoader.cpp:273: //

TODO(xiaochengh): can the undo stack be used again after frame
unloading?

TL;DR: No, we don't use undo stack after frame unloading.

In theory, following scenario should work:

1. <body contenteditable>foo<iframe srcdoc="<body
contenteditable>bar</body>"></iframe>baz</body>
2. Type "quux" after "bar" in IFRAME's BODY
3. Select "foo" to "baz" in main frame
4. Hit Delete key in main frame
4. Undo in main frame
=> IFRAME should be shown with "barbaz"
5. Undo in iframe
=> IFRAME should show "bar"

Chrome, Edge and Firefox doesn't show "barbaz". They show "bar" only.
So, we don't use UndoStack after frame destruction.

This means we can get rid of |UndoStack::didUnloadFrame()|.
WDYT?

https://codereview.chromium.org/2110543008/

[xiaoc...@chromium.org]

PTAL at Patch 3.

|UndoStack::didUnloadFrame()| is completely removed since it doesn't do any
useful work now.

|Editor::undoStack()| is removed since Editor is now the only client class of
UndoStack.

The layout test is also rewritten using w3c test harness.

https://codereview.chromium.org/2110543008/diff/20001/third_party/WebKit/LayoutTests/editing/undo/undo-iframe-location-change.html
File
third_party/WebKit/LayoutTests/editing/undo/undo-iframe-location-change.html
(right):

https://codereview.chromium.org/2110543008/diff/20001/third_party/WebKit/LayoutTests/editing/undo/undo-iframe-location-change.html#newcode12
third_party/WebKit/LayoutTests/editing/undo/undo-iframe-location-change.html:12:
function part1() {

On 2016/07/01 at 09:21:21, Yosi_UTC9 wrote:
> Optional: Could you convert this test to use w3c test harness to get
rid of undo-iframe-location-change-expected.html and upstream to w3c
test repository.
> Example: http://crrev.com/2010333003

Done.

https://codereview.chromium.org/2110543008/diff/20001/third_party/WebKit/LayoutTests/editing/undo/undo-iframe-location-change.html#newcode15
third_party/WebKit/LayoutTests/editing/undo/undo-iframe-location-change.html:15:
// Hack to perform the editing command. Should not be able to

On 2016/07/01 at 09:21:21, Yosi_UTC9 wrote:
> How about just say "Call execCommand for subframe document rather than
main window"?
> Or better English statement. ;-)

Done (kind of).

https://codereview.chromium.org/2110543008/diff/20001/third_party/WebKit/Source/core/editing/commands/UndoStack.cpp
File third_party/WebKit/Source/core/editing/commands/UndoStack.cpp
(right):

https://codereview.chromium.org/2110543008/diff/20001/third_party/WebKit/Source/core/editing/commands/UndoStack.cpp#newcode68
third_party/WebKit/Source/core/editing/commands/UndoStack.cpp:68: //
TODO(xiaochengh): What's the use of this scope? Can we remove it?

On 2016/07/01 at 09:21:21, Yosi_UTC9 wrote:
> This event scope to prevent JavaScript event handlers to destruct
|frame|.
> Since JavaScript is never executed during
|m_{undo,redo}Stack.clear()|, we can remove this
|EventDispatchForbiddenScope|.

Thanks. This method is removed.

https://codereview.chromium.org/2110543008/diff/20001/third_party/WebKit/Source/core/loader/FrameLoader.cpp
File third_party/WebKit/Source/core/loader/FrameLoader.cpp (right):

https://codereview.chromium.org/2110543008/diff/20001/third_party/WebKit/Source/core/loader/FrameLoader.cpp#newcode273
third_party/WebKit/Source/core/loader/FrameLoader.cpp:273: //
TODO(xiaochengh): can the undo stack be used again after frame
unloading?

On 2016/07/01 at 09:21:21, Yosi_UTC9 wrote:
> TL;DR: No, we don't use undo stack after frame unloading.
>
> In theory, following scenario should work:
>
> 1. <body contenteditable>foo<iframe srcdoc="<body
contenteditable>bar</body>"></iframe>baz</body>
> 2. Type "quux" after "bar" in IFRAME's BODY
> 3. Select "foo" to "baz" in main frame
> 4. Hit Delete key in main frame
> 4. Undo in main frame
> => IFRAME should be shown with "barbaz"
> 5. Undo in iframe
> => IFRAME should show "bar"
>
> Chrome, Edge and Firefox doesn't show "barbaz". They show "bar" only.
> So, we don't use UndoStack after frame destruction.
>
> This means we can get rid of |UndoStack::didUnloadFrame()|.
> WDYT?

I agree. Removed.

https://codereview.chromium.org/2110543008/

[yosin@chromium.org via codereview.chromium.org]

lgtm w/ small nit

Thanks for converting test!
Good job! (^_^)b


https://codereview.chromium.org/2110543008/diff/40001/third_party/WebKit/Source/core/editing/Editor.h
File third_party/WebKit/Source/core/editing/Editor.h (right):

https://codereview.chromium.org/2110543008/diff/40001/third_party/WebKit/Source/core/editing/Editor.h#newcode256
third_party/WebKit/Source/core/editing/Editor.h:256: Member<UndoStack>
m_undoStack;
nit: Let's use|const Member<UndoStack>|, since it is read-only member
variable.

https://codereview.chromium.org/2110543008/

[tk...@chromium.org]

lgtm




https://codereview.chromium.org/2110543008/diff/40001/third_party/WebKit/Source/core/editing/commands/UndoStack.h
File third_party/WebKit/Source/core/editing/commands/UndoStack.h
(right):

https://codereview.chromium.org/2110543008/diff/40001/third_party/WebKit/Source/core/editing/commands/UndoStack.h#newcode43
third_party/WebKit/Source/core/editing/commands/UndoStack.h:43: class
UndoStack final : public GarbageCollected<UndoStack> {
Please add a comment that UndoStack is owned by Editor, i.e. per-Frame.

https://codereview.chromium.org/2110543008/

[xiaoc...@chromium.org]

Thanks for the review!

I'm going ahead to commit since all comments are addressed.

https://codereview.chromium.org/2110543008/diff/40001/third_party/WebKit/Source/core/editing/Editor.h
File third_party/WebKit/Source/core/editing/Editor.h (right):

https://codereview.chromium.org/2110543008/diff/40001/third_party/WebKit/Source/core/editing/Editor.h#newcode256
third_party/WebKit/Source/core/editing/Editor.h:256: Member<UndoStack>
m_undoStack;

On 2016/07/04 at 07:27:41, Yosi_UTC9 wrote:
> nit: Let's use|const Member<UndoStack>|, since it is read-only member
variable.

Done.

https://codereview.chromium.org/2110543008/diff/40001/third_party/WebKit/Source/core/editing/commands/UndoStack.h
File third_party/WebKit/Source/core/editing/commands/UndoStack.h
(right):

https://codereview.chromium.org/2110543008/diff/40001/third_party/WebKit/Source/core/editing/commands/UndoStack.h#newcode43
third_party/WebKit/Source/core/editing/commands/UndoStack.h:43: class
UndoStack final : public GarbageCollected<UndoStack> {

On 2016/07/04 at 07:44:36, tkent wrote:
> Please add a comment that UndoStack is owned by Editor, i.e.
per-Frame.

Done.

https://codereview.chromium.org/2110543008/

[commit-bot@chromium.org via codereview.chromium.org]

CQ is trying da patch. Follow status at

https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2110543008/60001


https://codereview.chromium.org/2110543008/

[commit-bot@chromium.org via codereview.chromium.org]

Committed patchset #4 (id:60001)

https://codereview.chromium.org/2110543008/

[commit-bot@chromium.org via codereview.chromium.org]

CQ bit was unchecked.

https://codereview.chromium.org/2110543008/

[commit-bot@chromium.org via codereview.chromium.org]

Patchset 4 (id:??) landed as
https://crrev.com/6447f384cf6d95f70475798e3fd45b689316ce50
Cr-Commit-Position: refs/heads/master@{#403653}

https://codereview.chromium.org/2110543008/

[ksak...@chromium.org]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/2127463002/ by ksak...@chromium.org.

The reason for reverting is: speculative revert to see whether this caused
crbug.com/625736.

https://codereview.chromium.org/2110543008/

[tk...@chromium.org]

Some tests were flaky during Patch Set 4 was checked in.

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=editing%2Fselection%2Fprogrammatic-selection-on-mac-is-directionless.html
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=editing%2FexecCommand%2Fapply-inline-style-to-element-with-no-renderer-crash.html
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=editing%2Fundo%2Fundo-iframe-location-change.html

https://codereview.chromium.org/2110543008/