Tracking SelectionBounds for all RenderWidgets on the Browser Side (Mac)

Tracking SelectionBounds for all RenderWidgets on the Browser Side (Mac)

This CL uses the TextInputManager code path for tracking selection bounds on Mac OSX (a follow up to https://codereview.chromium.org/2057803002 for aura).
Other important changes include:
  * Activating the test SitePerProcessTextInputManagerTest.TrackSelectionBoundsForAllFrames on Mac OSX.
  * Removing all the overrides of SelectionBoundsChanged form Mac and Android.

BUG=578168, 602723, 602427
Committed: https://crrev.com/2f52009f42519eac3e40ff20718e31365c976773
Cr-Commit-Position: refs/heads/master@{#413568}

[EhsanK, 2016-08-03 20:30:06]

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

[commit-bot: I haz the power, 2016-08-03 20:30:33]

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

[EhsanK, 2016-08-03 20:35:30]

Description was changed from

==========
Tracking SelectionBounds for all RenderWidgets on the Browser Side (Mac)

This CL uses the TextInputManager code path for tracking selection bounds on Mac
OSX (a follow up to https://codereview.chromium.org/2057803002 for aura).
Other important changes include:
  * Activating the test
SitePerProcessTextInputManagerTest.TrackSelectionBoundsForAllFrames on Mac OSX.
  * Removing all the overrides of SelectionBoundsChanged form Mac and Android.

BUG=578168,602723
==========

to

==========
Tracking SelectionBounds for all RenderWidgets on the Browser Side (Mac)

This CL uses the TextInputManager code path for tracking selection bounds on Mac
OSX (a follow up to https://codereview.chromium.org/2057803002 for aura).
Other important changes include:
  * Activating the test
SitePerProcessTextInputManagerTest.TrackSelectionBoundsForAllFrames on Mac OSX.
  * Removing all the overrides of SelectionBoundsChanged form Mac and Android.

BUG=578168,602723,602427
==========

[commit-bot: I haz the power, 2016-08-03 20:44:40]

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

[commit-bot: I haz the power, 2016-08-03 20:44:42]

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

[EhsanK, 2016-08-03 23:16:36]

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

[EhsanK, 2016-08-03 23:16:36]

PTAL.

[Charlie Reis, 2016-08-04 18:44:51]

creis@chromium.org changed reviewers:
+ kenrb@chromium.org

[Charlie Reis, 2016-08-04 18:44:51]

Adding Ken to review as well.

I'm concerned about the platform-specific code within TextInputManager, so I'm
wondering if there's ways we can avoid it.  Maybe have that class support both
approaches and just use the relevant one on each platform?

https://codereview.chromium.org/2213503002/diff/40001/content/browser/rendere...
File content/browser/renderer_host/text_input_manager.h (right):

https://codereview.chromium.org/2213503002/diff/40001/content/browser/rendere...
content/browser/renderer_host/text_input_manager.h:112: #elif defined(OS_MACOSX)
Hmm, this seems unfortunate to need different selection APIs on different
platforms.  Ifdefs tend to have a code complexity cost, so I generally try to
avoid them unless they're part of a temporary transition, or a fundamental need.

Maybe we're stuck with this, but it's worth thinking whether there's a way to
have a consistent API across platforms here.

https://codereview.chromium.org/2213503002/diff/40001/content/browser/rendere...
content/browser/renderer_host/text_input_manager.h:189: #else   // OS_MACOSX
Same here, and more so.  This will be difficult to reason about and maintain,
and it'll be tough to follow the code in codesearch without cross-linking.

Is there a way to avoid this?

[EhsanK, 2016-08-04 23:53:01]

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

[EhsanK, 2016-08-04 23:53:03]

No new patch just some thoughts on how to avoid #ifdefs.

Also adding nasko@ since this is an IME for site isolation review.

kenrb@: Please take a look. Any suggestions on a nicer approach coding wise?

https://codereview.chromium.org/2213503002/diff/40001/content/browser/rendere...
File content/browser/renderer_host/text_input_manager.h (right):

https://codereview.chromium.org/2213503002/diff/40001/content/browser/rendere...
content/browser/renderer_host/text_input_manager.h:112: #elif defined(OS_MACOSX)
On 2016/08/04 18:44:51, Charlie Reis (OOO til 8-8) wrote:
> Hmm, this seems unfortunate to need different selection APIs on different
> platforms.  Ifdefs tend to have a code complexity cost, so I generally try to
> avoid them unless they're part of a temporary transition, or a fundamental
need.
> 
> Maybe we're stuck with this, but it's worth thinking whether there's a way to
> have a consistent API across platforms here.

There is more in the .cc file for each specific method.

The whole state tracking logic can be looked at 1) preprocessing the update
received from RWH and 2) making decision after that. Both parts were platform
specific before. The way we did TextInputManager, which might not necessarily be
optimal delegates part (2) to the tab's view for the platform specific
decisions. Part (1) unfortunately has to either be taken care of in RWHVBase, or
here.

I will give this another try tomorrow to see if I can refactor this better. That
being said, another alternative would be to have platform specific
TextInputManager implementations for Aura, Mac, and Android.

Also having the same class for all platforms, such as SelectionRegion below is
fine but I guess not idea since we consume more memory.

https://codereview.chromium.org/2213503002/diff/40001/content/browser/rendere...
content/browser/renderer_host/text_input_manager.h:189: #else   // OS_MACOSX
On 2016/08/04 18:44:51, Charlie Reis (OOO til 8-8) wrote:
> Same here, and more so.  This will be difficult to reason about and maintain,
> and it'll be tough to follow the code in codesearch without cross-linking.
> 
> Is there a way to avoid this?

Let me think about this. Meanwhile, all suggestions are welcome!

[Charlie Reis, 2016-08-08 20:58:01]

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

[Charlie Reis, 2016-08-08 20:58:01]

Avi, can you see if you have any suggestions for how to handle Mac-specific code
here?  Ehsan is trying to add IME support for Mac, and there's some platform
specific state and logic.  The current ifdefs don't seem great to me, though I'm
not sure if we want to end up with a bunch of platform-specific subclasses
(similar to RWHV) for TextInputManager.

[kenrb, 2016-08-08 22:03:05]

https://codereview.chromium.org/2213503002/diff/40001/content/browser/rendere...
File content/browser/renderer_host/text_input_manager.h (right):

https://codereview.chromium.org/2213503002/diff/40001/content/browser/rendere...
content/browser/renderer_host/text_input_manager.h:112: #elif defined(OS_MACOSX)
On 2016/08/04 23:53:02, EhsanK wrote:
> On 2016/08/04 18:44:51, Charlie Reis (OOO til 8-8) wrote:
> > Hmm, this seems unfortunate to need different selection APIs on different
> > platforms.  Ifdefs tend to have a code complexity cost, so I generally try
to
> > avoid them unless they're part of a temporary transition, or a fundamental
> need.
> > 
> > Maybe we're stuck with this, but it's worth thinking whether there's a way
to
> > have a consistent API across platforms here.
> 
> There is more in the .cc file for each specific method.
> 
> The whole state tracking logic can be looked at 1) preprocessing the update
> received from RWH and 2) making decision after that. Both parts were platform
> specific before. The way we did TextInputManager, which might not necessarily
be
> optimal delegates part (2) to the tab's view for the platform specific
> decisions. Part (1) unfortunately has to either be taken care of in RWHVBase,
or
> here.
> 
> I will give this another try tomorrow to see if I can refactor this better.
That
> being said, another alternative would be to have platform specific
> TextInputManager implementations for Aura, Mac, and Android.
> 
> Also having the same class for all platforms, such as SelectionRegion below is
> fine but I guess not idea since we consume more memory.

