[SelectMiss...Async #2] Change ContextMenuClient::SelectMisspellingAsync() to return pair of strings

Change ContextMenuClient::SelectMisspellingAsync() to return pair of strings

Currently this method returns a String that holds the text marked by the
spelling/grammar marker, and also takes a reference to a String as a param and
uses it to return the value of the marker's Description field.

This CL changes this method to eliminate the String& param and instead return
Optional<std::pair<String, String>> to improve code health.

BUG=

Review-Url: https://codereview.chromium.org/2978823002
Cr-Commit-Position: refs/heads/master@{#489139}
Committed: https://chromium.googlesource.com/chromium/src/+/4a07e22a932cb25cc338f6c10f6d04932ef95bd9

[rlanday, 2017-07-12 19:55:25]

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

[rlanday, 2017-07-12 19:55:53]

Note: this is a follow-up to this CL: https://codereview.chromium.org/2966223003

[yosin_UTC9, 2017-07-20 01:00:54]

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

https://codereview.chromium.org/2978823002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.cpp:915: return
Optional<std::pair<String, String>>();
I think we write |return {};|
Could you try this to make code simpler?

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

https://codereview.chromium.org/2978823002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/page/ContextMenuClient.cpp:365:
data.misspelled_word = misspelled_word_and_description
Because both |data.misspell_word| and |description| accept empty string,
we can make |SelectMisspellingAsync()| to return |std::pair<String, String>|.

Even if other clients, they can treat empty string as no misspelled word.

[rlanday, 2017-07-20 21:13:59]

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

[commit-bot: I haz the power, 2017-07-20 21:14:11]

Dry run: CQ is trying da patch.

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

[rlanday, 2017-07-20 21:14:28]

Updated

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

https://codereview.chromium.org/2978823002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.cpp:915: return
Optional<std::pair<String, String>>();
On 2017/07/20 at 01:00:54, yosin_UTC9 wrote:
> I think we write |return {};|
> Could you try this to make code simpler?

I am simplifying the implementation of this method in:

[MarkersIntersectingRange #4] Refactor
ContextMenuClient::SelectMisspellingAsync()
https://codereview.chromium.org/2959553002

[rlanday, 2017-07-20 23:36:29]

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

[commit-bot: I haz the power, 2017-07-20 23:36:41]

Dry run: CQ is trying da patch.

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

[rlanday, 2017-07-20 23:37:07]

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

https://codereview.chromium.org/2978823002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.cpp:915: return
Optional<std::pair<String, String>>();
I've updated this CL to return {}

[commit-bot: I haz the power, 2017-07-20 23:41:12]

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

[commit-bot: I haz the power, 2017-07-20 23:41:15]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[rlanday, 2017-07-21 00:08:10]

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

[commit-bot: I haz the power, 2017-07-21 00:08:24]

Dry run: CQ is trying da patch.

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

[yosin_UTC9, 2017-07-21 00:59:43]

lgtm

[commit-bot: I haz the power, 2017-07-21 01:56:49]

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

[commit-bot: I haz the power, 2017-07-21 01:56:50]

Dry run: This issue passed the CQ dry run.

[rlanday, 2017-07-24 18:05:10]

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

[commit-bot: I haz the power, 2017-07-24 18:05:24]

Dry run: CQ is trying da patch.

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

[rlanday, 2017-07-24 18:29:01]

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

[rlanday, 2017-07-24 18:29:25]

Adding tkent@ for OWNER review of
third_party/WebKit/Source/core/page/ContextMenuClient.cpp

[commit-bot: I haz the power, 2017-07-24 20:00:29]

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

[commit-bot: I haz the power, 2017-07-24 20:00:29]

Dry run: This issue passed the CQ dry run.

[tkent, 2017-07-24 23:14:12]

lgtm

[rlanday, 2017-07-24 23:23:57]

The CQ bit was checked by rlanday@chromium.org

[rlanday, 2017-07-24 23:23:58]

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

[commit-bot: I haz the power, 2017-07-24 23:24:10]

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-07-24 23:28:16]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1500938637713170,
"parent_rev": "da9b7fc7b29513554b9ee7d9335e61d5f97727c5", "commit_rev":
"4a07e22a932cb25cc338f6c10f6d04932ef95bd9"}

[commit-bot: I haz the power, 2017-07-24 23:28:33]

Description was changed from

==========
Change ContextMenuClient::SelectMisspellingAsync() to return pair of strings

Currently this method returns a String that holds the text marked by the
spelling/grammar marker, and also takes a reference to a String as a param and
uses it to return the value of the marker's Description field.

This CL changes this method to eliminate the String& param and instead return
Optional<std::pair<String, String>> to improve code health.

BUG=
==========

to

==========
Change ContextMenuClient::SelectMisspellingAsync() to return pair of strings

Currently this method returns a String that holds the text marked by the
spelling/grammar marker, and also takes a reference to a String as a param and
uses it to return the value of the marker's Description field.

This CL changes this method to eliminate the String& param and instead return
Optional<std::pair<String, String>> to improve code health.

BUG=

Review-Url: https://codereview.chromium.org/2978823002
Cr-Commit-Position: refs/heads/master@{#489139}
Committed:
https://chromium.googlesource.com/chromium/src/+/4a07e22a932cb25cc338f6c10f6d...
==========

[commit-bot: I haz the power, 2017-07-24 23:28:36]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/4a07e22a932cb25cc338f6c10f6d...