Move chromeos::EventRewriter to //ui/chromeos/events

Move chromeos::EventRewriter to //ui/chromeos/events

chromeos::EventRewriter is responsible for converting F1~F12 to media
function keys. Those conversion is not working in mus+ash. This CL moves
chromeos::EventRewriter to //ui/chromeos/events from
//chrome/browser/chromeos/events. So it can be used by mus as well.

BUG=693180
Review-Url: https://codereview.chromium.org/2724913002
Cr-Commit-Position: refs/heads/master@{#457592}
Committed: https://chromium.googlesource.com/chromium/src/+/e5df28289ab75276db7941bb8c5c76705ac1ef8f

[Peng, 2017-03-01 19:01:10]

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

[commit-bot: I haz the power, 2017-03-01 19:01:50]

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-01 19:14:07]

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

[commit-bot: I haz the power, 2017-03-01 19:14:08]

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

[Peng, 2017-03-01 19:30:23]

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

[commit-bot: I haz the power, 2017-03-01 19:31:15]

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-01 19:43:03]

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

[commit-bot: I haz the power, 2017-03-01 19:43:04]

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

[Peng, 2017-03-01 19:48:37]

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

[commit-bot: I haz the power, 2017-03-01 19:49:52]

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-01 19:58:32]

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

[commit-bot: I haz the power, 2017-03-01 19:58:33]

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

[Peng, 2017-03-01 20:32:06]

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

[commit-bot: I haz the power, 2017-03-01 20:33:10]

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-01 20:54:14]

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

[commit-bot: I haz the power, 2017-03-01 20:54:15]

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

[Peng, 2017-03-02 19:52:20]

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

[commit-bot: I haz the power, 2017-03-02 19:53:00]

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-02 20:24:38]

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

[commit-bot: I haz the power, 2017-03-02 20:24:39]

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

[Peng, 2017-03-02 20:27:37]

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

[commit-bot: I haz the power, 2017-03-02 20:28:32]

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

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

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

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_...)

[Peng, 2017-03-02 21:25:30]

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

[commit-bot: I haz the power, 2017-03-02 21:26:05]

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

[Peng, 2017-03-02 21:39:01]

Description was changed from

==========
WIP


WIP


WIP


WIP


WIP


Refactor event rewriter

BUG=
==========

to

==========
Move chromeos::EventRewriter to //ui/chromeos/events

chromeos::EventRewriter is responsible for converting F1~F12 to media
function keys. Those conversion is not working in mus+ash. This CL moves
chromeos::EventRewriter to //ui/chromeos/events from
//chrome/browser/chromeos/events. So it can be used by mus as well.

BUG=693180
==========

[Peng, 2017-03-02 21:51:12]

penghuang@chromium.org changed reviewers:
+ sadrul@chromium.org

[Peng, 2017-03-02 21:51:13]

Hi Sadrul, PTAL. Thanks.

[commit-bot: I haz the power, 2017-03-02 22:03:36]

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

[commit-bot: I haz the power, 2017-03-02 22:03:36]

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_...)

[Peng, 2017-03-02 22:10:01]

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

[commit-bot: I haz the power, 2017-03-02 22:10:34]

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-02 23:33:16]

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

[commit-bot: I haz the power, 2017-03-02 23:33:18]

Dry run: This issue passed the CQ dry run.

[sadrul, 2017-03-03 15:11:42]

https://codereview.chromium.org/2724913002/diff/160001/chrome/browser/chromeo...
File chrome/browser/chromeos/chrome_browser_main_chromeos.h (right):

https://codereview.chromium.org/2724913002/diff/160001/chrome/browser/chromeo...
chrome/browser/chromeos/chrome_browser_main_chromeos.h:92:
std::unique_ptr<EventRewriterDelegateImpl> event_rewriter_delegate_;
This should be destroyed *after* all the event-rewriters are destroyed.

https://codereview.chromium.org/2724913002/diff/160001/ui/chromeos/events/eve...
File ui/chromeos/events/event_rewriter.cc (right):

https://codereview.chromium.org/2724913002/diff/160001/ui/chromeos/events/eve...
ui/chromeos/events/event_rewriter.cc:40: #endif
We should get rid of the USE_X11 code, since it's no longer set for chromeos.
But that should happen separately (so after this CL ... or before, whichever is
convenient)

https://codereview.chromium.org/2724913002/diff/160001/ui/chromeos/events/eve...
File ui/chromeos/events/event_rewriter.h (right):

https://codereview.chromium.org/2724913002/diff/160001/ui/chromeos/events/eve...
ui/chromeos/events/event_rewriter.h:89: int* flags) = 0;
It looks to me like we don't actually need the StickeKeysController interface
here at all. This EventRewriter can simply take in another ui::EventRewriter,
and use that interface instead. The StickyKeysController overrides of
ui::EventRewriter would make the appropriate calls to RewriteKeyEvent() etc. Is
there something that wouldn't work with that approach?

