Change |EnsureCaretInRect| to |EnsureCaretNotInRect|.

Change |EnsureCaretInRect| to |EnsureCaretNotInRect|.

BUG=624521
TEST=n/a; no behavior change.

Committed: https://crrev.com/0ddd346fec6fc94531234f50964269069a29a708
Cr-Commit-Position: refs/heads/master@{#437185}

[yhanada, 2016-12-05 12:16:09]

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

[commit-bot: I haz the power, 2016-12-05 12:16:25]

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

[yhanada, 2016-12-05 12:57:15]

yhanada@chromium.org changed reviewers:
+ oshima@chromium.org, sadrul@chromium.org, sky@chromium.org

[yhanada, 2016-12-05 12:57:16]

This is a preparation for https://crrev.com/2553603002. PTAL. Thank you!

[yhanada1, 2016-12-06 05:15:23]

The CQ bit was checked by yhanada@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-12-06 05:15:38]

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

[oshima, 2016-12-06 06:26:18]

change lg. Can you add a test with multiple displays case?

[commit-bot: I haz the power, 2016-12-06 06:43:37]

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

[commit-bot: I haz the power, 2016-12-06 06:43:38]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[yhanada, 2016-12-06 07:37:48]

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

[yhanada, 2016-12-06 07:37:55]

On 2016/12/06 06:26:18, oshima wrote:
> change lg. Can you add a test with multiple displays case?

Done.

[commit-bot: I haz the power, 2016-12-06 07:38:01]

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-12-06 09:16:59]

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

[commit-bot: I haz the power, 2016-12-06 09:17:00]

Dry run: This issue passed the CQ dry run.

[oshima, 2016-12-06 14:02:26]

lgtm with nits

https://codereview.chromium.org/2554513002/diff/40001/ash/root_window_control...
File ash/root_window_controller_unittest.cc (right):

https://codereview.chromium.org/2554513002/diff/40001/ash/root_window_control...
ash/root_window_controller_unittest.cc:1133: ui->EnsureCaretInWorkArea();
we may want to remove this as well for consistency. I'll leave this to owner
though.

https://codereview.chromium.org/2554513002/diff/40001/ash/root_window_control...
ash/root_window_controller_unittest.cc:1135:
text_input_client.hidden_rect().width());
and contains

https://codereview.chromium.org/2554513002/diff/40001/ash/root_window_control...
ash/root_window_controller_unittest.cc:1138: .IsEmpty());
!contains?

https://codereview.chromium.org/2554513002/diff/40001/ash/root_window_control...
ash/root_window_controller_unittest.cc:1152: .IsEmpty());
ditto

[sky, 2016-12-06 17:32:14]

https://codereview.chromium.org/2554513002/diff/40001/ui/base/ime/text_input_...
File ui/base/ime/text_input_client.h (right):

https://codereview.chromium.org/2554513002/diff/40001/ui/base/ime/text_input_...
ui/base/ime/text_input_client.h:171: virtual void EnsureCaretOutOfRect(const
gfx::Rect& rect) = 0;
What does 'out of' rect mean in this context? Maybe you mean
EnsureCaretNotInRect() ?

[yhanada, 2016-12-07 03:25:52]

https://codereview.chromium.org/2554513002/diff/40001/ash/root_window_control...
File ash/root_window_controller_unittest.cc (right):

https://codereview.chromium.org/2554513002/diff/40001/ash/root_window_control...
ash/root_window_controller_unittest.cc:1133: ui->EnsureCaretInWorkArea();
On 2016/12/06 14:02:25, oshima wrote:
> we may want to remove this as well for consistency. I'll leave this to owner
> though.

Done.

https://codereview.chromium.org/2554513002/diff/40001/ash/root_window_control...
ash/root_window_controller_unittest.cc:1135:
text_input_client.hidden_rect().width());
On 2016/12/06 14:02:25, oshima wrote:
> and contains

Done.

https://codereview.chromium.org/2554513002/diff/40001/ash/root_window_control...
ash/root_window_controller_unittest.cc:1138: .IsEmpty());
On 2016/12/06 14:02:25, oshima wrote:
> !contains?

Done.

https://codereview.chromium.org/2554513002/diff/40001/ash/root_window_control...
ash/root_window_controller_unittest.cc:1152: .IsEmpty());
On 2016/12/06 14:02:25, oshima wrote:
> ditto

Done.