What if we just store (more or less) the raw contents of
ViewHostMsg_SelectionBounds_Params in this struct, and have the
RenderWidgetHostView calculate the values that it needs? You might need to store
a little bit extra for first_selection_rect on Mac, but having a data member
that isn't used on Aura isn't that bad.

[EhsanK, 2016-08-16 14:00:48]

On 2016/08/08 22:03:05, kenrb wrote:
>
https://codereview.chromium.org/2213503002/diff/40001/content/browser/rendere...
> File content/browser/renderer_host/text_input_manager.h (right):
> 
>
https://codereview.chromium.org/2213503002/diff/40001/content/browser/rendere...
> content/browser/renderer_host/text_input_manager.h:112: #elif
defined(OS_MACOSX)
> On 2016/08/04 23:53:02, EhsanK wrote:
> > On 2016/08/04 18:44:51, Charlie Reis (OOO til 8-8) wrote:
> > > Hmm, this seems unfortunate to need different selection APIs on different
> > > platforms.  Ifdefs tend to have a code complexity cost, so I generally try
> to
> > > avoid them unless they're part of a temporary transition, or a fundamental
> > need.
> > > 
> > > Maybe we're stuck with this, but it's worth thinking whether there's a way
> to
> > > have a consistent API across platforms here.
> > 
> > There is more in the .cc file for each specific method.
> > 
> > The whole state tracking logic can be looked at 1) preprocessing the update
> > received from RWH and 2) making decision after that. Both parts were
platform
> > specific before. The way we did TextInputManager, which might not
necessarily
> be
> > optimal delegates part (2) to the tab's view for the platform specific
> > decisions. Part (1) unfortunately has to either be taken care of in
RWHVBase,
> or
> > here.
> > 
> > I will give this another try tomorrow to see if I can refactor this better.
> That
> > being said, another alternative would be to have platform specific
> > TextInputManager implementations for Aura, Mac, and Android.
> > 
> > Also having the same class for all platforms, such as SelectionRegion below
is
> > fine but I guess not idea since we consume more memory.
> 
> What if we just store (more or less) the raw contents of
> ViewHostMsg_SelectionBounds_Params in this struct, and have the
> RenderWidgetHostView calculate the values that it needs? You might need to
store
> a little bit extra for first_selection_rect on Mac, but having a data member
> that isn't used on Aura isn't that bad.

