Add a mojo channel for frame messages.

Add a mojo channel for frame messages.

These messages are currently disabled since all input messages
need to be moved over to use mojo. But this enables a set of them
via the command line --mojo-input-messages

BUG=722928
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2884243003
Cr-Commit-Position: refs/heads/master@{#475079}
Committed: https://chromium.googlesource.com/chromium/src/+/9d46ef7db2e30e99230aaf9584723a9dab06a2d3

[dtapuska, 2017-05-16 18:18:15]

Description was changed from

==========
Add a mojo channel for frame messages.

These messages are currently disabled since all input messages
need to be moved over to use mojo. But this enables a set of them
via the command line --mojo-input-messages

BUG=722928
==========

to

==========
Add a mojo channel for frame messages.

These messages are currently disabled since all input messages
need to be moved over to use mojo. But this enables a set of them
via the command line --mojo-input-messages

BUG=722928
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[dtapuska, 2017-05-16 18:19:44]

dtapuska@chromium.org changed reviewers:
+ dcheng@chromium.org

[dtapuska, 2017-05-16 18:19:44]

Daniel you mind taking a pass at this?

[dtapuska, 2017-05-16 18:30:18]

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

[commit-bot: I haz the power, 2017-05-16 18:31:16]

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-05-16 19:05:39]

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

[commit-bot: I haz the power, 2017-05-16 19:05:40]

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

[dcheng, 2017-05-17 04:49:06]

https://codereview.chromium.org/2884243003/diff/1/content/browser/browser_plu...
File content/browser/browser_plugin/browser_plugin_guest.cc (right):

https://codereview.chromium.org/2884243003/diff/1/content/browser/browser_plu...
content/browser/browser_plugin/browser_plugin_guest.cc:894: name,
base::Optional<std::string>());
Or just nullopt

https://codereview.chromium.org/2884243003/diff/1/content/browser/frame_host/...
File content/browser/frame_host/input/legacy_ipc_frame_input_handler.cc (right):

