Add sequence number to undo steps

Add sequence number to undo steps

This patch adds a monotonically increasing sequence number to each
undo steps, to better identify the undo steps according to the
execution history.

The sequence number will be utilized by idle time spell checker [1].

[1] https://docs.google.com/document/d/1vo71cnlGOqOuXyy1zjzXSlUIjG7GsWgRjqmkrq1g5hk/edit

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

Review-Url: https://codereview.chromium.org/2639483002
Cr-Commit-Position: refs/heads/master@{#452377}
Committed: https://chromium.googlesource.com/chromium/src/+/6228190e1f23752ec1dd6530b59ce77235ec5b01

[Xiaocheng, 2017-01-17 02:45:06]

The CQ bit was checked by xiaochengh@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-17 02:45:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Xiaocheng, 2017-01-17 02:45:57]

Description was changed from

==========
Add sequence number to undo steps

Idle time spell checker will use undo steps to identify the ranges for
spell checking, and needs to distinguish processed and unprocessed undo
steps [1]. This patch makes undo steps carry an ascending sequence
number so that which steps are processed can be easily determined.

[1]
https://docs.google.com/document/d/1vo71cnlGOqOuXyy1zjzXSlUIjG7GsWgRjqmkrq1g5...

BUG=517298
==========

to

==========
Add sequence number to undo steps

Idle time spell checker will use undo steps to identify the ranges for
spell checking, and needs to distinguish processed and unprocessed undo
steps [1]. This patch makes undo steps carry an ascending sequence
number so that which steps are processed can be easily determined.

[1]
https://docs.google.com/document/d/1vo71cnlGOqOuXyy1zjzXSlUIjG7GsWgRjqmkrq1g5...

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

[commit-bot: I haz the power, 2017-01-17 04:36:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-17 04:36:33]

Dry run: This issue passed the CQ dry run.

[Xiaocheng, 2017-01-17 07:46:57]

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

[Xiaocheng, 2017-01-17 07:46:57]

PTAL.

[yosin_UTC9, 2017-01-17 08:13:29]

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

https://codereview.chromium.org/2639483002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/commands/UndoStack.cpp:52:
step->setSequence(++m_maxSequence);
Usage of |m_maxSequence| isn't clear. Why increment here?

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

https://codereview.chromium.org/2639483002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/commands/UndoStack.h:67: int
m_maxSequence;
Should we use |uint64_t| or |size_t|? But, I'm not sure. Could you explain
rationale to use |int|?

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

https://codereview.chromium.org/2639483002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/commands/UndoStep.h:50: virtual int
sequence() const = 0;
Do we really want to keep |UndoStep| as interface?

I think it is better to have sequence number as const member variable
initialized in ctor.


BTW,
We should get rid of GarbageCollectedFinalized<UndoStep> since it doesn't have
member variables to track.

[Xiaocheng, 2017-01-17 09:04:02]

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

https://codereview.chromium.org/2639483002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/commands/UndoStack.cpp:52:
step->setSequence(++m_maxSequence);
On 2017/01/17 at 08:13:29, Yosi_UTC9 wrote:
> Usage of |m_maxSequence| isn't clear. Why increment here?

This is the place where a new undo step is pushed to the stack.
We don't want to set the sequence of an undo step if it's just popped from the
redo stack.

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

https://codereview.chromium.org/2639483002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/commands/UndoStack.h:67: int
m_maxSequence;
On 2017/01/17 at 08:13:29, Yosi_UTC9 wrote:
> Should we use |uint64_t| or |size_t|? But, I'm not sure. Could you explain
rationale to use |int|?

No special rationale, just an arbitrary choice.

Let's use |uint64_t| to be on the safe side.

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

https://codereview.chromium.org/2639483002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/commands/UndoStep.h:50: virtual int
sequence() const = 0;
On 2017/01/17 at 08:13:29, Yosi_UTC9 wrote:
> Do we really want to keep |UndoStep| as interface?

Do you mean removing |UndoStep| and using |EditCommandComposition| directly? In
fact what's the reason of having |UndoStep|?

> 
> I think it is better to have sequence number as const member variable
initialized in ctor.

Is the construction order of the steps same as the order they are pushed to undo
stack? If so, then yes.

> 
> 
> BTW,
> We should get rid of GarbageCollectedFinalized<UndoStep> since it doesn't have
member variables to track.

[Xiaocheng, 2017-01-18 05:20:15]

Discussed offline. We should get rid of UndoStep and use EditCommandComposition
directly.

[Xiaocheng, 2017-01-19 06:08:47]

The CQ bit was checked by xiaochengh@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-19 06:09:12]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Xiaocheng, 2017-01-19 06:10:25]

