Implement input.ime.sendKeyEvents API.

Implement input.ime.sendKeyEvents API.

This cl implements input.ime.senKeyEvents API on Linux & Windows platform. The API calls the method InputMethodEngineBase::SendKeyEvents(), which contains the common code for ChromeOS/NonChromeOS. And the platform-specific codes are written in InputMethodEngine::SendKeyEvent().

On ChromeOS, the key event is dispatched from directly calling EventProcessor::OnEventFromSource(). While, On Linux & Windows, the key event is dispatched through ImeInputContextHandlerInterface::SendKeyEvent(), which will be implemented by InputMethodXxx.

This implementation is similar to input.ime.setComposition and input.ime.commitText APIs.

Add browser test in c/t/d/e/api_test/input_ime_nonchromeos/*.

BUG=517773
TEST=BrowserTest --gtest_filter=InputImeApiTest.*

Committed: https://crrev.com/b293444e8dbe567102fb82ab8831b4e7ac23a59a
Cr-Commit-Position: refs/heads/master@{#381175}

[Azure Wei, 2016-03-08 01:53:51]

Description was changed from

==========
Implement input.ime.sendKeyEvents API.

BUG=517773
TEST=BrowserTest --gtest_filter=InputImeApiNonChromeOSTest.TestKeyEvents
==========

to

==========
Implement input.ime.sendKeyEvents API.

This cl implements input.ime.senKeyEvents API on Linux & Windows platform. The
API calls the method InputMethodEngineBase::SendKeyEvents(), which contains the
common code for ChromeOS/NonChromeOS. And the platform-specific codes are
written in InputMethodEngine::SendKeyEvent().

Add browser test in c/t/d/e/api_test/input_ime_nonchromeos/key_event/*.
And move input.ime.createWindow test from
c/t/d/e/api_test/input_ime_nonchromeos/* to
c/t/d/e/api_test/input_ime_nonchromeos/create_window/*

BUG=517773
TEST=BrowserTest --gtest_filter=InputImeApiNonChromeOSTest.TestKeyEvents
==========

[Azure Wei, 2016-03-08 01:53:52]

azurewei@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org, shuchen@chromium.org

[Azure Wei, 2016-03-08 01:55:14]

Shu and Devlin, can you please help review this cl? Thank you!

[Azure Wei, 2016-03-08 03:06:04]

Description was changed from

==========
Implement input.ime.sendKeyEvents API.

This cl implements input.ime.senKeyEvents API on Linux & Windows platform. The
API calls the method InputMethodEngineBase::SendKeyEvents(), which contains the
common code for ChromeOS/NonChromeOS. And the platform-specific codes are
written in InputMethodEngine::SendKeyEvent().

Add browser test in c/t/d/e/api_test/input_ime_nonchromeos/key_event/*.
And move input.ime.createWindow test from
c/t/d/e/api_test/input_ime_nonchromeos/* to
c/t/d/e/api_test/input_ime_nonchromeos/create_window/*

BUG=517773
TEST=BrowserTest --gtest_filter=InputImeApiNonChromeOSTest.TestKeyEvents
==========

to

==========
Implement input.ime.sendKeyEvents API.

This cl implements input.ime.senKeyEvents API on Linux & Windows platform. The
API calls the method InputMethodEngineBase::SendKeyEvents(), which contains the
common code for ChromeOS/NonChromeOS. And the platform-specific codes are
written in InputMethodEngine::SendKeyEvent().

Add browser test in c/t/d/e/api_test/input_ime_nonchromeos/*.

BUG=517773
TEST=BrowserTest --gtest_filter=InputImeApiTest.*
==========

[Shu Chen, 2016-03-08 05:59:44]

https://codereview.chromium.org/1771173002/diff/20001/chrome/browser/extensio...
File chrome/browser/extensions/api/input_ime/input_ime_api.cc (right):

https://codereview.chromium.org/1771173002/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/api/input_ime/input_ime_api.cc:367:
engine->SendKeyEvents(params.context_id, key_data_out);
Better to expose the errors.

[Azure Wei, 2016-03-08 11:42:29]

https://codereview.chromium.org/1771173002/diff/20001/chrome/browser/extensio...
File chrome/browser/extensions/api/input_ime/input_ime_api.cc (right):

https://codereview.chromium.org/1771173002/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/api/input_ime/input_ime_api.cc:367:
engine->SendKeyEvents(params.context_id, key_data_out);
On 2016/03/08 05:59:44, Shu Chen wrote:
> Better to expose the errors.

Done.

[Shu Chen, 2016-03-08 13:04:31]

lgtm

[Shu Chen, 2016-03-08 14:38:04]

not lgtm.

https://codereview.chromium.org/1771173002/diff/40001/chrome/browser/ui/input...
File chrome/browser/ui/input_method/input_method_engine.cc (right):

https://codereview.chromium.org/1771173002/diff/40001/chrome/browser/ui/input...
chrome/browser/ui/input_method/input_method_engine.cc:180: #if defined(OS_WIN)
I think we should NOT hack for OS_WIN here.
Instead, we should fix InputMethodWin::DispatchFabricatedKeyEvent() method.
We can discuss offline about it.

[Shu Chen, 2016-03-09 04:00:24]

https://codereview.chromium.org/1771173002/diff/40001/chrome/browser/ui/input...
File chrome/browser/ui/input_method/input_method_engine.cc (right):

https://codereview.chromium.org/1771173002/diff/40001/chrome/browser/ui/input...
chrome/browser/ui/input_method/input_method_engine.cc:180: #if defined(OS_WIN)
On 2016/03/08 14:38:03, Shu Chen wrote:
> I think we should NOT hack for OS_WIN here.
> Instead, we should fix InputMethodWin::DispatchFabricatedKeyEvent() method.
> We can discuss offline about it.

Per offline discussion, please add SendKeyEvents() method in
IMEInputContextHandlerInterface.
Therefore, there is no need to get the input method here.

[Azure Wei, 2016-03-09 05:00:54]

Description was changed from

==========
Implement input.ime.sendKeyEvents API.

This cl implements input.ime.senKeyEvents API on Linux & Windows platform. The
API calls the method InputMethodEngineBase::SendKeyEvents(), which contains the
common code for ChromeOS/NonChromeOS. And the platform-specific codes are
written in InputMethodEngine::SendKeyEvent().

Add browser test in c/t/d/e/api_test/input_ime_nonchromeos/*.

BUG=517773
TEST=BrowserTest --gtest_filter=InputImeApiTest.*
==========

to

==========
Implement input.ime.sendKeyEvents API.

This cl implements input.ime.senKeyEvents API on Linux & Windows platform. The
API calls the method InputMethodEngineBase::SendKeyEvents(), which contains the
common code for ChromeOS/NonChromeOS. And the platform-specific codes are
written in InputMethodEngine::SendKeyEvent().

On ChromeOS, the key event is dispatched from directly calling
EventProcessor::OnEventFromSource(). While, On Linux & Windows, the key event is
dispatched through ImeInputContextHandlerInterface::SendKeyEvent(), which will
be implemented by InputMethodXxx. 

This implementation is similar to input.ime.setComposition and
input.ime.commitText APIs.

Add browser test in c/t/d/e/api_test/input_ime_nonchromeos/*.

BUG=517773
TEST=BrowserTest --gtest_filter=InputImeApiTest.*
==========

[Azure Wei, 2016-03-09 05:01:57]

https://codereview.chromium.org/1771173002/diff/40001/chrome/browser/ui/input...
File chrome/browser/ui/input_method/input_method_engine.cc (right):

https://codereview.chromium.org/1771173002/diff/40001/chrome/browser/ui/input...
chrome/browser/ui/input_method/input_method_engine.cc:180: #if defined(OS_WIN)
On 2016/03/09 04:00:24, Shu Chen wrote:
> On 2016/03/08 14:38:03, Shu Chen wrote:
> > I think we should NOT hack for OS_WIN here.
> > Instead, we should fix InputMethodWin::DispatchFabricatedKeyEvent() method.
> > We can discuss offline about it.
> 
> Per offline discussion, please add SendKeyEvents() method in
> IMEInputContextHandlerInterface.
> Therefore, there is no need to get the input method here.

Done. Please review the latest patch set.

[Shu Chen, 2016-03-09 05:50:50]

lgtm

https://codereview.chromium.org/1771173002/diff/80001/ui/base/ime/input_metho...
File ui/base/ime/input_method_win.cc (right):

https://codereview.chromium.org/1771173002/diff/80001/ui/base/ime/input_metho...
ui/base/ime/input_method_win.cc:202: if (char_msgs->empty() &&
Replaces the condition char_msgs->empty() with event->HasNativeEvent().

[Azure Wei, 2016-03-09 05:54:44]

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

[commit-bot: I haz the power, 2016-03-09 05:55:02]

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

[commit-bot: I haz the power, 2016-03-09 05:57:31]

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

[commit-bot: I haz the power, 2016-03-09 05:57:32]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Azure Wei, 2016-03-09 06:02:15]

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

[commit-bot: I haz the power, 2016-03-09 06:02:28]

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

[commit-bot: I haz the power, 2016-03-09 07:36:06]

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

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

Dry run: This issue passed the CQ dry run.

[Devlin, 2016-03-10 23:21:14]

https://codereview.chromium.org/1771173002/diff/120001/chrome/browser/extensi...
File chrome/browser/extensions/api/input_ime/input_ime_api.cc (right):

https://codereview.chromium.org/1771173002/diff/120001/chrome/browser/extensi...
chrome/browser/extensions/api/input_ime/input_ime_api.cc:346:
SendKeyEvents::Params::Create(*args_));
EXTENSION_FUNCTION_VALIDATE(parent_params);

https://codereview.chromium.org/1771173002/diff/120001/chrome/browser/extensi...
chrome/browser/extensions/api/input_ime/input_ime_api.cc:352: for (size_t i = 0;
i < key_data.size(); ++i) {
const auto& key_event : key_data

https://codereview.chromium.org/1771173002/diff/120001/chrome/browser/ui/inpu...
File chrome/browser/ui/input_method/input_method_engine_base.h (right):

https://codereview.chromium.org/1771173002/diff/120001/chrome/browser/ui/inpu...
chrome/browser/ui/input_method/input_method_engine_base.h:189: virtual bool
SendKeyEvent(ui::KeyEvent* ui_event) = 0;
Could this be a const ui::KeyEvent&?  If not, can you clarify here if it can be
null, and add a couple DCHECKs?

https://codereview.chromium.org/1771173002/diff/120001/chrome/common/extensio...
File chrome/common/extensions/api/input_ime.json (right):

https://codereview.chromium.org/1771173002/diff/120001/chrome/common/extensio...
chrome/common/extensions/api/input_ime.json:747: "description": "This event is
sent if this extension owns the active IME.",
While we're here, can we elaborate on this a bit?  i.e., when is the event fired
(not just "when the extension owns the active IME" - that would mean it was
fired constantly ;))

https://codereview.chromium.org/1771173002/diff/120001/chrome/test/data/exten...
File chrome/test/data/extensions/api_test/input_ime_nonchromeos/background.js
(right):

https://codereview.chromium.org/1771173002/diff/120001/chrome/test/data/exten...
chrome/test/data/extensions/api_test/input_ime_nonchromeos/background.js:58: var
key_vent_count = 0;
nit: event, not vent
nit: use js-style here (so keyEventCount).

https://codereview.chromium.org/1771173002/diff/120001/chrome/test/data/exten...
chrome/test/data/extensions/api_test/input_ime_nonchromeos/background.js:60:
chrome.input.ime.onKeyEvent.addListener(function(engineID, keyData) {
Hmm... it's expected behavior to receive key events for the events the extension
itself sends?  What's the use case here?

[Azure Wei, 2016-03-11 07:29:01]

https://codereview.chromium.org/1771173002/diff/120001/chrome/browser/extensi...
File chrome/browser/extensions/api/input_ime/input_ime_api.cc (right):

https://codereview.chromium.org/1771173002/diff/120001/chrome/browser/extensi...
chrome/browser/extensions/api/input_ime/input_ime_api.cc:346:
SendKeyEvents::Params::Create(*args_));
On 2016/03/10 23:21:14, Devlin wrote:
> EXTENSION_FUNCTION_VALIDATE(parent_params);

Done.

https://codereview.chromium.org/1771173002/diff/120001/chrome/browser/extensi...
chrome/browser/extensions/api/input_ime/input_ime_api.cc:352: for (size_t i = 0;
i < key_data.size(); ++i) {
On 2016/03/10 23:21:14, Devlin wrote:
> const auto& key_event : key_data

Done.

https://codereview.chromium.org/1771173002/diff/120001/chrome/browser/ui/inpu...
File chrome/browser/ui/input_method/input_method_engine_base.h (right):

https://codereview.chromium.org/1771173002/diff/120001/chrome/browser/ui/inpu...
chrome/browser/ui/input_method/input_method_engine_base.h:189: virtual bool
SendKeyEvent(ui::KeyEvent* ui_event) = 0;
On 2016/03/10 23:21:14, Devlin wrote:
> Could this be a const ui::KeyEvent&?  If not, can you clarify here if it can
be
> null, and add a couple DCHECKs?

DCHECKs were added. We need the set the state of the event, so it should be
ui::KeyEvent*.

https://codereview.chromium.org/1771173002/diff/120001/chrome/common/extensio...
File chrome/common/extensions/api/input_ime.json (right):

https://codereview.chromium.org/1771173002/diff/120001/chrome/common/extensio...
chrome/common/extensions/api/input_ime.json:747: "description": "This event is
sent if this extension owns the active IME.",
On 2016/03/10 23:21:14, Devlin wrote:
> While we're here, can we elaborate on this a bit?  i.e., when is the event
fired
> (not just "when the extension owns the active IME" - that would mean it was
> fired constantly ;))

Done.

https://codereview.chromium.org/1771173002/diff/120001/chrome/test/data/exten...
File chrome/test/data/extensions/api_test/input_ime_nonchromeos/background.js
(right):

https://codereview.chromium.org/1771173002/diff/120001/chrome/test/data/exten...
chrome/test/data/extensions/api_test/input_ime_nonchromeos/background.js:58: var
key_vent_count = 0;
On 2016/03/10 23:21:14, Devlin wrote:
> nit: event, not vent
> nit: use js-style here (so keyEventCount).

Done.

https://codereview.chromium.org/1771173002/diff/120001/chrome/test/data/exten...
chrome/test/data/extensions/api_test/input_ime_nonchromeos/background.js:60:
chrome.input.ime.onKeyEvent.addListener(function(engineID, keyData) {
On 2016/03/10 23:21:14, Devlin wrote:
> Hmm... it's expected behavior to receive key events for the events the
extension
> itself sends?  What's the use case here?

Actually the extension won't handle the key events if they are sent by the
extension itself. This logic was just for keeping consistent with the logic in
ChromeOS. It's fine to not pass the event to the extension, if you feel strongly
wearied about this. Please review the updated codes.

[Devlin, 2016-03-14 22:32:01]

extensions lgtm

https://codereview.chromium.org/1771173002/diff/200001/chrome/browser/chromeo...
File chrome/browser/chromeos/input_method/input_method_engine.cc (right):

https://codereview.chromium.org/1771173002/diff/200001/chrome/browser/chromeo...
chrome/browser/chromeos/input_method/input_method_engine.cc:337: return false;
nit: Could we do
return !details.dispatcher_destroyed;
?

https://codereview.chromium.org/1771173002/diff/200001/ui/base/ime/input_meth...
File ui/base/ime/input_method_auralinux.cc (right):

https://codereview.chromium.org/1771173002/diff/200001/ui/base/ime/input_meth...
ui/base/ime/input_method_auralinux.cc:79: return;
don't need this return

[Azure Wei, 2016-03-15 00:37:25]

https://codereview.chromium.org/1771173002/diff/200001/chrome/browser/chromeo...
File chrome/browser/chromeos/input_method/input_method_engine.cc (right):

https://codereview.chromium.org/1771173002/diff/200001/chrome/browser/chromeo...
chrome/browser/chromeos/input_method/input_method_engine.cc:337: return false;
On 2016/03/14 22:32:01, Devlin wrote:
> nit: Could we do
> return !details.dispatcher_destroyed;
> ?

Done.

https://codereview.chromium.org/1771173002/diff/200001/ui/base/ime/input_meth...
File ui/base/ime/input_method_auralinux.cc (right):

https://codereview.chromium.org/1771173002/diff/200001/ui/base/ime/input_meth...
ui/base/ime/input_method_auralinux.cc:79: return;
On 2016/03/14 22:32:01, Devlin wrote:
> don't need this return

This 'return' is necessary to prevent the function from running the following
codes.

[Azure Wei, 2016-03-15 00:37:41]

The CQ bit was checked by azurewei@chromium.org

[Azure Wei, 2016-03-15 00:37:42]

The patchset sent to the CQ was uploaded after l-g-t-m from
shuchen@chromium.org, rdevlin.cronin@chromium.org
Link to the patchset: https://codereview.chromium.org/1771173002/#ps220001
(title: " ")

[commit-bot: I haz the power, 2016-03-15 00:37:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1771173002/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1771173002/220001

[commit-bot: I haz the power, 2016-03-15 01:12:02]

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

[commit-bot: I haz the power, 2016-03-15 01:12:04]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_compile_dbg on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[Azure Wei, 2016-03-15 02:09:18]

The CQ bit was checked by azurewei@chromium.org

[Azure Wei, 2016-03-15 02:09:19]

The patchset sent to the CQ was uploaded after l-g-t-m from
shuchen@chromium.org, rdevlin.cronin@chromium.org
Link to the patchset: https://codereview.chromium.org/1771173002/#ps240001
(title: "Update gyp/gn file.")

[commit-bot: I haz the power, 2016-03-15 02:09:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1771173002/240001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1771173002/240001

[commit-bot: I haz the power, 2016-03-15 02:36:36]

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

[commit-bot: I haz the power, 2016-03-15 02:36:38]

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

[Azure Wei, 2016-03-15 03:30:53]

The CQ bit was checked by azurewei@chromium.org

[Azure Wei, 2016-03-15 03:30:53]

The patchset sent to the CQ was uploaded after l-g-t-m from
shuchen@chromium.org, rdevlin.cronin@chromium.org
Link to the patchset: https://codereview.chromium.org/1771173002/#ps260001
(title: "Fix test failure.")

[commit-bot: I haz the power, 2016-03-15 03:31:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1771173002/260001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1771173002/260001

[commit-bot: I haz the power, 2016-03-15 04:45:07]

Description was changed from

==========
Implement input.ime.sendKeyEvents API.

This cl implements input.ime.senKeyEvents API on Linux & Windows platform. The
API calls the method InputMethodEngineBase::SendKeyEvents(), which contains the
common code for ChromeOS/NonChromeOS. And the platform-specific codes are
written in InputMethodEngine::SendKeyEvent().

On ChromeOS, the key event is dispatched from directly calling
EventProcessor::OnEventFromSource(). While, On Linux & Windows, the key event is
dispatched through ImeInputContextHandlerInterface::SendKeyEvent(), which will
be implemented by InputMethodXxx. 

This implementation is similar to input.ime.setComposition and
input.ime.commitText APIs.

Add browser test in c/t/d/e/api_test/input_ime_nonchromeos/*.

BUG=517773
TEST=BrowserTest --gtest_filter=InputImeApiTest.*
==========

to

==========
Implement input.ime.sendKeyEvents API.

This cl implements input.ime.senKeyEvents API on Linux & Windows platform. The
API calls the method InputMethodEngineBase::SendKeyEvents(), which contains the
common code for ChromeOS/NonChromeOS. And the platform-specific codes are
written in InputMethodEngine::SendKeyEvent().

On ChromeOS, the key event is dispatched from directly calling
EventProcessor::OnEventFromSource(). While, On Linux & Windows, the key event is
dispatched through ImeInputContextHandlerInterface::SendKeyEvent(), which will
be implemented by InputMethodXxx. 

This implementation is similar to input.ime.setComposition and
input.ime.commitText APIs.

Add browser test in c/t/d/e/api_test/input_ime_nonchromeos/*.

BUG=517773
TEST=BrowserTest --gtest_filter=InputImeApiTest.*
==========

[commit-bot: I haz the power, 2016-03-15 04:45:09]

Committed patchset #14 (id:260001)

[commit-bot: I haz the power, 2016-03-15 04:46:22]

Description was changed from

==========
Implement input.ime.sendKeyEvents API.

This cl implements input.ime.senKeyEvents API on Linux & Windows platform. The
API calls the method InputMethodEngineBase::SendKeyEvents(), which contains the
common code for ChromeOS/NonChromeOS. And the platform-specific codes are
written in InputMethodEngine::SendKeyEvent().

On ChromeOS, the key event is dispatched from directly calling
EventProcessor::OnEventFromSource(). While, On Linux & Windows, the key event is
dispatched through ImeInputContextHandlerInterface::SendKeyEvent(), which will
be implemented by InputMethodXxx. 

This implementation is similar to input.ime.setComposition and
input.ime.commitText APIs.

Add browser test in c/t/d/e/api_test/input_ime_nonchromeos/*.

BUG=517773
TEST=BrowserTest --gtest_filter=InputImeApiTest.*
==========

to

==========
Implement input.ime.sendKeyEvents API.

This cl implements input.ime.senKeyEvents API on Linux & Windows platform. The
API calls the method InputMethodEngineBase::SendKeyEvents(), which contains the
common code for ChromeOS/NonChromeOS. And the platform-specific codes are
written in InputMethodEngine::SendKeyEvent().

On ChromeOS, the key event is dispatched from directly calling
EventProcessor::OnEventFromSource(). While, On Linux & Windows, the key event is
dispatched through ImeInputContextHandlerInterface::SendKeyEvent(), which will
be implemented by InputMethodXxx.

This implementation is similar to input.ime.setComposition and
input.ime.commitText APIs.

Add browser test in c/t/d/e/api_test/input_ime_nonchromeos/*.

BUG=517773
TEST=BrowserTest --gtest_filter=InputImeApiTest.*

Committed: https://crrev.com/b293444e8dbe567102fb82ab8831b4e7ac23a59a
Cr-Commit-Position: refs/heads/master@{#381175}
==========

[commit-bot: I haz the power, 2016-03-15 04:46:23]

Patchset 14 (id:??) landed as
https://crrev.com/b293444e8dbe567102fb82ab8831b4e7ac23a59a
Cr-Commit-Position: refs/heads/master@{#381175}