https://codereview.chromium.org/2884243003/diff/1/content/browser/frame_host/...
content/browser/frame_host/input/legacy_ipc_frame_input_handler.cc:34:
SendInput(base::WrapUnique(new InputMsg_SetCompositionFromExistingText(
General nit: writing base::MakeUnique<T>(...) is preferable to
base::WrapUnique(new T(...))

https://codereview.chromium.org/2884243003/diff/1/content/browser/frame_host/...
content/browser/frame_host/input/legacy_ipc_frame_input_handler.cc:66: if
(!value.has_value()) {
Nit: this could be written as !value

https://codereview.chromium.org/2884243003/diff/1/content/browser/web_content...
File content/browser/web_contents/web_contents_impl.cc (right):

https://codereview.chromium.org/2884243003/diff/1/content/browser/web_content...
content/browser/web_contents/web_contents_impl.cc:3021:
base::UTF16ToUTF8(word));
Btw, should we have a bug tracking cleaning this up? We unconditionally convert
to UTF-16 elsewhere, only to have to convert it back here, which seems a bit
unfortunate

https://codereview.chromium.org/2884243003/diff/1/content/renderer/input/fram...
File content/renderer/input/frame_input_handler_impl.cc (right):

https://codereview.chromium.org/2884243003/diff/1/content/renderer/input/fram...
content/renderer/input/frame_input_handler_impl.cc:159:
GetRenderFrame()->OnCopyToFindPboard();
Does this eventually need to thread-hop too?

(I guess it's still using legacy IPC for now, which is why it's OK?)

https://codereview.chromium.org/2884243003/diff/1/content/renderer/input/fram...
File content/renderer/input/frame_input_handler_impl.h (right):

https://codereview.chromium.org/2884243003/diff/1/content/renderer/input/fram...
content/renderer/input/frame_input_handler_impl.h:16: class
FrameInputHandlerImpl
I think this class could probably use a class-level comment to document the
complexities of input handling--as someone who doesn't really touch input, it's
not super obvious why having an input event queue means we need to handle
messages on the compositor thread.

[dtapuska, 2017-05-17 17:08:08]

https://codereview.chromium.org/2884243003/diff/1/content/browser/browser_plu...
File content/browser/browser_plugin/browser_plugin_guest.cc (right):

https://codereview.chromium.org/2884243003/diff/1/content/browser/browser_plu...
content/browser/browser_plugin/browser_plugin_guest.cc:894: name,
base::Optional<std::string>());
On 2017/05/17 04:49:06, dcheng (in AEST) wrote:
> Or just nullopt

Done.

https://codereview.chromium.org/2884243003/diff/1/content/browser/frame_host/...
File content/browser/frame_host/input/legacy_ipc_frame_input_handler.cc (right):

https://codereview.chromium.org/2884243003/diff/1/content/browser/frame_host/...
content/browser/frame_host/input/legacy_ipc_frame_input_handler.cc:34:
SendInput(base::WrapUnique(new InputMsg_SetCompositionFromExistingText(
On 2017/05/17 04:49:06, dcheng (in AEST) wrote:
> General nit: writing base::MakeUnique<T>(...) is preferable to
> base::WrapUnique(new T(...))

Done.

https://codereview.chromium.org/2884243003/diff/1/content/browser/frame_host/...
content/browser/frame_host/input/legacy_ipc_frame_input_handler.cc:66: if
(!value.has_value()) {
On 2017/05/17 04:49:06, dcheng (in AEST) wrote:
> Nit: this could be written as !value

Done.

https://codereview.chromium.org/2884243003/diff/1/content/browser/web_content...
File content/browser/web_contents/web_contents_impl.cc (right):

https://codereview.chromium.org/2884243003/diff/1/content/browser/web_content...
content/browser/web_contents/web_contents_impl.cc:3021:
base::UTF16ToUTF8(word));
On 2017/05/17 04:49:06, dcheng (in AEST) wrote:
> Btw, should we have a bug tracking cleaning this up? We unconditionally
convert
> to UTF-16 elsewhere, only to have to convert it back here, which seems a bit
> unfortunate

Changed interface to support string16 instead.

https://codereview.chromium.org/2884243003/diff/1/content/renderer/input/fram...
File content/renderer/input/frame_input_handler_impl.cc (right):

https://codereview.chromium.org/2884243003/diff/1/content/renderer/input/fram...
content/renderer/input/frame_input_handler_impl.cc:159:
GetRenderFrame()->OnCopyToFindPboard();
On 2017/05/17 04:49:06, dcheng (in AEST) wrote:
> Does this eventually need to thread-hop too?
> 
> (I guess it's still using legacy IPC for now, which is why it's OK?)

Yes it does

https://codereview.chromium.org/2884243003/diff/1/content/renderer/input/fram...
File content/renderer/input/frame_input_handler_impl.h (right):

https://codereview.chromium.org/2884243003/diff/1/content/renderer/input/fram...
content/renderer/input/frame_input_handler_impl.h:16: class
FrameInputHandlerImpl
On 2017/05/17 04:49:06, dcheng (in AEST) wrote:
> I think this class could probably use a class-level comment to document the
> complexities of input handling--as someone who doesn't really touch input,
it's
> not super obvious why having an input event queue means we need to handle
> messages on the compositor thread.

Done.

[dtapuska, 2017-05-19 14:13:36]

On 2017/05/17 17:08:08, dtapuska wrote:
>
https://codereview.chromium.org/2884243003/diff/1/content/browser/browser_plu...
> File content/browser/browser_plugin/browser_plugin_guest.cc (right):
> 
>
https://codereview.chromium.org/2884243003/diff/1/content/browser/browser_plu...
> content/browser/browser_plugin/browser_plugin_guest.cc:894: name,
> base::Optional<std::string>());
> On 2017/05/17 04:49:06, dcheng (in AEST) wrote:
> > Or just nullopt
> 
> Done.
> 
>
https://codereview.chromium.org/2884243003/diff/1/content/browser/frame_host/...
> File content/browser/frame_host/input/legacy_ipc_frame_input_handler.cc
(right):
> 
>
https://codereview.chromium.org/2884243003/diff/1/content/browser/frame_host/...
> content/browser/frame_host/input/legacy_ipc_frame_input_handler.cc:34:
> SendInput(base::WrapUnique(new InputMsg_SetCompositionFromExistingText(
> On 2017/05/17 04:49:06, dcheng (in AEST) wrote:
> > General nit: writing base::MakeUnique<T>(...) is preferable to
> > base::WrapUnique(new T(...))
> 
> Done.
> 
>
https://codereview.chromium.org/2884243003/diff/1/content/browser/frame_host/...
> content/browser/frame_host/input/legacy_ipc_frame_input_handler.cc:66: if
> (!value.has_value()) {
> On 2017/05/17 04:49:06, dcheng (in AEST) wrote:
> > Nit: this could be written as !value
> 
> Done.
> 
>
https://codereview.chromium.org/2884243003/diff/1/content/browser/web_content...
> File content/browser/web_contents/web_contents_impl.cc (right):
> 
>
https://codereview.chromium.org/2884243003/diff/1/content/browser/web_content...
> content/browser/web_contents/web_contents_impl.cc:3021:
> base::UTF16ToUTF8(word));
> On 2017/05/17 04:49:06, dcheng (in AEST) wrote:
> > Btw, should we have a bug tracking cleaning this up? We unconditionally
> convert
> > to UTF-16 elsewhere, only to have to convert it back here, which seems a bit
> > unfortunate
> 
> Changed interface to support string16 instead.
> 
>
https://codereview.chromium.org/2884243003/diff/1/content/renderer/input/fram...
> File content/renderer/input/frame_input_handler_impl.cc (right):
> 
>
https://codereview.chromium.org/2884243003/diff/1/content/renderer/input/fram...
> content/renderer/input/frame_input_handler_impl.cc:159:
> GetRenderFrame()->OnCopyToFindPboard();
> On 2017/05/17 04:49:06, dcheng (in AEST) wrote:
> > Does this eventually need to thread-hop too?
> > 
> > (I guess it's still using legacy IPC for now, which is why it's OK?)
> 
> Yes it does
> 
>
https://codereview.chromium.org/2884243003/diff/1/content/renderer/input/fram...
> File content/renderer/input/frame_input_handler_impl.h (right):
> 
>
https://codereview.chromium.org/2884243003/diff/1/content/renderer/input/fram...
> content/renderer/input/frame_input_handler_impl.h:16: class
> FrameInputHandlerImpl
> On 2017/05/17 04:49:06, dcheng (in AEST) wrote:
> > I think this class could probably use a class-level comment to document the
> > complexities of input handling--as someone who doesn't really touch input,
> it's
> > not super obvious why having an input event queue means we need to handle
> > messages on the compositor thread.
> 
> Done.

dcheng@ ping; I understand you are in AEST timezone. Just let me know how I
should proceed with this if you can't devote any time to it.

[dcheng, 2017-05-19 14:59:34]

https://codereview.chromium.org/2884243003/diff/20001/content/browser/frame_h...
File content/browser/frame_host/render_frame_host_impl.cc (right):

https://codereview.chromium.org/2884243003/diff/20001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_impl.cc:994: if
(legacy_frame_input_handler_.get())
Nit: no.get() in conditional tests

https://codereview.chromium.org/2884243003/diff/20001/content/common/input/in...
File content/common/input/input_handler.mojom (right):

https://codereview.chromium.org/2884243003/diff/20001/content/common/input/in...
content/common/input/input_handler.mojom:11: interface FrameInputHandler
Nit: Add some high-level comments for this interface and its relationship to the
other classes (basically what we talked about in the chat wrt association and
such)

https://codereview.chromium.org/2884243003/diff/20001/content/common/input/in...
content/common/input/input_handler.mojom:13:
SetCompositionFromExistingText(int32 start, int32 end,
array<ui.mojom.CompositionUnderline> underlines);
Add a TODO to import the IPC message comments (or if you're feeling ambitious,
in this CL works too)

https://codereview.chromium.org/2884243003/diff/20001/content/renderer/input/...
File content/renderer/input/frame_input_handler_impl.cc (right):

https://codereview.chromium.org/2884243003/diff/20001/content/renderer/input/...
content/renderer/input/frame_input_handler_impl.cc:73:
base::Bind(&FrameInputHandlerImpl::SetCompositionFromExistingText, this,
It's not clear to me if we should be binding as a scoped_refptr. If the
RenderFrame is destroyed, this object will be kept alive by any pending
callbacks. When they run, that will be a UaF. It seems like WeakPtr might be a
better fit (but let me know if I misunderstood how this works)

[dtapuska, 2017-05-23 16:01:19]

https://codereview.chromium.org/2884243003/diff/20001/content/browser/frame_h...
File content/browser/frame_host/render_frame_host_impl.cc (right):

https://codereview.chromium.org/2884243003/diff/20001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_impl.cc:994: if
(legacy_frame_input_handler_.get())
On 2017/05/19 14:59:33, dcheng wrote:
> Nit: no.get() in conditional tests

Done.

https://codereview.chromium.org/2884243003/diff/20001/content/common/input/in...
File content/common/input/input_handler.mojom (right):

https://codereview.chromium.org/2884243003/diff/20001/content/common/input/in...
content/common/input/input_handler.mojom:11: interface FrameInputHandler
On 2017/05/19 14:59:33, dcheng wrote:
> Nit: Add some high-level comments for this interface and its relationship to
the
> other classes (basically what we talked about in the chat wrt association and
> such)

Done.

https://codereview.chromium.org/2884243003/diff/20001/content/common/input/in...
content/common/input/input_handler.mojom:13:
SetCompositionFromExistingText(int32 start, int32 end,
array<ui.mojom.CompositionUnderline> underlines);
On 2017/05/19 14:59:33, dcheng wrote:
> Add a TODO to import the IPC message comments (or if you're feeling ambitious,
> in this CL works too)

Done.

https://codereview.chromium.org/2884243003/diff/20001/content/renderer/input/...
File content/renderer/input/frame_input_handler_impl.cc (right):

https://codereview.chromium.org/2884243003/diff/20001/content/renderer/input/...
content/renderer/input/frame_input_handler_impl.cc:73:
base::Bind(&FrameInputHandlerImpl::SetCompositionFromExistingText, this,
On 2017/05/19 14:59:33, dcheng wrote:
> It's not clear to me if we should be binding as a scoped_refptr. If the
> RenderFrame is destroyed, this object will be kept alive by any pending
> callbacks. When they run, that will be a UaF. It seems like WeakPtr might be a
> better fit (but let me know if I misunderstood how this works)

So this object is destroyed on the connection handle thread (which generally is
the compositor thread). Having a WeakPtr across the threads isn't safe due to
this cross thread behaviour. However I think I have addressed your concern by
using a WeakPtr inside the FrameInputHandler which is only used on the main
thread. Thoughts?

[dtapuska, 2017-05-24 21:44:12]

On 2017/05/23 16:01:19, dtapuska wrote:
>
https://codereview.chromium.org/2884243003/diff/20001/content/browser/frame_h...
> File content/browser/frame_host/render_frame_host_impl.cc (right):
> 
>
https://codereview.chromium.org/2884243003/diff/20001/content/browser/frame_h...
> content/browser/frame_host/render_frame_host_impl.cc:994: if
> (legacy_frame_input_handler_.get())
> On 2017/05/19 14:59:33, dcheng wrote:
> > Nit: no.get() in conditional tests
> 
> Done.
> 
>
https://codereview.chromium.org/2884243003/diff/20001/content/common/input/in...
> File content/common/input/input_handler.mojom (right):
> 
>
https://codereview.chromium.org/2884243003/diff/20001/content/common/input/in...
> content/common/input/input_handler.mojom:11: interface FrameInputHandler
> On 2017/05/19 14:59:33, dcheng wrote:
> > Nit: Add some high-level comments for this interface and its relationship to
> the
> > other classes (basically what we talked about in the chat wrt association
and
> > such)
> 
> Done.
> 
>
https://codereview.chromium.org/2884243003/diff/20001/content/common/input/in...
> content/common/input/input_handler.mojom:13:
> SetCompositionFromExistingText(int32 start, int32 end,
> array<ui.mojom.CompositionUnderline> underlines);
> On 2017/05/19 14:59:33, dcheng wrote:
> > Add a TODO to import the IPC message comments (or if you're feeling
ambitious,
> > in this CL works too)
> 
> Done.
> 
>
https://codereview.chromium.org/2884243003/diff/20001/content/renderer/input/...
> File content/renderer/input/frame_input_handler_impl.cc (right):
> 
>
https://codereview.chromium.org/2884243003/diff/20001/content/renderer/input/...
> content/renderer/input/frame_input_handler_impl.cc:73:
> base::Bind(&FrameInputHandlerImpl::SetCompositionFromExistingText, this,
> On 2017/05/19 14:59:33, dcheng wrote:
> > It's not clear to me if we should be binding as a scoped_refptr. If the
> > RenderFrame is destroyed, this object will be kept alive by any pending
> > callbacks. When they run, that will be a UaF. It seems like WeakPtr might be
a
> > better fit (but let me know if I misunderstood how this works)
> 
> So this object is destroyed on the connection handle thread (which generally
is
> the compositor thread). Having a WeakPtr across the threads isn't safe due to
> this cross thread behaviour. However I think I have addressed your concern by
> using a WeakPtr inside the FrameInputHandler which is only used on the main
> thread. Thoughts?

dcheng@ ping

[dcheng, 2017-05-25 00:03:58]

LGTM with comments addressed

https://codereview.chromium.org/2884243003/diff/40001/content/common/input/in...
File content/common/input/input_handler.mojom (right):

https://codereview.chromium.org/2884243003/diff/40001/content/common/input/in...
content/common/input/input_handler.mojom:25:
SetCompositionFromExistingText(int32 start, int32 end,
array<ui.mojom.CompositionUnderline> underlines);
Nit: 80 chars

https://codereview.chromium.org/2884243003/diff/40001/content/renderer/input/...
File content/renderer/input/frame_input_handler_impl.cc (right):

https://codereview.chromium.org/2884243003/diff/40001/content/renderer/input/...
content/renderer/input/frame_input_handler_impl.cc:42: main_thread_task_runner_
= base::ThreadTaskRunnerHandle::Get();
Nit: bind this in the initializer list for consistency

https://codereview.chromium.org/2884243003/diff/40001/content/renderer/input/...
content/renderer/input/frame_input_handler_impl.cc:72:
base::Bind(&FrameInputHandlerImpl::SetCompositionFromExistingText, this,
It's safe to vend weak pointers from another thread. However, there's a catch:
if the internal flag object has not been created yet, then GetWeakPtr() is racy.

So to make this all safe:
1. Make this class not refcounted.
2. Add a WeakPtr<FrameInputHandlerImpl> weak_this_.
3. In the constructor body, weak_this_ = weak_ptr_factory_.GetWeakPtr(). Note
that this can't go in the initializer list because the clang plugin enforces
that WeakPtrFactory is the last member.
4. Then there's no need to try dereferencing the weak pointer below; the
callback machinery will do that automatically.

[dtapuska, 2017-05-26 14:02:06]

https://codereview.chromium.org/2884243003/diff/40001/content/common/input/in...
File content/common/input/input_handler.mojom (right):

https://codereview.chromium.org/2884243003/diff/40001/content/common/input/in...
content/common/input/input_handler.mojom:25:
SetCompositionFromExistingText(int32 start, int32 end,
array<ui.mojom.CompositionUnderline> underlines);
On 2017/05/25 00:03:58, dcheng wrote:
> Nit: 80 chars

Done. Although git cl format doesn't clang format mojom files it seems.

https://codereview.chromium.org/2884243003/diff/40001/content/renderer/input/...
File content/renderer/input/frame_input_handler_impl.cc (right):

https://codereview.chromium.org/2884243003/diff/40001/content/renderer/input/...
content/renderer/input/frame_input_handler_impl.cc:42: main_thread_task_runner_
= base::ThreadTaskRunnerHandle::Get();
On 2017/05/25 00:03:58, dcheng wrote:
> Nit: bind this in the initializer list for consistency

Done.

https://codereview.chromium.org/2884243003/diff/40001/content/renderer/input/...
content/renderer/input/frame_input_handler_impl.cc:72:
base::Bind(&FrameInputHandlerImpl::SetCompositionFromExistingText, this,
On 2017/05/25 00:03:58, dcheng wrote:
> It's safe to vend weak pointers from another thread. However, there's a catch:
> if the internal flag object has not been created yet, then GetWeakPtr() is
racy.
> 
> So to make this all safe:
> 1. Make this class not refcounted.
> 2. Add a WeakPtr<FrameInputHandlerImpl> weak_this_.
> 3. In the constructor body, weak_this_ = weak_ptr_factory_.GetWeakPtr(). Note
> that this can't go in the initializer list because the clang plugin enforces
> that WeakPtrFactory is the last member.
> 4. Then there's no need to try dereferencing the weak pointer below; the
> callback machinery will do that automatically.

Done. Although I needed to clear the bindings in the Release on the compositor
thread and then proxy the delete over to the main thread of the object. Either
way it was fundamentally wrong with the ref counted object because the bindings
could end up getting destroyed on the main thread which isn't allowed.

[dtapuska, 2017-05-26 14:22:08]

dtapuska@chromium.org changed reviewers:
+ avi@chromium.org

[dtapuska, 2017-05-26 14:22:09]

avi@chromium.org: Please review changes in 

content/*

[Avi (use Gerrit), 2017-05-26 16:08:40]

lgtm with nits

https://codereview.chromium.org/2884243003/diff/80001/content/browser/android...
File content/browser/android/ime_adapter_android.cc (right):

https://codereview.chromium.org/2884243003/diff/80001/content/browser/android...
content/browser/android/ime_adapter_android.cc:356:
rfh->GetFrameInputHandler()->DeleteSurroundingTextInCodePoints(before,
Multi-line if() body needs {}.

https://codereview.chromium.org/2884243003/diff/80001/content/public/common/c...
File content/public/common/content_switches.cc (right):

https://codereview.chromium.org/2884243003/diff/80001/content/public/common/c...
content/public/common/content_switches.cc:636: const char kMojoInputMessages[] =
"mojo-input-messages";
Indent before "=" to match everything else.

https://codereview.chromium.org/2884243003/diff/80001/content/renderer/input/...
File content/renderer/input/frame_input_handler_impl.cc (right):

https://codereview.chromium.org/2884243003/diff/80001/content/renderer/input/...
content/renderer/input/frame_input_handler_impl.cc:274: // and ack scheme for
this. We should be able to clobber them in the
"an ack scheme"?

https://codereview.chromium.org/2884243003/diff/80001/content/renderer/input/...
content/renderer/input/frame_input_handler_impl.cc:325: // and ack scheme for
this. We should be able to clobber them in the
"an ack scheme"?

[dtapuska, 2017-05-26 16:25:24]

https://codereview.chromium.org/2884243003/diff/80001/content/browser/android...
File content/browser/android/ime_adapter_android.cc (right):

https://codereview.chromium.org/2884243003/diff/80001/content/browser/android...
content/browser/android/ime_adapter_android.cc:356:
rfh->GetFrameInputHandler()->DeleteSurroundingTextInCodePoints(before,
On 2017/05/26 16:08:40, Avi (ping after 24h) wrote:
> Multi-line if() body needs {}.

Done.

https://codereview.chromium.org/2884243003/diff/80001/content/public/common/c...
File content/public/common/content_switches.cc (right):

https://codereview.chromium.org/2884243003/diff/80001/content/public/common/c...
content/public/common/content_switches.cc:636: const char kMojoInputMessages[] =
"mojo-input-messages";
On 2017/05/26 16:08:40, Avi (ping after 24h) wrote:
> Indent before "=" to match everything else.

Done.

https://codereview.chromium.org/2884243003/diff/80001/content/renderer/input/...
File content/renderer/input/frame_input_handler_impl.cc (right):

https://codereview.chromium.org/2884243003/diff/80001/content/renderer/input/...
content/renderer/input/frame_input_handler_impl.cc:274: // and ack scheme for
this. We should be able to clobber them in the
On 2017/05/26 16:08:40, Avi (ping after 24h) wrote:
> "an ack scheme"?

Done.

https://codereview.chromium.org/2884243003/diff/80001/content/renderer/input/...
content/renderer/input/frame_input_handler_impl.cc:325: // and ack scheme for
this. We should be able to clobber them in the
On 2017/05/26 16:08:40, Avi (ping after 24h) wrote:
> "an ack scheme"?

Done.

[dtapuska, 2017-05-26 16:25:59]

The CQ bit was checked by dtapuska@chromium.org

[dtapuska, 2017-05-26 16:25:59]

The patchset sent to the CQ was uploaded after l-g-t-m from dcheng@chromium.org,
avi@chromium.org
Link to the patchset: https://codereview.chromium.org/2884243003/#ps100001
(title: "Fix nits")

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

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-05-26 16:37:14]

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

[commit-bot: I haz the power, 2017-05-26 16:37:15]

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

[dtapuska, 2017-05-26 16:40:17]

The CQ bit was checked by dtapuska@chromium.org

[dtapuska, 2017-05-26 16:40:17]

The patchset sent to the CQ was uploaded after l-g-t-m from avi@chromium.org,
dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2884243003/#ps120001
(title: "Fix presubmit warning I ignored")

[commit-bot: I haz the power, 2017-05-26 16:40:48]

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-05-26 18:18:53]

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

[commit-bot: I haz the power, 2017-05-26 18:18:54]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[dtapuska, 2017-05-26 18:20:38]

The CQ bit was checked by dtapuska@chromium.org

[commit-bot: I haz the power, 2017-05-26 18:21:34]

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-05-26 19:06:12]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1495822838771720,
"parent_rev": "0503f5125ed2c1f6b1cd8001b1f83731493d5ea7", "commit_rev":
"bc252327573f5d689b060bafa9aff1a1dd7e674e"}

[commit-bot: I haz the power, 2017-05-26 19:06:31]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1495822838771720,
"parent_rev": "c5102fb5cee5adc7756df788ade7a7ab169cdd58", "commit_rev":
"9d46ef7db2e30e99230aaf9584723a9dab06a2d3"}

[commit-bot: I haz the power, 2017-05-26 19:06:43]

Description was changed from

==========
Add a mojo channel for frame messages.

These messages are currently disabled since all input messages
need to be moved over to use mojo. But this enables a set of them
via the command line --mojo-input-messages

BUG=722928
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Add a mojo channel for frame messages.

These messages are currently disabled since all input messages
need to be moved over to use mojo. But this enables a set of them
via the command line --mojo-input-messages

BUG=722928
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2884243003
Cr-Commit-Position: refs/heads/master@{#475079}
Committed:
https://chromium.googlesource.com/chromium/src/+/9d46ef7db2e30e99230aaf958472...
==========

[commit-bot: I haz the power, 2017-05-26 19:06:44]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/9d46ef7db2e30e99230aaf958472...