[Peng, 2017-03-03 16:07:05]

https://codereview.chromium.org/2724913002/diff/160001/chrome/browser/chromeo...
File chrome/browser/chromeos/chrome_browser_main_chromeos.h (right):

https://codereview.chromium.org/2724913002/diff/160001/chrome/browser/chromeo...
chrome/browser/chromeos/chrome_browser_main_chromeos.h:92:
std::unique_ptr<EventRewriterDelegateImpl> event_rewriter_delegate_;
On 2017/03/03 15:11:41, sadrul wrote:
> This should be destroyed *after* all the event-rewriters are destroyed.

Done.

https://codereview.chromium.org/2724913002/diff/160001/ui/chromeos/events/eve...
File ui/chromeos/events/event_rewriter.cc (right):

https://codereview.chromium.org/2724913002/diff/160001/ui/chromeos/events/eve...
ui/chromeos/events/event_rewriter.cc:40: #endif
On 2017/03/03 15:11:41, sadrul wrote:
> We should get rid of the USE_X11 code, since it's no longer set for chromeos.
> But that should happen separately (so after this CL ... or before, whichever
is
> convenient)

Sure. I will do it in a separate CL.

https://codereview.chromium.org/2724913002/diff/160001/ui/chromeos/events/eve...
File ui/chromeos/events/event_rewriter.h (right):

https://codereview.chromium.org/2724913002/diff/160001/ui/chromeos/events/eve...
ui/chromeos/events/event_rewriter.h:89: int* flags) = 0;
On 2017/03/03 15:11:41, sadrul wrote:
> It looks to me like we don't actually need the StickeKeysController interface
> here at all. This EventRewriter can simply take in another ui::EventRewriter,
> and use that interface instead. The StickyKeysController overrides of
> ui::EventRewriter would make the appropriate calls to RewriteKeyEvent() etc.
Is
> there something that wouldn't work with that approach?

The signatures of EventRewriter is different. It is not straightforward to use
EventRewriter. So I prefer to make this CL simple. How about we do it in a
separate CL?

[Wez, 2017-03-03 23:31:19]

nit: Note that there are some typos in the comments of the new code in
EventRewriter header.

On 3 March 2017 at 08:07, <penghuang@chromium.org> wrote:

>
> https://codereview.chromium.org/2724913002/diff/160001/
> chrome/browser/chromeos/chrome_browser_main_chromeos.h
> File chrome/browser/chromeos/chrome_browser_main_chromeos.h (right):
>
> https://codereview.chromium.org/2724913002/diff/160001/
> chrome/browser/chromeos/chrome_browser_main_chromeos.h#newcode92
> chrome/browser/chromeos/chrome_browser_main_chromeos.h:92:
> std::unique_ptr<EventRewriterDelegateImpl> event_rewriter_delegate_;
> On 2017/03/03 15:11:41, sadrul wrote:
> > This should be destroyed *after* all the event-rewriters are
> destroyed.
>
> Done.
>
> https://codereview.chromium.org/2724913002/diff/160001/ui/
> chromeos/events/event_rewriter.cc
> File ui/chromeos/events/event_rewriter.cc (right):
>
> https://codereview.chromium.org/2724913002/diff/160001/ui/
> chromeos/events/event_rewriter.cc#newcode40
> ui/chromeos/events/event_rewriter.cc:40: #endif
> On 2017/03/03 15:11:41, sadrul wrote:
> > We should get rid of the USE_X11 code, since it's no longer set for
> chromeos.
> > But that should happen separately (so after this CL ... or before,
> whichever is
> > convenient)
>
> Sure. I will do it in a separate CL.
>
> https://codereview.chromium.org/2724913002/diff/160001/ui/
> chromeos/events/event_rewriter.h
> File ui/chromeos/events/event_rewriter.h (right):
>
> https://codereview.chromium.org/2724913002/diff/160001/ui/
> chromeos/events/event_rewriter.h#newcode89
> ui/chromeos/events/event_rewriter.h:89: int* flags) = 0;
> On 2017/03/03 15:11:41, sadrul wrote:
> > It looks to me like we don't actually need the StickeKeysController
> interface
> > here at all. This EventRewriter can simply take in another
> ui::EventRewriter,
> > and use that interface instead. The StickyKeysController overrides of
> > ui::EventRewriter would make the appropriate calls to
> RewriteKeyEvent() etc. Is
> > there something that wouldn't work with that approach?
>
> The signatures of EventRewriter is different. It is not straightforward
> to use EventRewriter. So I prefer to make this CL simple. How about we
> do it in a separate CL?
>
> https://codereview.chromium.org/2724913002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Peng, 2017-03-06 15:18:18]