The problem with storing raw values is that we need to preprocess the values
first to verify if the reported selection bounds are new and 'different' (in
Aura). So I guess we should store than as ui::SelectionBounds in that case.

[EhsanK, 2016-08-17 18:50:37]

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

[commit-bot: I haz the power, 2016-08-17 18:51:00]

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, 2016-08-17 19:04:32]

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

[commit-bot: I haz the power, 2016-08-17 19:04:35]

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

[EhsanK, 2016-08-17 19:23:10]

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

[commit-bot: I haz the power, 2016-08-17 19:23:31]

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

[EhsanK, 2016-08-17 19:23:37]

Patchset #6 (id:100001) has been deleted

[EhsanK, 2016-08-17 19:36:35]

Patchset #7 (id:140001) has been deleted

[EhsanK, 2016-08-17 19:36:44]

Patchset #6 (id:120001) has been deleted

[EhsanK, 2016-08-17 20:26:35]

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

[EhsanK, 2016-08-17 20:26:36]

kenrb@, avi@: Could you please take a look at this (hopefully) the last
Mac-IME-OOPIF CL?

Also adding erikchen@ due to Mac code and his involvement in the last two
similar CLs.

erikchen@: Could you please take a look?

Thanks!

[Avi (use Gerrit), 2016-08-17 20:31:41]

lgtm

[erikchen, 2016-08-17 23:31:16]

https://codereview.chromium.org/2213503002/diff/160001/chrome/browser/rendere...
File chrome/browser/renderer_host/site_per_process_text_input_browsertest.cc
(right):

https://codereview.chromium.org/2213503002/diff/160001/chrome/browser/rendere...
chrome/browser/renderer_host/site_per_process_text_input_browsertest.cc:599:
bounds_observer.Wait();
I know that this is the same test we're using on Aura, but should we be checking
some properties of the new selection bounds, rather than just checking that the
observer got a callback? Ditto for TrackCompositionRangeForAllFrames.

https://codereview.chromium.org/2213503002/diff/160001/content/browser/render...
File content/browser/renderer_host/render_widget_host_view_mac.mm (right):

https://codereview.chromium.org/2213503002/diff/160001/content/browser/render...
content/browser/renderer_host/render_widget_host_view_mac.mm:1248:
->GetSelectionRegion(GetFocusedWidget()->GetView())
This relies on the assumption that GetFocusedWidget() is not null. I'm wiling to
believe this is always true - can you add a DCHECK?

