Fix a bug in TextInputManager::GetCompositionRangeInfo

Fix a bug in TextInputManager::GetCompositionRangeInfo

After the CL: https://codereview.chromium.org/2235283003 which implemented composition
range information tracking on the browser side for Mac, the method TIM::GetCompositionRangeInfo
was changed to take the RenderWidgetHostView for which the information is required. However,
the view pointer is not actually used and the method returns the stored information for |active_view_|
instead.

BUG=NONE

Review-Url: https://codereview.chromium.org/2814443003
Cr-Commit-Position: refs/heads/master@{#463391}
Committed: https://chromium.googlesource.com/chromium/src/+/79819af0cb936831671dd43047159d472f9a05ed

[EhsanK, 2017-04-10 14:50:28]

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

[EhsanK, 2017-04-10 14:50:29]

Hello Charlie,

Could you please review this CL? There is a question to answer as well. Thanks!

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

https://codereview.chromium.org/2814443003/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/text_input_manager.cc:80: return active_view_ ?
&composition_range_info_map_.at(active_view_) : nullptr;
Right now we do not actually even use this method for a non-null
|view| (all the calls to the method use the default nullptr value for view).
That being said I am working on moving IME tracking to make use of focused frame
(instead of purely relying on TextInputState) and I guess I will be using this
method on focused view. That said, I can either remove |view| as an argument
here now and maybe add it back later, or just keep it as is for now. WDYT?

[Charlie Reis, 2017-04-10 18:31:21]

Thanks!  LGTM with the note below.

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

https://codereview.chromium.org/2814443003/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/text_input_manager.cc:80: return active_view_ ?
&composition_range_info_map_.at(active_view_) : nullptr;
On 2017/04/10 14:50:29, EhsanK wrote:
> Right now we do not actually even use this method for a non-null
> |view| (all the calls to the method use the default nullptr value for view).
> That being said I am working on moving IME tracking to make use of focused
frame
> (instead of purely relying on TextInputState) and I guess I will be using this
> method on focused view. That said, I can either remove |view| as an argument
> here now and maybe add it back later, or just keep it as is for now. WDYT?

Let's go ahead and remove it for now, since it's easy to add back if you need it
later.

[EhsanK, 2017-04-10 18:36:31]

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

[EhsanK, 2017-04-10 18:37:11]

Thanks Charlie! The argument is removed in the latest patch. I will CQ soon.

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

https://codereview.chromium.org/2814443003/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/text_input_manager.cc:80: return active_view_ ?
&composition_range_info_map_.at(active_view_) : nullptr;
On 2017/04/10 18:31:21, Charlie Reis wrote:
> On 2017/04/10 14:50:29, EhsanK wrote:
> > Right now we do not actually even use this method for a non-null
> > |view| (all the calls to the method use the default nullptr value for view).
> > That being said I am working on moving IME tracking to make use of focused
> frame
> > (instead of purely relying on TextInputState) and I guess I will be using
this
> > method on focused view. That said, I can either remove |view| as an argument
> > here now and maybe add it back later, or just keep it as is for now. WDYT?
> 
> Let's go ahead and remove it for now, since it's easy to add back if you need
it
> later.

Acknowledged.

[commit-bot: I haz the power, 2017-04-10 18:37:29]

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-04-10 19:40:47]

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

[commit-bot: I haz the power, 2017-04-10 19:40:48]

Dry run: This issue passed the CQ dry run.

[Charlie Reis, 2017-04-10 19:55:37]

Thanks!  LGTM.

[EhsanK, 2017-04-10 20:53:55]

The CQ bit was checked by ekaramad@chromium.org

[commit-bot: I haz the power, 2017-04-10 20:54:46]

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-04-10 21:10:34]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1491857635542010,
"parent_rev": "460ffe75e23987ac6760d668963d9d2f1a83962b", "commit_rev":
"79819af0cb936831671dd43047159d472f9a05ed"}

[commit-bot: I haz the power, 2017-04-10 21:11:33]

Description was changed from

==========
Fix a bug in TextInputManager::GetCompositionRangeInfo

After the CL: https://codereview.chromium.org/2235283003 which implemented
composition
range information tracking on the browser side for Mac, the method
TIM::GetCompositionRangeInfo
was changed to take the RenderWidgetHostView for which the information is
required. However,
the view pointer is not actually used and the method returns the stored
information for |active_view_|
instead.

BUG=NONE
==========

to

==========
Fix a bug in TextInputManager::GetCompositionRangeInfo

After the CL: https://codereview.chromium.org/2235283003 which implemented
composition
range information tracking on the browser side for Mac, the method
TIM::GetCompositionRangeInfo
was changed to take the RenderWidgetHostView for which the information is
required. However,
the view pointer is not actually used and the method returns the stored
information for |active_view_|
instead.

BUG=NONE

Review-Url: https://codereview.chromium.org/2814443003
Cr-Commit-Position: refs/heads/master@{#463391}
Committed:
https://chromium.googlesource.com/chromium/src/+/79819af0cb936831671dd4304715...
==========

[commit-bot: I haz the power, 2017-04-10 21:11:37]

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