Description was changed from

==========
Add sequence number to undo steps

Idle time spell checker will use undo steps to identify the ranges for
spell checking, and needs to distinguish processed and unprocessed undo
steps [1]. This patch makes undo steps carry an ascending sequence
number so that which steps are processed can be easily determined.

[1]
https://docs.google.com/document/d/1vo71cnlGOqOuXyy1zjzXSlUIjG7GsWgRjqmkrq1g5...

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

to

==========
Add sequence number to undo steps

This patch adds an ascending sequence number to undo steps, so that
undo steps are numbered in the same order as they are pushed to the
undo stack.

The sequence number will be utilized by idle time spell checker [1].

[1]
https://docs.google.com/document/d/1vo71cnlGOqOuXyy1zjzXSlUIjG7GsWgRjqmkrq1g5...

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

[yosin_UTC9, 2017-01-19 06:40:52]

>This patch adds an ascending sequence number to undo steps, so that
>undo steps are numbered in the same order as they are pushed to the
>undo stack.

s/ascending sequence number/monotonically increased number/

I could write:
This patch introduces monotonically increased number as identifier to
undo step to know command execution history.

https://codereview.chromium.org/2639483002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/commands/UndoStep.h (right):

https://codereview.chromium.org/2639483002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/commands/UndoStep.h:68: uint64_t
sequence() const { return m_sequence; }
s/sequence/sequenceNumber()/

Term "sequence" is ambiguous, it can be list, array, etc.

https://codereview.chromium.org/2639483002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/commands/UndoStep.h:86: uint64_t
m_sequence;
Can we make |const unit64_t m_sequenceNumber| and initialized in constructor?
I fill changing sequence number is some what strange since usually sequence
number is used for object identifier.

[commit-bot: I haz the power, 2017-01-19 07:29:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-19 07:29:33]

Dry run: This issue passed the CQ dry run.

[Xiaocheng, 2017-01-19 07:52:07]

Description was changed from

==========
Add sequence number to undo steps

This patch adds an ascending sequence number to undo steps, so that
undo steps are numbered in the same order as they are pushed to the
undo stack.

The sequence number will be utilized by idle time spell checker [1].

[1]
https://docs.google.com/document/d/1vo71cnlGOqOuXyy1zjzXSlUIjG7GsWgRjqmkrq1g5...

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

to

==========
Add sequence number to undo steps

This patch adds a monotonically increasing sequence number to each
undo steps, to better identify the undo steps according to the
execution history.

The sequence number will be utilized by idle time spell checker [1].

[1]
https://docs.google.com/document/d/1vo71cnlGOqOuXyy1zjzXSlUIjG7GsWgRjqmkrq1g5...

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

[Xiaocheng, 2017-01-19 07:52:30]

Thanks for the review. Changed patch description accordingly.

https://codereview.chromium.org/2639483002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/commands/UndoStep.h (right):

https://codereview.chromium.org/2639483002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/commands/UndoStep.h:68: uint64_t
sequence() const { return m_sequence; }
On 2017/01/19 at 06:40:52, Yosi_UTC9 wrote:
> s/sequence/sequenceNumber()/
> 
> Term "sequence" is ambiguous, it can be list, array, etc.

Will do.

https://codereview.chromium.org/2639483002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/commands/UndoStep.h:86: uint64_t
m_sequence;
On 2017/01/19 at 06:40:52, Yosi_UTC9 wrote:
> Can we make |const unit64_t m_sequenceNumber| and initialized in constructor?
> I fill changing sequence number is some what strange since usually sequence
number is used for object identifier.

For undo steps, is the creation order same as the push-to-undo-stack order? We
can initialize the sequence number only if this is true.

[yosin_UTC9, 2017-01-19 08:14:02]

On 2017/01/19 at 07:52:30, xiaochengh wrote:
> Thanks for the review. Changed patch description accordingly.
> 
>
https://codereview.chromium.org/2639483002/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/editing/commands/UndoStep.h (right):
> 
>
https://codereview.chromium.org/2639483002/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/editing/commands/UndoStep.h:68: uint64_t
sequence() const { return m_sequence; }
> On 2017/01/19 at 06:40:52, Yosi_UTC9 wrote:
> > s/sequence/sequenceNumber()/
> > 
> > Term "sequence" is ambiguous, it can be list, array, etc.
> 
> Will do.
> 
>
https://codereview.chromium.org/2639483002/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/editing/commands/UndoStep.h:86: uint64_t
m_sequence;
> On 2017/01/19 at 06:40:52, Yosi_UTC9 wrote:
> > Can we make |const unit64_t m_sequenceNumber| and initialized in
constructor?
> > I fill changing sequence number is some what strange since usually sequence
number is used for object identifier.
> 
> For undo steps, is the creation order same as the push-to-undo-stack order? We
can initialize the sequence number only if this is true.