https://codereview.chromium.org/2213503002/diff/160001/content/browser/render...
File content/browser/renderer_host/text_input_manager.cc (right):

https://codereview.chromium.org/2213503002/diff/160001/content/browser/render...
content/browser/renderer_host/text_input_manager.cc:196: if (params.anchor_rect
== params.focus_rect) {
When this condition evaluates to false, why don't we need to update caret_rect?
I'm having trouble understanding the relationship between
params.anchor_rect/params.focus_rect and ....caret_rect and
first_selection_rect. Is there documentation/design doc somewhere I can look
into?

[EhsanK, 2016-08-20 06:13:10]

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

[commit-bot: I haz the power, 2016-08-20 06:13:26]

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, 2016-08-20 07:26:39]

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

[commit-bot: I haz the power, 2016-08-20 07:26:40]

Dry run: This issue passed the CQ dry run.

[EhsanK, 2016-08-22 14:48:29]

Thank you for the reviews!

PTAL.

https://codereview.chromium.org/2213503002/diff/160001/chrome/browser/rendere...
File chrome/browser/renderer_host/site_per_process_text_input_browsertest.cc
(right):

https://codereview.chromium.org/2213503002/diff/160001/chrome/browser/rendere...
chrome/browser/renderer_host/site_per_process_text_input_browsertest.cc:599:
bounds_observer.Wait();
On 2016/08/17 23:31:16, erikchen wrote:
> I know that this is the same test we're using on Aura, but should we be
checking
> some properties of the new selection bounds, rather than just checking that
the
> observer got a callback? Ditto for TrackCompositionRangeForAllFrames.

That is a good question. I think we should have tests which validate the IME
experience we expect in response to the input we are sending to the renderer.
But I think they should stay as separate tests.

IMO, the tests here should only verify that any generated IME state by an
<iframe> is tracked by TextInputManager on the browser side. I think keeping
these specific tests at this level of complexity is better since mixing the
tracking and correctness of the tracked state might make a test too complicated.
But that is only my opinion.

https://codereview.chromium.org/2213503002/diff/160001/content/browser/render...
File content/browser/renderer_host/render_widget_host_view_mac.mm (right):

https://codereview.chromium.org/2213503002/diff/160001/content/browser/render...
content/browser/renderer_host/render_widget_host_view_mac.mm:1248:
->GetSelectionRegion(GetFocusedWidget()->GetView())
On 2016/08/17 23:31:16, erikchen wrote:
> This relies on the assumption that GetFocusedWidget() is not null. I'm wiling
to
> believe this is always true - can you add a DCHECK?

Done. I strongly believe that is the case since the only way to get here, as far
as I can tell, is to have active IME which requires a focused <input> which
indeed means GetFocusedWidget() != nullptr.

https://codereview.chromium.org/2213503002/diff/160001/content/browser/render...
File content/browser/renderer_host/text_input_manager.cc (right):

