Synthesize proper keyIdentifier for devtools protocol key events.

Synthesize proper keyIdentifier for devtools protocol key events.

When bug 472077 moved devtools protocol key handling to browser one part
of code that sythesized keyIdentifier was lost (it was prevoiusly
handled in InspectorInputClient::dispatchKeyEvent and removed in
https://codereview.chromium.org/1063653002/patch/40001/50004). Restore
that functionality else sending some key events (like enter) is not
handled correctly.

BUG=

Committed: https://crrev.com/387858480cc7aec410fe8c776ccc15abfaf873ae
Cr-Commit-Position: refs/heads/master@{#331546}

[mharanczyk, 2015-05-20 10:34:04]

mharanczyk@opera.com changed reviewers:
+ dgozman@chromium.org, pfeldman@chromium.org

[mharanczyk, 2015-05-20 10:34:40]

PTAL.

[dgozman, 2015-05-21 21:24:17]

Thank you for the fix! How did you encounter the problem? Perhaps, we can add a
layout test to inspector-protocol/input?

lgtm

[mharanczyk, 2015-05-25 07:34:53]

On 2015/05/21 21:24:17, dgozman_ooo wrote:
> Thank you for the fix! How did you encounter the problem? Perhaps, we can add
a
> layout test to inspector-protocol/input?
> 
> lgtm

I noticed webdriver TC not working correctly when sending "enter" key after the
change and it previously worked correctly. In new code event was properly sent
to renderer but then ignored/nothing happened. I will try to write a test for
that.

[mharanczyk, 2015-05-25 13:56:24]

I've added browser test for devtools protocol, could you please take a look at
it?

[dgozman, 2015-05-25 16:02:10]

lgtm

https://codereview.chromium.org/1148803002/diff/60001/content/browser/devtool...
File content/browser/devtools/protocol/devtools_protocol_browsertest.cc (right):

https://codereview.chromium.org/1148803002/diff/60001/content/browser/devtool...
content/browser/devtools/protocol/devtools_protocol_browsertest.cc:100: if
(base::MessageLoop::current()->is_running())
I probably don't understand something, but how current runloop may be not
running?

[mharanczyk, 2015-05-26 07:45:04]

https://codereview.chromium.org/1148803002/diff/60001/content/browser/devtool...
File content/browser/devtools/protocol/devtools_protocol_browsertest.cc (right):

https://codereview.chromium.org/1148803002/diff/60001/content/browser/devtool...
content/browser/devtools/protocol/devtools_protocol_browsertest.cc:100: if
(base::MessageLoop::current()->is_running())
On 2015/05/25 16:02:10, dgozman_ooo wrote:
> I probably don't understand something, but how current runloop may be not
> running?

Those tests are executed from main thread startup from
ShellBrowserMainParts::PreMainMessageLoopRun where message loop context is
initialized but is not run unless test startup
(ContentBrowserTest::RunTestOnMainThreadLoop calls
base::MessageLoopForUI::current()->RunUntilIdle() which starts and quits run
loop when startup tasks are done) or test itself decides to run it. In case of
DevToolsProtocolTest::SendCommand we start run loop only if message was not
handled synchronously, so for synchronous callback to
DevToolsProtocolTest::DispatchProtocolMessage run loop is not running and
calling QuitNow causes assertion.

[mharanczyk, 2015-05-26 07:58:01]

The CQ bit was checked by mharanczyk@opera.com

[mharanczyk, 2015-05-26 07:58:02]

The patchset sent to the CQ was uploaded after l-g-t-m from dgozman@chromium.org
Link to the patchset: https://codereview.chromium.org/1148803002/#ps80001
(title: "Minor style fix: keyIdentifier -> key_identifier")

[commit-bot: I haz the power, 2015-05-26 07:58:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1148803002/80001

[mharanczyk, 2015-05-26 09:10:21]

The CQ bit was unchecked by mharanczyk@opera.com

[mharanczyk, 2015-05-26 09:26:42]

The CQ bit was checked by mharanczyk@opera.com to run a CQ dry run

[mharanczyk, 2015-05-26 09:26:42]

The patchset sent to the CQ was uploaded after l-g-t-m from dgozman@chromium.org
Link to the patchset: https://codereview.chromium.org/1148803002/#ps100001
(title: "Do all the queued work when message is dispatched synchronously in
test.")

[commit-bot: I haz the power, 2015-05-26 09:26:58]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1148803002/100001

[commit-bot: I haz the power, 2015-05-26 12:28:26]

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

[commit-bot: I haz the power, 2015-05-26 12:28:28]

Dry run: This issue passed the CQ dry run.

[mharanczyk, 2015-05-26 13:58:30]

The CQ bit was checked by mharanczyk@opera.com to run a CQ dry run

[mharanczyk, 2015-05-26 13:58:30]

The patchset sent to the CQ was uploaded after l-g-t-m from dgozman@chromium.org
Link to the patchset: https://codereview.chromium.org/1148803002/#ps120001
(title: "Attempt to fix test flakiness.")

[commit-bot: I haz the power, 2015-05-26 13:58:39]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1148803002/120001

[mharanczyk, 2015-05-26 15:33:48]

Sorry for commit-bot message spam, I had to root out test flakiness which I
could not reproduce locally. Instead of probing renderer state after key events
are dispatched, send events back to test when keys are handled in renderer. This
prevents race condition when probing could be done before key events were
handled in renderer.

I've checked test results (at least ones which I had access to) to see if this
test randomly fails, and it looks like flakiness is fixed. Would you be so kind
to check some logs I had not access to (for example {win, mac, linux} chromium
rel ng) if this test passed without any problems and take a look at code again.
If everything is fine from your point of view feel free to commit this CL. Red
win_chromium_rel_ng build does not seem to be related to this change.

[dgozman, 2015-05-26 16:43:53]

On 2015/05/26 15:33:48, mharanczyk wrote:
> Sorry for commit-bot message spam, I had to root out test flakiness which I
> could not reproduce locally. Instead of probing renderer state after key
events
> are dispatched, send events back to test when keys are handled in renderer.
This
> prevents race condition when probing could be done before key events were
> handled in renderer.
> 
> I've checked test results (at least ones which I had access to) to see if this
> test randomly fails, and it looks like flakiness is fixed. Would you be so
kind
> to check some logs I had not access to (for example {win, mac, linux} chromium
> rel ng) if this test passed without any problems and take a look at code
again.
> If everything is fine from your point of view feel free to commit this CL. Red
> win_chromium_rel_ng build does not seem to be related to this change.

Patchset 7 has no suspicious failures and lgtm.

[commit-bot: I haz the power, 2015-05-26 19:21:08]

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

[commit-bot: I haz the power, 2015-05-26 19:21:10]

Dry run: This issue passed the CQ dry run.

[mharanczyk, 2015-05-27 07:49:08]

The CQ bit was checked by mharanczyk@opera.com

[commit-bot: I haz the power, 2015-05-27 07:49:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1148803002/120001

[commit-bot: I haz the power, 2015-05-27 07:53:58]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-05-27 07:54:54]

Patchset 7 (id:??) landed as
https://crrev.com/387858480cc7aec410fe8c776ccc15abfaf873ae
Cr-Commit-Position: refs/heads/master@{#331546}