On 2017/03/03 23:31:19, Wez wrote:
> nit: Note that there are some typos in the comments of the new code in
> EventRewriter header.
> 
> On 3 March 2017 at 08:07, <mailto:penghuang@chromium.org> wrote:
> 
> >
> > https://codereview.chromium.org/2724913002/diff/160001/
> > chrome/browser/chromeos/chrome_browser_main_chromeos.h
> > File chrome/browser/chromeos/chrome_browser_main_chromeos.h (right):
> >
> > https://codereview.chromium.org/2724913002/diff/160001/
> > chrome/browser/chromeos/chrome_browser_main_chromeos.h#newcode92
> > chrome/browser/chromeos/chrome_browser_main_chromeos.h:92:
> > std::unique_ptr<EventRewriterDelegateImpl> event_rewriter_delegate_;
> > On 2017/03/03 15:11:41, sadrul wrote:
> > > This should be destroyed *after* all the event-rewriters are
> > destroyed.
> >
> > Done.
> >
> > https://codereview.chromium.org/2724913002/diff/160001/ui/
> > chromeos/events/event_rewriter.cc
> > File ui/chromeos/events/event_rewriter.cc (right):
> >
> > https://codereview.chromium.org/2724913002/diff/160001/ui/
> > chromeos/events/event_rewriter.cc#newcode40
> > ui/chromeos/events/event_rewriter.cc:40: #endif
> > On 2017/03/03 15:11:41, sadrul wrote:
> > > We should get rid of the USE_X11 code, since it's no longer set for
> > chromeos.
> > > But that should happen separately (so after this CL ... or before,
> > whichever is
> > > convenient)
> >
> > Sure. I will do it in a separate CL.
> >
> > https://codereview.chromium.org/2724913002/diff/160001/ui/
> > chromeos/events/event_rewriter.h
> > File ui/chromeos/events/event_rewriter.h (right):
> >
> > https://codereview.chromium.org/2724913002/diff/160001/ui/
> > chromeos/events/event_rewriter.h#newcode89
> > ui/chromeos/events/event_rewriter.h:89: int* flags) = 0;
> > On 2017/03/03 15:11:41, sadrul wrote:
> > > It looks to me like we don't actually need the StickeKeysController
> > interface
> > > here at all. This EventRewriter can simply take in another
> > ui::EventRewriter,
> > > and use that interface instead. The StickyKeysController overrides of
> > > ui::EventRewriter would make the appropriate calls to
> > RewriteKeyEvent() etc. Is
> > > there something that wouldn't work with that approach?
> >
> > The signatures of EventRewriter is different. It is not straightforward
> > to use EventRewriter. So I prefer to make this CL simple. How about we
> > do it in a separate CL?
> >
> > https://codereview.chromium.org/2724913002/
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[Peng, 2017-03-06 16:14:52]

On 2017/03/03 23:31:19, Wez wrote:
> nit: Note that there are some typos in the comments of the new code in
> EventRewriter header.

Where are those typos? Could you please add comment inline? Thanks.

> 
> On 3 March 2017 at 08:07, <mailto:penghuang@chromium.org> wrote:
> 
> >
> > https://codereview.chromium.org/2724913002/diff/160001/
> > chrome/browser/chromeos/chrome_browser_main_chromeos.h
> > File chrome/browser/chromeos/chrome_browser_main_chromeos.h (right):
> >
> > https://codereview.chromium.org/2724913002/diff/160001/
> > chrome/browser/chromeos/chrome_browser_main_chromeos.h#newcode92
> > chrome/browser/chromeos/chrome_browser_main_chromeos.h:92:
> > std::unique_ptr<EventRewriterDelegateImpl> event_rewriter_delegate_;
> > On 2017/03/03 15:11:41, sadrul wrote:
> > > This should be destroyed *after* all the event-rewriters are
> > destroyed.
> >
> > Done.
> >
> > https://codereview.chromium.org/2724913002/diff/160001/ui/
> > chromeos/events/event_rewriter.cc
> > File ui/chromeos/events/event_rewriter.cc (right):
> >
> > https://codereview.chromium.org/2724913002/diff/160001/ui/
> > chromeos/events/event_rewriter.cc#newcode40
> > ui/chromeos/events/event_rewriter.cc:40: #endif
> > On 2017/03/03 15:11:41, sadrul wrote:
> > > We should get rid of the USE_X11 code, since it's no longer set for
> > chromeos.
> > > But that should happen separately (so after this CL ... or before,
> > whichever is
> > > convenient)
> >
> > Sure. I will do it in a separate CL.
> >
> > https://codereview.chromium.org/2724913002/diff/160001/ui/
> > chromeos/events/event_rewriter.h
> > File ui/chromeos/events/event_rewriter.h (right):
> >
> > https://codereview.chromium.org/2724913002/diff/160001/ui/
> > chromeos/events/event_rewriter.h#newcode89
> > ui/chromeos/events/event_rewriter.h:89: int* flags) = 0;
> > On 2017/03/03 15:11:41, sadrul wrote:
> > > It looks to me like we don't actually need the StickeKeysController
> > interface
> > > here at all. This EventRewriter can simply take in another
> > ui::EventRewriter,
> > > and use that interface instead. The StickyKeysController overrides of
> > > ui::EventRewriter would make the appropriate calls to
> > RewriteKeyEvent() etc. Is
> > > there something that wouldn't work with that approach?
> >
> > The signatures of EventRewriter is different. It is not straightforward
> > to use EventRewriter. So I prefer to make this CL simple. How about we
> > do it in a separate CL?
> >
> > https://codereview.chromium.org/2724913002/
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[sadrul, 2017-03-06 16:54:27]

