Split general DocumentMarkerController::AddMarker() method into spelling/grammar versions

Split general DMC::AddMarker() method into spelling/grammar versions

Right now DocumentMarker::AddMarker() appears to be usable for any marker type,
when really the params it takes (start/end position and a description) are
really only suitable for spelling/grammar markers. So I'm splitting it into
AddSpellingMarker() and AddGrammarMarker(). After this, we'll have a 1-to-1
mapping between marker types and Add*Marker() methods.

I will change these methods in a follow-up CL to take an EphemeralRange instead
of two Positions.

BUG=707867
Review-Url: https://codereview.chromium.org/2895253003
Cr-Commit-Position: refs/heads/master@{#474532}
Committed: https://chromium.googlesource.com/chromium/src/+/43ffa71e810074eb21a15188cc408ab67039b0a8

[rlanday, 2017-05-22 23:20:08]

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

[rlanday, 2017-05-22 23:20:26]

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

[commit-bot: I haz the power, 2017-05-22 23:20:44]

Dry run: CQ is trying da patch.

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

[Xiaocheng, 2017-05-22 23:47:55]

lgtm

[commit-bot: I haz the power, 2017-05-23 01:08:10]

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

[commit-bot: I haz the power, 2017-05-23 01:08:10]

Dry run: This issue passed the CQ dry run.

[yosin_UTC9, 2017-05-23 09:35:04]

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

https://codereview.chromium.org/2895253003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/InputMethodControllerTest.cpp:1539:
GetDocument().Markers().AddTextMatchMarker(
Could you move IMCTest.cpp changes into another patch to make introducing
Add{Spelling,Grammar}Maker patch smaller.

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

https://codereview.chromium.org/2895253003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp:143:
DCHECK(type == DocumentMarker::kSpelling || type == DocumentMarker::kGrammar);
nit: s/);/) << type;/

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

https://codereview.chromium.org/2895253003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.cpp:541:
DCHECK(type == DocumentMarker::kSpelling || type == DocumentMarker::kGrammar);
nit: s/);/) << type;/

https://codereview.chromium.org/2895253003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.cpp:554:
description);
Please add |return| for early-return style.

https://codereview.chromium.org/2895253003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.cpp:556:
document->Markers().AddGrammarMarker(range_to_mark.StartPosition(),
Please add DCHECK_EQ(type, DM::kGrammar);

[rlanday, 2017-05-23 19:08:46]

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

[commit-bot: I haz the power, 2017-05-23 19:09:23]

Dry run: CQ is trying da patch.

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

[rlanday, 2017-05-23 19:09:33]

Ok, I've split off the InputMethodControllerTest changes into another CL:
https://codereview.chromium.org/2897103003

(This CL now depends on that one)

[commit-bot: I haz the power, 2017-05-23 19:17:26]

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

[commit-bot: I haz the power, 2017-05-23 19:17:28]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  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-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[yosin_UTC9, 2017-05-24 09:03:13]

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

[yosin_UTC9, 2017-05-24 09:03:14]

lgtm

+tkent@ for core/testing and web/

[tkent, 2017-05-24 09:05:52]

lgtm

[rlanday, 2017-05-25 01:00:15]

The CQ bit was checked by rlanday@chromium.org

[rlanday, 2017-05-25 01:00:15]

The patchset sent to the CQ was uploaded after l-g-t-m from
xiaochengh@chromium.org
Link to the patchset: https://codereview.chromium.org/2895253003/#ps20001
(title: "Split off InputMethodControllerTest changes, make other requested
changes")

[commit-bot: I haz the power, 2017-05-25 01:01:09]

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-05-25 02:40:23]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1495674015032920,
"parent_rev": "77c3cca04cb5a0501e2cd9c4db2b90db4fd246d4", "commit_rev":
"43ffa71e810074eb21a15188cc408ab67039b0a8"}

[commit-bot: I haz the power, 2017-05-25 02:40:34]

Description was changed from

==========
Split general DMC::AddMarker() method into spelling/grammar versions

Right now DocumentMarker::AddMarker() appears to be usable for any marker type,
when really the params it takes (start/end position and a description) are
really only suitable for spelling/grammar markers. So I'm splitting it into
AddSpellingMarker() and AddGrammarMarker(). After this, we'll have a 1-to-1
mapping between marker types and Add*Marker() methods.

I will change these methods in a follow-up CL to take an EphemeralRange instead
of two Positions.

BUG=707867
==========

to

==========
Split general DMC::AddMarker() method into spelling/grammar versions

Right now DocumentMarker::AddMarker() appears to be usable for any marker type,
when really the params it takes (start/end position and a description) are
really only suitable for spelling/grammar markers. So I'm splitting it into
AddSpellingMarker() and AddGrammarMarker(). After this, we'll have a 1-to-1
mapping between marker types and Add*Marker() methods.

I will change these methods in a follow-up CL to take an EphemeralRange instead
of two Positions.

BUG=707867

Review-Url: https://codereview.chromium.org/2895253003
Cr-Commit-Position: refs/heads/master@{#474532}
Committed:
https://chromium.googlesource.com/chromium/src/+/43ffa71e810074eb21a15188cc40...
==========

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

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/43ffa71e810074eb21a15188cc40...