Should be same. If not, something is wrong. Or, we can adjust creation order of
UndoStep.

[Xiaocheng, 2017-01-19 08:31:47]

On 2017/01/19 at 08:14:02, yosin wrote:
> On 2017/01/19 at 07:52:30, xiaochengh wrote:
> > Thanks for the review. Changed patch description accordingly.
> > 
> >
https://codereview.chromium.org/2639483002/diff/20001/third_party/WebKit/Sour...
> > File third_party/WebKit/Source/core/editing/commands/UndoStep.h (right):
> > 
> >
https://codereview.chromium.org/2639483002/diff/20001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/core/editing/commands/UndoStep.h:68: uint64_t
sequence() const { return m_sequence; }
> > On 2017/01/19 at 06:40:52, Yosi_UTC9 wrote:
> > > s/sequence/sequenceNumber()/
> > > 
> > > Term "sequence" is ambiguous, it can be list, array, etc.
> > 
> > Will do.
> > 
> >
https://codereview.chromium.org/2639483002/diff/20001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/core/editing/commands/UndoStep.h:86: uint64_t
m_sequence;
> > On 2017/01/19 at 06:40:52, Yosi_UTC9 wrote:
> > > Can we make |const unit64_t m_sequenceNumber| and initialized in
constructor?
> > > I fill changing sequence number is some what strange since usually
sequence number is used for object identifier.
> > 
> > For undo steps, is the creation order same as the push-to-undo-stack order?
We can initialize the sequence number only if this is true.
> 
> Should be same. If not, something is wrong. Or, we can adjust creation order
of UndoStep.

Found a counterexample: 
+editing/selection/crash-on-drag-with-mutation-events.html

Investigating...

[Xiaocheng, 2017-01-27 07:19:46]

Investigation done. Conclusion: for undo steps, the creation order and enstack
order can be different, and this is expected in the current design. We should
use enstack order instead of creation order.

Example:

Command 1 application starts
  EventQueueScope starts
  Command 1 changes DOM, triggering some event
  EventQueueScope ends
  The event handler creates Command 2
  Command 2 applied and pushed to undo stack
Command 1 applied and pushed to undo stack

Here the creation order and enstack order of the two commands are reversed

[Xiaocheng, 2017-01-27 07:41:02]

The CQ bit was checked by xiaochengh@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-27 07:41:22]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Xiaocheng, 2017-01-27 09:13:12]

Discussed offline.

In the above example, the enstack order is wrong --- in any case, the enstack
order must be the reverse of the application order, which is violated in the
example.

[commit-bot: I haz the power, 2017-01-27 11:09:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-27 11:09:12]

Dry run: This issue passed the CQ dry run.

[Xiaocheng, 2017-02-17 19:54:05]

The CQ bit was checked by xiaochengh@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-17 19:54:56]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-17 20:36:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-17 20:36:42]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)

[Xiaocheng, 2017-02-17 21:06:08]

The CQ bit was checked by xiaochengh@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-17 21:07:00]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Xiaocheng, 2017-02-17 21:14:07]

The CQ bit was checked by xiaochengh@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-17 21:15:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-17 23:02:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-17 23:02:08]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[yosin_UTC9, 2017-02-20 06:17:50]

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

https://codereview.chromium.org/2639483002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/editing/commands/UndoStack.cpp:52: if (!m_inRedo)
{
nit: No need to have braces.

https://codereview.chromium.org/2639483002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/editing/commands/UndoStep.cpp (right):

https://codereview.chromium.org/2639483002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/editing/commands/UndoStep.cpp:13: 
nit: Get rid this extra blank line or add a blank before L15.

https://codereview.chromium.org/2639483002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/editing/commands/UndoStep.cpp:14: uint64_t
s_nextSequenceNumber = 0;
nit:s/nextSequneceNumber/currentSequenceNumber/
Since, we do pre-increment.

The another way is

uint64_t nextSequenceNumbr() {
  static uint64_t currentSequenceNumber = 0;
  return ++currentSequenceNumber;
}

This is more encapsulating management of sequence number.

[Xiaocheng, 2017-02-22 02:41:30]

The CQ bit was checked by xiaochengh@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-22 02:42:39]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Xiaocheng, 2017-02-22 02:43:34]

The CQ bit was checked by xiaochengh@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-22 02:44:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Xiaocheng, 2017-02-22 02:47:32]

Updated. PTAL.

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

