Fix an issue with not clearing |selected_text_| when text selection range is empty (Mac)

Fix an issue with not clearing |selected_text_| when text selection range is empty (Mac)

After the CL (https://codereview.chromium.org/2240553003) on text selection
tracking (for OOPIF) the new implementation missed some minor details in
tracking text selection. Specificaly, the value of RWHVMac::selected_text_
is not cleared when reported text or range from renderer is empty.

This has caused a regression in context menu items where some of the items shown
could relate to the previous selection.

This CL will:
    1- Make sure |selected_text_| is cleared when reported text selection from
       renderer is empty.

    2- Makes sure the overal behavior is perserved from that of text selection
       before OOPIF-IME changes.

BUG=675514
Committed: https://crrev.com/a7d3c829829627d5739ee07b24c79ea97fc57886
Cr-Commit-Position: refs/heads/master@{#440550}

[EhsanK, 2016-12-22 18:16:46]

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

[commit-bot: I haz the power, 2016-12-22 18:17:09]

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

[EhsanK, 2016-12-22 20:06:22]

ekaramad@chromium.org changed reviewers:
+ avi@chromium.org

[EhsanK, 2016-12-22 20:06:23]

Hello Avi,

Could you please review this CL?

Thanks!

https://codereview.chromium.org/2596303002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host_view_mac.mm (right):

https://codereview.chromium.org/2596303002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_view_mac.mm:1015: return;
This is to make sure we do not pass on incorrect range and selection text
values. It is compatible with previous behavior where we were DCHECK-ing and
then returning early.

https://codereview.chromium.org/2596303002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/text_input_manager.cc (right):

https://codereview.chromium.org/2596303002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/text_input_manager.cc:336: if (pos + n >
text.length()) {
Before, we used to DCHECK on this so return false is not necessary.

https://codereview.chromium.org/2596303002/diff/20001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_view_mac_unittest.mm
(right):

https://codereview.chromium.org/2596303002/diff/20001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_mac_unittest.mm:313:
private:
Weird formatting here with private->protected->private and also some spacing
between methods are wrong.

I can file this as a separate CL TBR if that sounds OK.

[Avi (use Gerrit), 2016-12-22 20:11:56]

lgtm

https://codereview.chromium.org/2596303002/diff/20001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_view_mac_unittest.mm
(right):

https://codereview.chromium.org/2596303002/diff/20001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_mac_unittest.mm:313:
private:
On 2016/12/22 20:06:23, EhsanK wrote:
> Weird formatting here with private->protected->private and also some spacing
> between methods are wrong.
> 
> I can file this as a separate CL TBR if that sounds OK.

Don't TBR, but please do send a followup CL to me to clean it up.

[commit-bot: I haz the power, 2016-12-22 20:12:24]

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

[commit-bot: I haz the power, 2016-12-22 20:12:25]

Dry run: This issue passed the CQ dry run.

[EhsanK, 2016-12-22 23:56:28]

The CQ bit was checked by ekaramad@chromium.org

[EhsanK, 2016-12-22 23:56:32]

The CQ bit was unchecked by ekaramad@chromium.org

[EhsanK, 2016-12-23 00:04:28]

The CQ bit was checked by ekaramad@chromium.org

[commit-bot: I haz the power, 2016-12-23 00:05:02]

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, 2016-12-23 00:08:57]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1482451468942230,
"parent_rev": "a1a8370e8e8a82b4d8566f20339c1282017ce1fb", "commit_rev":
"eceb5372a8a4680a82f5211cbf594c2351e290b3"}

[commit-bot: I haz the power, 2016-12-23 00:09:25]

Description was changed from

==========
Fix an issue with not clearing |selected_text_| when text selection range is
empty (Mac)

After the CL (https://codereview.chromium.org/2240553003) on text selection
tracking (for OOPIF) the new implementation missed some minor details in
tracking text selection. Specificaly, the value of RWHVMac::selected_text_
is not cleared when reported text or range from renderer is empty.

This has caused a regression in context menu items where some of the items shown
could relate to the previous selection.

This CL will:
    1- Make sure |selected_text_| is cleared when reported text selection from
       renderer is empty.

    2- Makes sure the overal behavior is perserved from that of text selection
       before OOPIF-IME changes.

BUG=675514
==========

to

==========
Fix an issue with not clearing |selected_text_| when text selection range is
empty (Mac)

After the CL (https://codereview.chromium.org/2240553003) on text selection
tracking (for OOPIF) the new implementation missed some minor details in
tracking text selection. Specificaly, the value of RWHVMac::selected_text_
is not cleared when reported text or range from renderer is empty.

This has caused a regression in context menu items where some of the items shown
could relate to the previous selection.

This CL will:
    1- Make sure |selected_text_| is cleared when reported text selection from
       renderer is empty.

    2- Makes sure the overal behavior is perserved from that of text selection
       before OOPIF-IME changes.

BUG=675514

Review-Url: https://codereview.chromium.org/2596303002
==========

[commit-bot: I haz the power, 2016-12-23 00:09:27]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-12-23 00:13:27]

Description was changed from

==========
Fix an issue with not clearing |selected_text_| when text selection range is
empty (Mac)

After the CL (https://codereview.chromium.org/2240553003) on text selection
tracking (for OOPIF) the new implementation missed some minor details in
tracking text selection. Specificaly, the value of RWHVMac::selected_text_
is not cleared when reported text or range from renderer is empty.

This has caused a regression in context menu items where some of the items shown
could relate to the previous selection.

This CL will:
    1- Make sure |selected_text_| is cleared when reported text selection from
       renderer is empty.

    2- Makes sure the overal behavior is perserved from that of text selection
       before OOPIF-IME changes.

BUG=675514

Review-Url: https://codereview.chromium.org/2596303002
==========

to

==========
Fix an issue with not clearing |selected_text_| when text selection range is
empty (Mac)

After the CL (https://codereview.chromium.org/2240553003) on text selection
tracking (for OOPIF) the new implementation missed some minor details in
tracking text selection. Specificaly, the value of RWHVMac::selected_text_
is not cleared when reported text or range from renderer is empty.

This has caused a regression in context menu items where some of the items shown
could relate to the previous selection.

This CL will:
    1- Make sure |selected_text_| is cleared when reported text selection from
       renderer is empty.

    2- Makes sure the overal behavior is perserved from that of text selection
       before OOPIF-IME changes.

BUG=675514

Committed: https://crrev.com/a7d3c829829627d5739ee07b24c79ea97fc57886
Cr-Commit-Position: refs/heads/master@{#440550}
==========

[commit-bot: I haz the power, 2016-12-23 00:13:28]

Patchset 2 (id:??) landed as
https://crrev.com/a7d3c829829627d5739ee07b24c79ea97fc57886
Cr-Commit-Position: refs/heads/master@{#440550}