https://codereview.chromium.org/2724913002/diff/160001/ui/chromeos/events/eve...
File ui/chromeos/events/event_rewriter.h (right):

https://codereview.chromium.org/2724913002/diff/160001/ui/chromeos/events/eve...
ui/chromeos/events/event_rewriter.h:89: int* flags) = 0;
On 2017/03/03 16:07:05, Peng wrote:
> On 2017/03/03 15:11:41, sadrul wrote:
> > It looks to me like we don't actually need the StickeKeysController
interface
> > here at all. This EventRewriter can simply take in another
ui::EventRewriter,
> > and use that interface instead. The StickyKeysController overrides of
> > ui::EventRewriter would make the appropriate calls to RewriteKeyEvent() etc.
> Is
> > there something that wouldn't work with that approach?
> 
> The signatures of EventRewriter is different. It is not straightforward to use
> EventRewriter. So I prefer to make this CL simple. How about we do it in a
> separate CL?

Separate CL makes sense, but I think we should that first, rather than introduce
this short-lived interface.

[Peng, 2017-03-06 16:57:31]

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

[commit-bot: I haz the power, 2017-03-06 16:58:00]

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-06 17:01:56]

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

[commit-bot: I haz the power, 2017-03-06 17:01:57]

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

[Peng, 2017-03-09 19:07:26]

On 2017/03/06 16:54:27, sadrul wrote:
>
https://codereview.chromium.org/2724913002/diff/160001/ui/chromeos/events/eve...
> File ui/chromeos/events/event_rewriter.h (right):
> 
>
https://codereview.chromium.org/2724913002/diff/160001/ui/chromeos/events/eve...
> ui/chromeos/events/event_rewriter.h:89: int* flags) = 0;
> On 2017/03/03 16:07:05, Peng wrote:
> > On 2017/03/03 15:11:41, sadrul wrote:
> > > It looks to me like we don't actually need the StickeKeysController
> interface
> > > here at all. This EventRewriter can simply take in another
> ui::EventRewriter,
> > > and use that interface instead. The StickyKeysController overrides of
> > > ui::EventRewriter would make the appropriate calls to RewriteKeyEvent()
etc.
> > Is
> > > there something that wouldn't work with that approach?
> > 
> > The signatures of EventRewriter is different. It is not straightforward to
use
> > EventRewriter. So I prefer to make this CL simple. How about we do it in a
> > separate CL?
> 
> Separate CL makes sense, but I think we should that first, rather than
introduce
> this short-lived interface.

Hi Sadrul, I have created CL https://codereview.chromium.org/2731283004/ to use
ui::EventRewriter
and updated this CL. PTAL. Thanks.

[Peng, 2017-03-13 14:45:34]

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

[commit-bot: I haz the power, 2017-03-13 14:45:59]

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-13 14:50:25]

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

[commit-bot: I haz the power, 2017-03-13 14:50:27]

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

[Peng, 2017-03-13 14:55:42]

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

[commit-bot: I haz the power, 2017-03-13 14:56:08]

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-13 16:18:19]

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

[commit-bot: I haz the power, 2017-03-13 16:18:20]

Dry run: This issue passed the CQ dry run.

[Peng, 2017-03-13 17:16:17]

On 2017/03/09 19:07:26, Peng wrote:
> On 2017/03/06 16:54:27, sadrul wrote:
> >
>
https://codereview.chromium.org/2724913002/diff/160001/ui/chromeos/events/eve...
> > File ui/chromeos/events/event_rewriter.h (right):
> > 
> >
>
https://codereview.chromium.org/2724913002/diff/160001/ui/chromeos/events/eve...
> > ui/chromeos/events/event_rewriter.h:89: int* flags) = 0;
> > On 2017/03/03 16:07:05, Peng wrote:
> > > On 2017/03/03 15:11:41, sadrul wrote:
> > > > It looks to me like we don't actually need the StickeKeysController
> > interface
> > > > here at all. This EventRewriter can simply take in another
> > ui::EventRewriter,
> > > > and use that interface instead. The StickyKeysController overrides of
> > > > ui::EventRewriter would make the appropriate calls to RewriteKeyEvent()
> etc.
> > > Is
> > > > there something that wouldn't work with that approach?
> > > 
> > > The signatures of EventRewriter is different. It is not straightforward to
> use
> > > EventRewriter. So I prefer to make this CL simple. How about we do it in a
> > > separate CL?
> > 
> > Separate CL makes sense, but I think we should that first, rather than
> introduce
> > this short-lived interface.
> 
> Hi Sadrul, I have created CL https://codereview.chromium.org/2731283004/ to
use
> ui::EventRewriter
> and updated this CL. PTAL. Thanks.