https://codereview.chromium.org/2554513002/diff/40001/ui/base/ime/text_input_...
File ui/base/ime/text_input_client.h (right):

https://codereview.chromium.org/2554513002/diff/40001/ui/base/ime/text_input_...
ui/base/ime/text_input_client.h:171: virtual void EnsureCaretOutOfRect(const
gfx::Rect& rect) = 0;
On 2016/12/06 17:32:14, sky wrote:
> What does 'out of' rect mean in this context? Maybe you mean
> EnsureCaretNotInRect() ?

Done.

[yhanada, 2016-12-07 03:26:00]

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

[commit-bot: I haz the power, 2016-12-07 03:26:36]

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-12-07 04:28:24]

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

[commit-bot: I haz the power, 2016-12-07 04:28:25]

Dry run: This issue passed the CQ dry run.

[sky, 2016-12-07 05:06:21]

LGTM

https://codereview.chromium.org/2554513002/diff/80001/ash/root_window_control...
File ash/root_window_controller_unittest.cc (right):

https://codereview.chromium.org/2554513002/diff/80001/ash/root_window_control...
ash/root_window_controller_unittest.cc:727: hidden_rect_ = rect;
hidden_rect_ doesn't really relate to the name of this function. Maybe
caret_exclude_rect?

https://codereview.chromium.org/2554513002/diff/80001/ash/root_window_control...
ash/root_window_controller_unittest.cc:1105: aura::Window* primary_root_window =
Shell::GetPrimaryRootWindow();
Why don't you set primary to root_windows[0] and secondary to root_windows[1]?
You could assert the size is 2.

https://codereview.chromium.org/2554513002/diff/80001/ash/root_window_control...
ash/root_window_controller_unittest.cc:1141: keyboard_container->Show();
Is this really necessary? Isn't this the same as line 1126?

[yhanada, 2016-12-07 06:15:25]

Thank you for reviewing.

https://codereview.chromium.org/2554513002/diff/80001/ash/root_window_control...
File ash/root_window_controller_unittest.cc (right):

https://codereview.chromium.org/2554513002/diff/80001/ash/root_window_control...
ash/root_window_controller_unittest.cc:727: hidden_rect_ = rect;
On 2016/12/07 05:06:20, sky wrote:
> hidden_rect_ doesn't really relate to the name of this function. Maybe
> caret_exclude_rect?

Done.

https://codereview.chromium.org/2554513002/diff/80001/ash/root_window_control...
ash/root_window_controller_unittest.cc:1105: aura::Window* primary_root_window =
Shell::GetPrimaryRootWindow();
On 2016/12/07 05:06:21, sky wrote:
> Why don't you set primary to root_windows[0] and secondary to root_windows[1]?
> You could assert the size is 2.

Done.

https://codereview.chromium.org/2554513002/diff/80001/ash/root_window_control...
ash/root_window_controller_unittest.cc:1141: keyboard_container->Show();
On 2016/12/07 05:06:21, sky wrote:
> Is this really necessary? Isn't this the same as line 1126?

Done.

[yhanada, 2016-12-07 06:19:24]

yhanada@chromium.org changed reviewers:
+ hidehiko@chromium.org
- yhanada@google.com

[yhanada, 2016-12-07 06:19:24]

Hi hidehiko@, could you take a look at arc_ime_service.h as an OWNER?

[sadrul, 2016-12-08 00:57:29]

lgtm

https://codereview.chromium.org/2554513002/diff/100001/content/browser/render...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://codereview.chromium.org/2554513002/diff/100001/content/browser/render...
content/browser/renderer_host/render_widget_host_view_aura.cc:1436:
hiding_area_in_this_window)));
If you change |rect| from screen-space to local-space at the beginning of this
function, then you don't need to use GetBoundsInScreen(), or convert it back to
local-space here again using ConvertRectFromScreen(). I think doing that would
make this simpler.

[hidehiko, 2016-12-08 02:38:08]

components/arc LGTM.

Could you change the subject and description comment to reflect your update
(EnsureCaretOutOfRect -> EnsureCaretNotInRect)?

[yhanada1, 2016-12-08 03:20:34]

Description was changed from

==========
Change |EnsureCaretInRect| to |EnsureCaretOutOfRect|.

BUG=624521
TEST=n/a; no behavior change.
==========

to

==========
Change |EnsureCaretInRect| to |EnsureCaretNotInRect|.

