Fixed an IME bug due to a race in TextInputStateChanged

Fixed an IME bug due to a race in TextInputStateChanged

BrowserPlugin-based guests are hidden from the frame tree and embedder
RenderWidgetHostViews. Any IME update, including TextInputState goes to
embedder's RWHV with the embedder RWHV not knowing it is actually from a
separate process.

This is prone to bugs and race conditions. For example, leaving an <input>
inside embedder to get into another one inside <guest> might lead to
ui::TEXT_INPUT_TYPE_TEXT reported by the guest process overwritten by a
ui::TEXT_INPUT_TYPE_NONE from the embedder process.

This CL will stop RenderWidgetHostImpl fixes this race on the browser side by
avoiding sending the update up from RenderWidgetHostImpl to
RenderWidgetHostViewBase when there is a focused guest with a focused <input>
on the page.

BUG=697045

[EhsanK, 2017-03-15 17:38:28]

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

[commit-bot: I haz the power, 2017-03-15 17:39:14]

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-03-15 18:33:31]

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

[commit-bot: I haz the power, 2017-03-15 18:33:31]

Dry run: This issue passed the CQ dry run.

[EhsanK, 2017-03-15 20:36:08]

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

[EhsanK, 2017-03-15 20:36:43]

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

[EhsanK, 2017-03-15 20:36:43]

Hello Charlie,

Could you please review this, or if busy forward to someone who could? Thanks!

[commit-bot: I haz the power, 2017-03-15 20:36:57]

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-03-15 21:58:02]

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

[commit-bot: I haz the power, 2017-03-15 21:58:03]

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

[Charlie Reis, 2017-03-15 23:48:36]

On 2017/03/15 20:36:43, EhsanK wrote:
> Hello Charlie,
> 
> Could you please review this, or if busy forward to someone who could? Thanks!

Hmm, this is a bit too subtle for me, given my very limited understanding of
IME.  Is there anyone else who knows IME that could do a first pass before I
provide an owner's stamp?

I'd also recommend including a test, since we seem to know what the steps are
from comment 11 on the bug.

[EhsanK, 2017-03-16 14:50:31]

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

[EhsanK, 2017-03-16 14:50:32]

Thanks Charlie!

I will be working on a test, perhaps a <webview> IME test which can stay for
OOPIF-webview as well.

Meanwhile adding changwan@ as an IME reviewer who was also cc-ed on the bug.

changwan@: PTAL.

[Changwan Ryu, 2017-03-16 16:51:48]

I'm not too familiar with TextInputManager and desktop code flows, so these
might be completely out of context, but could you check if the following case
can happen?

There are multiple BrowserPluginGuests. One gets the focus first
but the other gets initialized as blurred. For example, 
BrowserPluginGuest::Init() called with params.focused=false. Then
set_focused_guest(false) will be called. Is it possible for BrowserPluginGuest
to create another unfocused BrowserPluginGuest when focused?

[EhsanK, 2017-03-23 21:17:03]

Patchset #3 (id:40001) has been deleted

[EhsanK, 2017-03-23 21:18:48]

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

[commit-bot: I haz the power, 2017-03-23 21:19:33]

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-03-23 22:15:26]

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

[commit-bot: I haz the power, 2017-03-23 22:15:27]

Dry run: This issue passed the CQ dry run.

[EhsanK, 2017-03-24 14:40:00]

On 2017/03/16 16:51:48, Changwan Ryu wrote:
> I'm not too familiar with TextInputManager and desktop code flows, so these
> might be completely out of context, but could you check if the following case
> can happen?
> 
> There are multiple BrowserPluginGuests. One gets the focus first
> but the other gets initialized as blurred. For example, 
> BrowserPluginGuest::Init() called with params.focused=false. Then
> set_focused_guest(false) will be called. Is it possible for BrowserPluginGuest
> to create another unfocused BrowserPluginGuest when focused?

Thanks! This is quite valid. Actually, it does not need be during attach but
simply having nested BrowserPlugins complicates things this way. For instance if
a PDF embedded inside <webview> loses focus we would reset this flag.

I actually ended up using BrowserPluginEmbedder class to search for any focused
guest (BrowserPlugin-based). I am now struggling with finding a proper test
case.