Hide SpellCheckRequest from other classes in editing/spellcheck

Hide SpellCheckRequest from other classes in editing/spellcheck

Currently, all external clients of SpellCheckRequest use it in only
one way -- pass it to SpellCheckRequester::requestCheckingFor
directly.

Hence, this patch makes the clients call the function directly without
knowing about details of this class, to reduce exposure of implementation
details.

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

Review-Url: https://codereview.chromium.org/2740603003
Cr-Commit-Position: refs/heads/master@{#456240}
Committed: https://chromium.googlesource.com/chromium/src/+/659edf80d6784f79ddad9b384391bc11588df846

[Xiaocheng, 2017-03-08 02:49:16]

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

[commit-bot: I haz the power, 2017-03-08 02:49:54]

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-03-08 03:43:04]

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

[commit-bot: I haz the power, 2017-03-08 03:43:05]

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

[Xiaocheng, 2017-03-08 18:10:46]

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

[commit-bot: I haz the power, 2017-03-08 18:11: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-03-08 20:15:34]

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

[commit-bot: I haz the power, 2017-03-08 20:15:35]

Dry run: This issue passed the CQ dry run.

[Xiaocheng, 2017-03-08 21:16:51]

Description was changed from

==========
Hide SpellCheckRequest from other classes in editing/spellcheck

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

to

==========
Hide SpellCheckRequest from other classes in editing/spellcheck

Currently, all external clients of SpellCheckRequest use it in only
one way -- pass it to SpellCheckRequester::requestCheckingFor
directly.

Hence, this patch makes the clients call the function directly without
knowing about details of this class, to reduce exposure of implementation
details.

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

[Xiaocheng, 2017-03-08 21:17:01]

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

[Xiaocheng, 2017-03-08 21:17:01]

PTAL.

[yosin_UTC9, 2017-03-09 01:28:15]

lgtm w/ nits

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

https://codereview.chromium.org/2740603003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/spellcheck/SpellCheckRequester.cpp:152:
SpellCheckRequest* request = SpellCheckRequest::create(range, requestNum);
nit: s/SpellCheckRequest*/SpellCheckRequest* const/

Please add |DCHECK(!range.isNull())|.

https://codereview.chromium.org/2740603003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/editing/spellcheck/SpellCheckRequester.h
(left):

https://codereview.chromium.org/2740603003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/spellcheck/SpellCheckRequester.h:49: int
requestNumber = 0);
Good! We should avoid to use default parameter.

[Xiaocheng, 2017-03-10 23:13:22]

The CQ bit was checked by xiaochengh@chromium.org

[Xiaocheng, 2017-03-10 23:13:23]

Thanks for reviewing

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

https://codereview.chromium.org/2740603003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/spellcheck/SpellCheckRequester.cpp:152:
SpellCheckRequest* request = SpellCheckRequest::create(range, requestNum);
On 2017/03/09 at 01:28:15, yosin_UTC9 wrote:
> nit: s/SpellCheckRequest*/SpellCheckRequest* const/
> 
> Please add |DCHECK(!range.isNull())|.

SpellCheckRequest::ctor checks if the range is null.

Not sure if the current callers pass null ranges, and don't want to change the
code now...

[commit-bot: I haz the power, 2017-03-10 23:13:41]

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-03-11 01:07:00]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1489187602368660, "parent_rev":
"e1d64e748a34ba84f739207b74a3bba8a6c4b6af", "commit_rev":
"659edf80d6784f79ddad9b384391bc11588df846"}

[commit-bot: I haz the power, 2017-03-11 01:07:42]

Description was changed from

==========
Hide SpellCheckRequest from other classes in editing/spellcheck

Currently, all external clients of SpellCheckRequest use it in only
one way -- pass it to SpellCheckRequester::requestCheckingFor
directly.

Hence, this patch makes the clients call the function directly without
knowing about details of this class, to reduce exposure of implementation
details.

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

to

==========
Hide SpellCheckRequest from other classes in editing/spellcheck

Currently, all external clients of SpellCheckRequest use it in only
one way -- pass it to SpellCheckRequester::requestCheckingFor
directly.

Hence, this patch makes the clients call the function directly without
knowing about details of this class, to reduce exposure of implementation
details.

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

Review-Url: https://codereview.chromium.org/2740603003
Cr-Commit-Position: refs/heads/master@{#456240}
Committed:
https://chromium.googlesource.com/chromium/src/+/659edf80d6784f79ddad9b384391...
==========

[commit-bot: I haz the power, 2017-03-11 01:07:43]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/659edf80d6784f79ddad9b384391...