https://codereview.chromium.org/2639483002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/editing/commands/UndoStack.cpp:52: if (!m_inRedo)
{
On 2017/02/20 at 06:17:50, yosin_UTC9 wrote:
> nit: No need to have braces.

Whoops. Done.

https://codereview.chromium.org/2639483002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/editing/commands/UndoStep.cpp (right):

https://codereview.chromium.org/2639483002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/editing/commands/UndoStep.cpp:13: 
On 2017/02/20 at 06:17:50, yosin_UTC9 wrote:
> nit: Get rid this extra blank line or add a blank before L15.

|git cl format| did this.

I'll remove the blank line.

https://codereview.chromium.org/2639483002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/editing/commands/UndoStep.cpp:14: uint64_t
s_nextSequenceNumber = 0;
On 2017/02/20 at 06:17:50, yosin_UTC9 wrote:
> nit:s/nextSequneceNumber/currentSequenceNumber/
> Since, we do pre-increment.

Done.

> 
> The another way is
> 
> uint64_t nextSequenceNumbr() {
>   static uint64_t currentSequenceNumber = 0;
>   return ++currentSequenceNumber;
> }
> 
> This is more encapsulating management of sequence number.

This appears to me as introducing unnecessary complexity...

[commit-bot: I haz the power, 2017-02-22 04:46:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-22 04:46:21]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)

[yosin_UTC9, 2017-02-22 05:04:07]

On 2017/02/22 at 02:47:32, xiaochengh wrote:
> > 
> > The another way is
> > 
> > uint64_t nextSequenceNumbr() {
> >   static uint64_t currentSequenceNumber = 0;
> >   return ++currentSequenceNumber;
> > }
> > 
> > This is more encapsulating management of sequence number.
> 
> This appears to me as introducing unnecessary complexity...

This is the poor man's solution for identifying places where we update the
counter.
And, prevent to write |currentSequenceNumber++|.

I don't think this increases complexity.

nextSequenceNumber: 4 line for function, 1 function call: total 5 lines
++crrentSquenceNumber: 1 line for declaration, 1 increment, 1 reference; total 2
line

Anyway, this is a kind of personal preference.

[yosin_UTC9, 2017-02-22 05:04:45]

lgtm

[yosin_UTC9, 2017-02-22 05:04:48]

The CQ bit was checked by yosin@chromium.org

[commit-bot: I haz the power, 2017-02-22 05:06:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-22 05:06:20]

This CL has an open dependency (Issue 2674173002 Patch 60001). Please resolve
the dependency and try again.
If you are sure that there is no real dependency, please use one of the options
listed in https://goo.gl/9Es4OR to land the CL.

[Xiaocheng, 2017-02-23 00:33:40]

The CQ bit was checked by xiaochengh@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-23 00:34:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-23 02:25:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-23 02:25:24]

Dry run: This issue passed the CQ dry run.

[Xiaocheng, 2017-02-23 03:32:00]

The CQ bit was checked by xiaochengh@chromium.org

[Xiaocheng, 2017-02-23 03:32:00]

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/2639483002/#ps180001
(title: "Rebase")

[commit-bot: I haz the power, 2017-02-23 03:33:04]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-23 03:38:30]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1487820720300960,
"parent_rev": "cf729d6b29b5ff36ef3115f88b45193c59ab3f52", "commit_rev":
"6228190e1f23752ec1dd6530b59ce77235ec5b01"}

[commit-bot: I haz the power, 2017-02-23 03:39:08]

Description was changed from

==========
Add sequence number to undo steps

This patch adds a monotonically increasing sequence number to each
undo steps, to better identify the undo steps according to the
execution history.

The sequence number will be utilized by idle time spell checker [1].

[1]
https://docs.google.com/document/d/1vo71cnlGOqOuXyy1zjzXSlUIjG7GsWgRjqmkrq1g5...

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

to

==========
Add sequence number to undo steps

This patch adds a monotonically increasing sequence number to each
undo steps, to better identify the undo steps according to the
execution history.

The sequence number will be utilized by idle time spell checker [1].

[1]
https://docs.google.com/document/d/1vo71cnlGOqOuXyy1zjzXSlUIjG7GsWgRjqmkrq1g5...

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

Review-Url: https://codereview.chromium.org/2639483002
Cr-Commit-Position: refs/heads/master@{#452377}
Committed:
https://chromium.googlesource.com/chromium/src/+/6228190e1f23752ec1dd6530b59c...
==========

[commit-bot: I haz the power, 2017-02-23 03:39:10]

Committed patchset #10 (id:180001) as
https://chromium.googlesource.com/chromium/src/+/6228190e1f23752ec1dd6530b59c...