BUG=624521
TEST=n/a; no behavior change.
==========

[yhanada, 2016-12-08 03:31:37]

Thank you very much for reviewing!

https://codereview.chromium.org/2554513002/diff/100001/content/browser/render...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://codereview.chromium.org/2554513002/diff/100001/content/browser/render...
content/browser/renderer_host/render_widget_host_view_aura.cc:1436:
hiding_area_in_this_window)));
On 2016/12/08 00:57:29, sadrul wrote:
> If you change |rect| from screen-space to local-space at the beginning of this
> function, then you don't need to use GetBoundsInScreen(), or convert it back
to
> local-space here again using ConvertRectFromScreen(). I think doing that would
> make this simpler.

Done.

[yhanada, 2016-12-08 03:31:43]

The CQ bit was checked by yhanada@chromium.org

[yhanada, 2016-12-08 03:31:43]

The patchset sent to the CQ was uploaded after l-g-t-m from oshima@chromium.org,
sky@chromium.org, hidehiko@chromium.org, sadrul@chromium.org
Link to the patchset: https://codereview.chromium.org/2554513002/#ps120001
(title: "rebase")

[commit-bot: I haz the power, 2016-12-08 03:32:20]

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

[sadrul, 2016-12-08 03:34:45]

https://codereview.chromium.org/2554513002/diff/120001/content/browser/render...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://codereview.chromium.org/2554513002/diff/120001/content/browser/render...
content/browser/renderer_host/render_widget_host_view_aura.cc:1429:
gfx::IntersectRects(rect, window_->GetBoundsInScreen())));
I was thinking this would be:
  gfx::Rect rect_in_local_space = ConvertRectFromScreen(rect);
  gfx::Rect hiding_area_in_this_window =
      gfx::IntersectRects(rect_in_local_space, window_->bounds());
  ...

[yhanada1, 2016-12-08 03:36:46]

The CQ bit was unchecked by yhanada@google.com

[yhanada, 2016-12-08 03:47:49]

https://codereview.chromium.org/2554513002/diff/120001/content/browser/render...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://codereview.chromium.org/2554513002/diff/120001/content/browser/render...
content/browser/renderer_host/render_widget_host_view_aura.cc:1429:
gfx::IntersectRects(rect, window_->GetBoundsInScreen())));
On 2016/12/08 03:34:45, sadrul wrote:
> I was thinking this would be:
>   gfx::Rect rect_in_local_space = ConvertRectFromScreen(rect);
>   gfx::Rect hiding_area_in_this_window =
>       gfx::IntersectRects(rect_in_local_space, window_->bounds());
>   ...

Sorry, I misunderstood what you meant. Done.

[yhanada, 2016-12-08 03:48:21]

The CQ bit was checked by yhanada@chromium.org

[yhanada, 2016-12-08 03:48:21]

The patchset sent to the CQ was uploaded after l-g-t-m from
hidehiko@chromium.org, sadrul@chromium.org, oshima@chromium.org,
sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2554513002/#ps160001
(title: "")

[commit-bot: I haz the power, 2016-12-08 03:48:57]

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

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1481168901063650,
"parent_rev": "a1f746fecad94f2dff889abf6c54f0b3342924bb", "commit_rev":
"e4466f19ae0f1bffd420d9f41954dc648092d679"}

[commit-bot: I haz the power, 2016-12-08 05:04:46]

Description was changed from

==========
Change |EnsureCaretInRect| to |EnsureCaretNotInRect|.

BUG=624521
TEST=n/a; no behavior change.
==========

to

==========
Change |EnsureCaretInRect| to |EnsureCaretNotInRect|.

BUG=624521
TEST=n/a; no behavior change.
==========

[commit-bot: I haz the power, 2016-12-08 05:04:47]

Committed patchset #9 (id:160001)

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

Description was changed from

==========
Change |EnsureCaretInRect| to |EnsureCaretNotInRect|.

BUG=624521
TEST=n/a; no behavior change.
==========

to

==========
Change |EnsureCaretInRect| to |EnsureCaretNotInRect|.

BUG=624521
TEST=n/a; no behavior change.

Committed: https://crrev.com/0ddd346fec6fc94531234f50964269069a29a708
Cr-Commit-Position: refs/heads/master@{#437185}
==========

[commit-bot: I haz the power, 2016-12-08 05:08:09]

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