https://codereview.chromium.org/2213503002/diff/160001/content/browser/render...
content/browser/renderer_host/text_input_manager.cc:196: if (params.anchor_rect
== params.focus_rect) {
On 2016/08/17 23:31:16, erikchen wrote:
> When this condition evaluates to false, why don't we need to update
caret_rect?
> I'm having trouble understanding the relationship between
> params.anchor_rect/params.focus_rect and ....caret_rect and
> first_selection_rect. Is there documentation/design doc somewhere I can look
> into?

Sadly, I don't know of any. But here is what I figured out:
anchor_rect: First point of entry for selection.
focus_rect: the end of selection, i.e., were caret is now.

When the condition is true there is actually no selection (highlighted text)
which is where the caret is. Hence the caret_rect update.

For the most part, first_selection_rect is pretty much equal to caret_rect. The
only exception I found so far is Katakana IME (Japanese). For this IME, we have
a sequence of 'composition clauses' of different length. If during composition
we use the arrow keys to move in between them, the first selection rect will be
different than caret rect (it will be the beginning of the clause).

[erikchen, 2016-08-22 17:30:10]

On 2016/08/22 14:48:29, EhsanK wrote:
> Thank you for the reviews!
> 
> PTAL.
> 
>
https://codereview.chromium.org/2213503002/diff/160001/chrome/browser/rendere...
> File chrome/browser/renderer_host/site_per_process_text_input_browsertest.cc
> (right):
> 
>
https://codereview.chromium.org/2213503002/diff/160001/chrome/browser/rendere...
> chrome/browser/renderer_host/site_per_process_text_input_browsertest.cc:599:
> bounds_observer.Wait();
> On 2016/08/17 23:31:16, erikchen wrote:
> > I know that this is the same test we're using on Aura, but should we be
> checking
> > some properties of the new selection bounds, rather than just checking that
> the
> > observer got a callback? Ditto for TrackCompositionRangeForAllFrames.
> 
> That is a good question. I think we should have tests which validate the IME
> experience we expect in response to the input we are sending to the renderer.
> But I think they should stay as separate tests.
> 
> IMO, the tests here should only verify that any generated IME state by an
> <iframe> is tracked by TextInputManager on the browser side. I think keeping
> these specific tests at this level of complexity is better since mixing the
> tracking and correctness of the tracked state might make a test too
complicated.
> But that is only my opinion.
> 
>
https://codereview.chromium.org/2213503002/diff/160001/content/browser/render...
> File content/browser/renderer_host/render_widget_host_view_mac.mm (right):
> 
>
https://codereview.chromium.org/2213503002/diff/160001/content/browser/render...
> content/browser/renderer_host/render_widget_host_view_mac.mm:1248:
> ->GetSelectionRegion(GetFocusedWidget()->GetView())
> On 2016/08/17 23:31:16, erikchen wrote:
> > This relies on the assumption that GetFocusedWidget() is not null. I'm
wiling
> to
> > believe this is always true - can you add a DCHECK?
> 
> Done. I strongly believe that is the case since the only way to get here, as
far
> as I can tell, is to have active IME which requires a focused <input> which
> indeed means GetFocusedWidget() != nullptr.
> 
>
https://codereview.chromium.org/2213503002/diff/160001/content/browser/render...
> File content/browser/renderer_host/text_input_manager.cc (right):
> 
>
https://codereview.chromium.org/2213503002/diff/160001/content/browser/render...
> content/browser/renderer_host/text_input_manager.cc:196: if
(params.anchor_rect
> == params.focus_rect) {
> On 2016/08/17 23:31:16, erikchen wrote:
> > When this condition evaluates to false, why don't we need to update
> caret_rect?
> > I'm having trouble understanding the relationship between
> > params.anchor_rect/params.focus_rect and ....caret_rect and
> > first_selection_rect. Is there documentation/design doc somewhere I can look
> > into?
> 
> Sadly, I don't know of any. But here is what I figured out:
> anchor_rect: First point of entry for selection.
> focus_rect: the end of selection, i.e., were caret is now.
> 
> When the condition is true there is actually no selection (highlighted text)
> which is where the caret is. Hence the caret_rect update.
> 
> For the most part, first_selection_rect is pretty much equal to caret_rect.
The
> only exception I found so far is Katakana IME (Japanese). For this IME, we
have
> a sequence of 'composition clauses' of different length. If during composition
> we use the arrow keys to move in between them, the first selection rect will
be
> different than caret rect (it will be the beginning of the clause).

lgtm.

[EhsanK, 2016-08-22 17:32:51]

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

[EhsanK, 2016-08-22 17:32:51]

Thanks erikchen@ for the reviews!

sky@: Could you please take a look at this change? creis@ is OOO so I will have
to bother you for the test file's owner's approval. Thanks!
(chrome/browser/renderer_host/site_per_process_text_input_browsertest.cc).

[sky, 2016-08-22 18:14:30]

LGTM

[EhsanK, 2016-08-22 18:16:41]

Thanks for the reviews! FYI, I will CQ after dry-run. Thanks!

[EhsanK, 2016-08-22 18:16:50]

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

[erikchen, 2016-08-22 18:18:35]

On 2016/08/22 18:16:41, EhsanK wrote:
> Thanks for the reviews! FYI, I will CQ after dry-run. Thanks!

fwiw: If you uncheck, and then recheck the CQ button, it will turn your dry-run
into a CQ run (no time lost)

[EhsanK, 2016-08-22 18:19:23]

On 2016/08/22 18:18:35, erikchen wrote:
> On 2016/08/22 18:16:41, EhsanK wrote:
> > Thanks for the reviews! FYI, I will CQ after dry-run. Thanks!
> 
> fwiw: If you uncheck, and then recheck the CQ button, it will turn your
dry-run
> into a CQ run (no time lost)

Thanks for reminding me! I sometimes forget that :)!

[EhsanK, 2016-08-22 18:19:28]

The CQ bit was unchecked by ekaramad@chromium.org

[EhsanK, 2016-08-22 18:19:29]

The CQ bit was checked by ekaramad@chromium.org

[EhsanK, 2016-08-22 18:19:29]

The patchset sent to the CQ was uploaded after l-g-t-m from avi@chromium.org
Link to the patchset: https://codereview.chromium.org/2213503002/#ps220001
(title: "Rebased")

[commit-bot: I haz the power, 2016-08-22 18:33:00]

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-08-22 19:43:59]

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