sadrul, kindly ping.

[sadrul, 2017-03-14 01:15:41]

Looks pretty close. Some cleanups

https://codereview.chromium.org/2724913002/diff/260001/chrome/browser/chromeo...
File chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc (right):

https://codereview.chromium.org/2724913002/diff/260001/chrome/browser/chromeo...
chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc:73: return
preference->GetValue()->GetBool();
Just use preference->GetBoolean(...), instead of DCHECK GetType() and
GetValue()->GetBool()?

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/BUI...
File ui/chromeos/events/BUILD.gn (right):

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/BUI...
ui/chromeos/events/BUILD.gn:10: component("events") {
Can this be a source_set instead?

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/BUI...
ui/chromeos/events/BUILD.gn:18: defines = [ "UI_CHROMEOS_IMPLEMENTATION" ]
Probably don't need this if this becomes a source_set

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/BUI...
ui/chromeos/events/BUILD.gn:21: "//chromeos:chromeos",
Just //chromeos

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/eve...
File ui/chromeos/events/event_rewriter.cc (right):

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/eve...
ui/chromeos/events/event_rewriter.cc:275: ime_keyboard_for_testing_(NULL),
While you are here: nullptr?

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/eve...
File ui/chromeos/events/event_rewriter.h (right):

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/eve...
ui/chromeos/events/event_rewriter.h:29: namespace chromeos {
It doesn't look like any other code in //ui/chromeos uses namespace chromeos. So
let's remove this nested namespace?

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/eve...
ui/chromeos/events/event_rewriter.h:58: class StickyKeysController {
I thought we can get rid of this now?

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/eve...
ui/chromeos/events/event_rewriter.h:109: virtual bool RewriteModifierKeys() = 0;
Document.

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/eve...
ui/chromeos/events/event_rewriter.h:122: // Retunrs true if the |key_code| and
|flags| have been resgistered for
Returns

If |key_code| and |flags| are registered by extensions, does that mean the event
is not rewritten?

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/pre...
File ui/chromeos/events/pref_names.cc (right):

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/pre...
ui/chromeos/events/pref_names.cc:7: #include "base/macros.h"
Is this needed?

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/pre...
File ui/chromeos/events/pref_names.h (right):

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/pre...
ui/chromeos/events/pref_names.h:5: // Constants for the names of various
preferences, for easier changing.
Move this comment down in line ~13?

Can you also add a TODO that this probably needs to live in a better place?

[Peng, 2017-03-14 16:06:39]

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

[Peng, 2017-03-14 16:06:53]

https://codereview.chromium.org/2724913002/diff/260001/chrome/browser/chromeo...
File chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc (right):

https://codereview.chromium.org/2724913002/diff/260001/chrome/browser/chromeo...
chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc:73: return
preference->GetValue()->GetBool();
On 2017/03/14 01:15:41, sadrul wrote:
> Just use preference->GetBoolean(...), instead of DCHECK GetType() and
> GetValue()->GetBool()?

I checked the code and found the preference object doesn't have GetBoolean.

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/BUI...
File ui/chromeos/events/BUILD.gn (right):

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/BUI...
ui/chromeos/events/BUILD.gn:10: component("events") {
On 2017/03/14 01:15:41, sadrul wrote:
> Can this be a source_set instead?

Done.

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/BUI...
ui/chromeos/events/BUILD.gn:18: defines = [ "UI_CHROMEOS_IMPLEMENTATION" ]
On 2017/03/14 01:15:41, sadrul wrote:
> Probably don't need this if this becomes a source_set

Done.

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/BUI...
ui/chromeos/events/BUILD.gn:21: "//chromeos:chromeos",
On 2017/03/14 01:15:41, sadrul wrote:
> Just //chromeos

Done.

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/eve...
File ui/chromeos/events/event_rewriter.cc (right):

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/eve...
ui/chromeos/events/event_rewriter.cc:275: ime_keyboard_for_testing_(NULL),
On 2017/03/14 01:15:41, sadrul wrote:
> While you are here: nullptr?

Done. And replace all NULL with nullptr.

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/eve...
File ui/chromeos/events/event_rewriter.h (right):

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/eve...
ui/chromeos/events/event_rewriter.h:29: namespace chromeos {
On 2017/03/14 01:15:41, sadrul wrote:
> It doesn't look like any other code in //ui/chromeos uses namespace chromeos.
So
> let's remove this nested namespace?

Without it, ui::chromeos::EventRewriter will conflict with ui::EventRewriter.

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/eve...
ui/chromeos/events/event_rewriter.h:58: class StickyKeysController {
On 2017/03/14 01:15:41, sadrul wrote:
> I thought we can get rid of this now?

Oops! Forgot remove it. Done

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/eve...
ui/chromeos/events/event_rewriter.h:109: virtual bool RewriteModifierKeys() = 0;
On 2017/03/14 01:15:41, sadrul wrote:
> Document.

Done.

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/eve...
ui/chromeos/events/event_rewriter.h:122: // Retunrs true if the |key_code| and
|flags| have been resgistered for
On 2017/03/14 01:15:41, sadrul wrote:
> Returns
> 
> If |key_code| and |flags| are registered by extensions, does that mean the
event
> is not rewritten?

Yes. If key_code and flags combination has been registered by extension, the
rewriter will not rewrite the event.

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/pre...
File ui/chromeos/events/pref_names.cc (right):

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/pre...
ui/chromeos/events/pref_names.cc:7: #include "base/macros.h"
On 2017/03/14 01:15:41, sadrul wrote:
> Is this needed?

Done.

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/pre...
File ui/chromeos/events/pref_names.h (right):

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/pre...
ui/chromeos/events/pref_names.h:5: // Constants for the names of various
preferences, for easier changing.
On 2017/03/14 01:15:41, sadrul wrote:
> Move this comment down in line ~13?
> 
> Can you also add a TODO that this probably needs to live in a better place?

Done.

[commit-bot: I haz the power, 2017-03-14 16:07:22]

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-14 17:23:29]

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

[commit-bot: I haz the power, 2017-03-14 17:23:31]

Dry run: This issue passed the CQ dry run.

[sadrul, 2017-03-16 05:02:43]

lgtm with the suggested changes below

https://codereview.chromium.org/2724913002/diff/260001/chrome/browser/chromeo...
File chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc (right):

https://codereview.chromium.org/2724913002/diff/260001/chrome/browser/chromeo...
chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc:73: return
preference->GetValue()->GetBool();
On 2017/03/14 16:06:52, Peng wrote:
> On 2017/03/14 01:15:41, sadrul wrote:
> > Just use preference->GetBoolean(...), instead of DCHECK GetType() and
> > GetValue()->GetBool()?
> 
> I checked the code and found the preference object doesn't have GetBoolean.

Sorry, yeah, I was referring to using PrefService::GetBoolean() [1], which is
what the current code uses.

1:
https://cs.chromium.org/chromium/src/components/prefs/pref_service.h?type=cs&...

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/eve...
File ui/chromeos/events/event_rewriter.h (right):

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/eve...
ui/chromeos/events/event_rewriter.h:29: namespace chromeos {
On 2017/03/14 16:06:52, Peng wrote:
> On 2017/03/14 01:15:41, sadrul wrote:
> > It doesn't look like any other code in //ui/chromeos uses namespace
chromeos.
> So
> > let's remove this nested namespace?
> 
> Without it, ui::chromeos::EventRewriter will conflict with ui::EventRewriter.

Can you rename it EventRewriterChromeOS instead, then?

https://codereview.chromium.org/2724913002/diff/280001/ui/chromeos/events/eve...
File ui/chromeos/events/event_rewriter.h (right):

https://codereview.chromium.org/2724913002/diff/280001/ui/chromeos/events/eve...
ui/chromeos/events/event_rewriter.h:76: // Retunrs true if the |key_code| and
|flags| have been resgistered for
*Returns

[sadrul, 2017-03-16 05:04:00]

https://codereview.chromium.org/2724913002/diff/280001/chrome/browser/chromeo...
File chrome/browser/chromeos/events/event_rewriter_unittest.cc (right):

https://codereview.chromium.org/2724913002/diff/280001/chrome/browser/chromeo...
chrome/browser/chromeos/events/event_rewriter_unittest.cc:2606: 
In a follow-up CL, can you look into moving these tests (at least the ones that
can be extracted without pulling in dependency on chrome or ash) into
ui/chromeos/events/ too?

[Peng, 2017-03-16 15:44:32]

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

[commit-bot: I haz the power, 2017-03-16 15:44:53]

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

[Peng, 2017-03-16 15:44:58]

https://codereview.chromium.org/2724913002/diff/260001/chrome/browser/chromeo...
File chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc (right):

https://codereview.chromium.org/2724913002/diff/260001/chrome/browser/chromeo...
chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc:73: return
preference->GetValue()->GetBool();
On 2017/03/16 05:02:43, sadrul wrote:
> On 2017/03/14 16:06:52, Peng wrote:
> > On 2017/03/14 01:15:41, sadrul wrote:
> > > Just use preference->GetBoolean(...), instead of DCHECK GetType() and
> > > GetValue()->GetBool()?
> > 
> > I checked the code and found the preference object doesn't have GetBoolean.
> 
> Sorry, yeah, I was referring to using PrefService::GetBoolean() [1], which is
> what the current code uses.
> 
> 1:
>
https://cs.chromium.org/chromium/src/components/prefs/pref_service.h?type=cs&...

Done.

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/eve...
File ui/chromeos/events/event_rewriter.h (right):

https://codereview.chromium.org/2724913002/diff/260001/ui/chromeos/events/eve...
ui/chromeos/events/event_rewriter.h:29: namespace chromeos {
On 2017/03/16 05:02:43, sadrul wrote:
> On 2017/03/14 16:06:52, Peng wrote:
> > On 2017/03/14 01:15:41, sadrul wrote:
> > > It doesn't look like any other code in //ui/chromeos uses namespace
> chromeos.
> > So
> > > let's remove this nested namespace?
> > 
> > Without it, ui::chromeos::EventRewriter will conflict with
ui::EventRewriter.
> 
> Can you rename it EventRewriterChromeOS instead, then?

Done.

https://codereview.chromium.org/2724913002/diff/280001/ui/chromeos/events/eve...
File ui/chromeos/events/event_rewriter.h (right):

https://codereview.chromium.org/2724913002/diff/280001/ui/chromeos/events/eve...
ui/chromeos/events/event_rewriter.h:76: // Retunrs true if the |key_code| and
|flags| have been resgistered for
On 2017/03/16 05:02:43, sadrul wrote:
> *Returns

Done.

[commit-bot: I haz the power, 2017-03-16 16:50:31]

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

[commit-bot: I haz the power, 2017-03-16 16:50:32]

Dry run: This issue passed the CQ dry run.

[Peng, 2017-03-16 17:20:37]

penghuang@chromium.org changed reviewers:
+ oshima@chromium.org, tengs@chromium.org

[Peng, 2017-03-16 17:20:39]

tengs@chromium.org: Please review changes in //ash/sticky_keys/

oshima@chromium.org: Please review changes in //chrome/browser/chromeos and
//ui/chromeos

Hi Tim and Oshima, PTAL. Thanks.

[oshima, 2017-03-16 18:51:08]

*/chromeos/* lgtm

https://codereview.chromium.org/2724913002/diff/300001/chrome/browser/chromeo...
File chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc (right):

https://codereview.chromium.org/2724913002/diff/300001/chrome/browser/chromeo...
chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc:88:
ui::EF_ALT_DOWN | ui::EF_COMMAND_DOWN);
nit: define mask as constexpr. you may inline modifiers

https://codereview.chromium.org/2724913002/diff/300001/ui/chromeos/events/eve...
File ui/chromeos/events/event_rewriter_chromeos.cc (right):

https://codereview.chromium.org/2724913002/diff/300001/ui/chromeos/events/eve...
ui/chromeos/events/event_rewriter_chromeos.cc:437:
GetRemappedKey(kModifierRemappings[i].pref_name, delegate_);
optional: I slightly prefer to handle delegate_ == null case in caller side as
it's done in other places. (other usage seems to be already guarded by delegate_
== null.)

[Peng, 2017-03-16 19:06:31]

penghuang@chromium.org changed reviewers:
+ sky@chromium.org

[Peng, 2017-03-16 19:06:33]

sky@chromium.org: Please review changes in //chrome/browser/ui/* and 
//chrome/browser/extensions/*

Hi Scott, PTAL. Thanks.

[sky, 2017-03-16 20:30:31]

LGTM

[Peng, 2017-03-16 20:33:27]

https://codereview.chromium.org/2724913002/diff/300001/chrome/browser/chromeo...
File chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc (right):

https://codereview.chromium.org/2724913002/diff/300001/chrome/browser/chromeo...
chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc:88:
ui::EF_ALT_DOWN | ui::EF_COMMAND_DOWN);
On 2017/03/16 18:51:08, oshima wrote:
> nit: define mask as constexpr. you may inline modifiers

Done.

https://codereview.chromium.org/2724913002/diff/300001/ui/chromeos/events/eve...
File ui/chromeos/events/event_rewriter_chromeos.cc (right):

https://codereview.chromium.org/2724913002/diff/300001/ui/chromeos/events/eve...
ui/chromeos/events/event_rewriter_chromeos.cc:437:
GetRemappedKey(kModifierRemappings[i].pref_name, delegate_);
On 2017/03/16 18:51:08, oshima wrote:
> optional: I slightly prefer to handle delegate_ == null case in caller side as
> it's done in other places. (other usage seems to be already guarded by
delegate_
> == null.)

GetRemappedKey() is called so many places. It is hard to check (delegate_ ==
null) every place.

[Peng, 2017-03-16 20:36:57]

Hi Wez, Could you please take a look
remoting/client/normalizing_input_filter_cros.cc? Thanks.

On 2017/03/16 20:33:27, Peng wrote:
>
https://codereview.chromium.org/2724913002/diff/300001/chrome/browser/chromeo...
> File chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc (right):
> 
>
https://codereview.chromium.org/2724913002/diff/300001/chrome/browser/chromeo...
> chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc:88:
> ui::EF_ALT_DOWN | ui::EF_COMMAND_DOWN);
> On 2017/03/16 18:51:08, oshima wrote:
> > nit: define mask as constexpr. you may inline modifiers
> 
> Done.
> 
>
https://codereview.chromium.org/2724913002/diff/300001/ui/chromeos/events/eve...
> File ui/chromeos/events/event_rewriter_chromeos.cc (right):
> 
>
https://codereview.chromium.org/2724913002/diff/300001/ui/chromeos/events/eve...
> ui/chromeos/events/event_rewriter_chromeos.cc:437:
> GetRemappedKey(kModifierRemappings[i].pref_name, delegate_);
> On 2017/03/16 18:51:08, oshima wrote:
> > optional: I slightly prefer to handle delegate_ == null case in caller side
as
> > it's done in other places. (other usage seems to be already guarded by
> delegate_
> > == null.)
> 
> GetRemappedKey() is called so many places. It is hard to check (delegate_ ==
> null) every place.

[Peng, 2017-03-16 20:56:29]

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

[commit-bot: I haz the power, 2017-03-16 20:57:09]

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

[Tim Song, 2017-03-16 22:20:26]

lgtm

[Wez, 2017-03-16 22:20:39]

On 2017/03/16 20:36:57, Peng wrote:
> Hi Wez, Could you please take a look
> remoting/client/normalizing_input_filter_cros.cc? Thanks.
> 
> On 2017/03/16 20:33:27, Peng wrote:
> >
>
https://codereview.chromium.org/2724913002/diff/300001/chrome/browser/chromeo...
> > File chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc (right):
> > 
> >
>
https://codereview.chromium.org/2724913002/diff/300001/chrome/browser/chromeo...
> > chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc:88:
> > ui::EF_ALT_DOWN | ui::EF_COMMAND_DOWN);
> > On 2017/03/16 18:51:08, oshima wrote:
> > > nit: define mask as constexpr. you may inline modifiers
> > 
> > Done.
> > 
> >
>
https://codereview.chromium.org/2724913002/diff/300001/ui/chromeos/events/eve...
> > File ui/chromeos/events/event_rewriter_chromeos.cc (right):
> > 
> >
>
https://codereview.chromium.org/2724913002/diff/300001/ui/chromeos/events/eve...
> > ui/chromeos/events/event_rewriter_chromeos.cc:437:
> > GetRemappedKey(kModifierRemappings[i].pref_name, delegate_);
> > On 2017/03/16 18:51:08, oshima wrote:
> > > optional: I slightly prefer to handle delegate_ == null case in caller
side
> as
> > > it's done in other places. (other usage seems to be already guarded by
> > delegate_
> > > == null.)
> > 
> > GetRemappedKey() is called so many places. It is hard to check (delegate_ ==
> > null) every place.

remoting/ LGTM

[commit-bot: I haz the power, 2017-03-16 22:29:18]

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

[commit-bot: I haz the power, 2017-03-16 22:29:19]

Dry run: This issue passed the CQ dry run.

[Peng, 2017-03-16 22:47:30]

The CQ bit was checked by penghuang@chromium.org

[Peng, 2017-03-16 22:47:30]

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

[commit-bot: I haz the power, 2017-03-16 22:48:03]

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-16 22:54:41]

CQ is committing da patch.

Bot data: {"patchset_id": 340001, "attempt_start_ts": 1489704450019610,
"parent_rev": "23587869aa25a885015ceabeeb63f44274f6ae26", "commit_rev":
"e5df28289ab75276db7941bb8c5c76705ac1ef8f"}

[commit-bot: I haz the power, 2017-03-16 22:55:30]

Description was changed from

==========
Move chromeos::EventRewriter to //ui/chromeos/events

chromeos::EventRewriter is responsible for converting F1~F12 to media
function keys. Those conversion is not working in mus+ash. This CL moves
chromeos::EventRewriter to //ui/chromeos/events from
//chrome/browser/chromeos/events. So it can be used by mus as well.

BUG=693180
==========

to

==========
Move chromeos::EventRewriter to //ui/chromeos/events

chromeos::EventRewriter is responsible for converting F1~F12 to media
function keys. Those conversion is not working in mus+ash. This CL moves
chromeos::EventRewriter to //ui/chromeos/events from
//chrome/browser/chromeos/events. So it can be used by mus as well.

BUG=693180

Review-Url: https://codereview.chromium.org/2724913002
Cr-Commit-Position: refs/heads/master@{#457592}
Committed:
https://chromium.googlesource.com/chromium/src/+/e5df28289ab75276db7941bb8c5c...
==========

[commit-bot: I haz the power, 2017-03-16 22:55:32]

Committed patchset #18 (id:340001) as
https://chromium.googlesource.com/chromium/src/+/e5df28289ab75276db7941bb8c5c...