[commit-bot: I haz the power, 2016-08-22 19:44:01]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Charlie Reis, 2016-08-22 19:55:58]

The CQ bit was checked by creis@chromium.org

[commit-bot: I haz the power, 2016-08-22 19:56:53]

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-08-22 22:05:35]

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

[commit-bot: I haz the power, 2016-08-22 22:05:38]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[nasko, 2016-08-22 22:07:41]

The CQ bit was checked by nasko@chromium.org

[commit-bot: I haz the power, 2016-08-22 22:08:52]

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-08-22 23:10:43]

Description was changed from

==========
Tracking SelectionBounds for all RenderWidgets on the Browser Side (Mac)

This CL uses the TextInputManager code path for tracking selection bounds on Mac
OSX (a follow up to https://codereview.chromium.org/2057803002 for aura).
Other important changes include:
  * Activating the test
SitePerProcessTextInputManagerTest.TrackSelectionBoundsForAllFrames on Mac OSX.
  * Removing all the overrides of SelectionBoundsChanged form Mac and Android.

BUG=578168,602723,602427
==========

to

==========
Tracking SelectionBounds for all RenderWidgets on the Browser Side (Mac)

This CL uses the TextInputManager code path for tracking selection bounds on Mac
OSX (a follow up to https://codereview.chromium.org/2057803002 for aura).
Other important changes include:
  * Activating the test
SitePerProcessTextInputManagerTest.TrackSelectionBoundsForAllFrames on Mac OSX.
  * Removing all the overrides of SelectionBoundsChanged form Mac and Android.

BUG=578168,602723,602427
==========

[commit-bot: I haz the power, 2016-08-22 23:10:45]

Committed patchset #9 (id:220001)

[commit-bot: I haz the power, 2016-08-22 23:12:51]

Description was changed from

==========
Tracking SelectionBounds for all RenderWidgets on the Browser Side (Mac)

This CL uses the TextInputManager code path for tracking selection bounds on Mac
OSX (a follow up to https://codereview.chromium.org/2057803002 for aura).
Other important changes include:
  * Activating the test
SitePerProcessTextInputManagerTest.TrackSelectionBoundsForAllFrames on Mac OSX.
  * Removing all the overrides of SelectionBoundsChanged form Mac and Android.

BUG=578168,602723,602427
==========

to

==========
Tracking SelectionBounds for all RenderWidgets on the Browser Side (Mac)

This CL uses the TextInputManager code path for tracking selection bounds on Mac
OSX (a follow up to https://codereview.chromium.org/2057803002 for aura).
Other important changes include:
  * Activating the test
SitePerProcessTextInputManagerTest.TrackSelectionBoundsForAllFrames on Mac OSX.
  * Removing all the overrides of SelectionBoundsChanged form Mac and Android.

BUG=578168,602723,602427

Committed: https://crrev.com/2f52009f42519eac3e40ff20718e31365c976773
Cr-Commit-Position: refs/heads/master@{#413568}
==========

[commit-bot: I haz the power, 2016-08-22 23:12:53]

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