Moving input related classes to the input folder.

Moving input related classes to the input folder.

Review-Url: https://codereview.chromium.org/2903623002
Cr-Commit-Position: refs/heads/master@{#474301}
Committed: https://chromium.googlesource.com/chromium/src/+/7666a289ebf86264fd006e1d9192b32405216094

[nicholss, 2017-05-23 16:52:39]

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

[commit-bot: I haz the power, 2017-05-23 16:53:04]

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-23 17:03:41]

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

[commit-bot: I haz the power, 2017-05-23 17:03:42]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[nicholss, 2017-05-23 20:44:45]

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

[commit-bot: I haz the power, 2017-05-23 20:45:39]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[nicholss, 2017-05-23 20:50:12]

nicholss@chromium.org changed reviewers:
+ yuweih@chromium.org

[Yuwei, 2017-05-23 21:28:26]

Thanks for cleaning this up :)

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/BUILD.gn
File remoting/client/BUILD.gn (right):

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/BUILD.g...
remoting/client/BUILD.gn:59: "//remoting/client/input",
You may need //remoting/client/ui here to use RendererProxy.

Also given that RendererProxy is only used by GestureInterpreter and platform
specific code, you may just move it to client/ and move the client/display
dependency from client/ui to client/.

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/gesture...
File remoting/client/gesture_interpreter.cc (right):

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/gesture...
remoting/client/gesture_interpreter.cc:23: ChromotingSession* input_stub)
I think you'll need to change ChromotingSession to ClientInputInjector and add
new methods there..

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/input/B...
File remoting/client/input/BUILD.gn (right):

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/input/B...
remoting/client/input/BUILD.gn:16: "native_device_keymap_android.cc",
This seems confusing to me... If we are building this for the WebApp then we
will compile all three of the cc files? Why compiling both _android and _ios at
the same time?

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/input/d...
File remoting/client/input/direct_input_strategy.h (right):

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/input/d...
remoting/client/input/direct_input_strategy.h:15: class DirectInputStrategy :
public InputStrategy {
You may need the same prefix here

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/input/i...
File remoting/client/input/input_strategy.h (right):

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/input/i...
remoting/client/input/input_strategy.h:16: class InputStrategy {
Could you name it GestureInputStrategy or whatever to distinguish with the
KeyboardInputStrategy?

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/input/t...
File remoting/client/input/touch_input_scaler.h (left):

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/input/t...
remoting/client/input/touch_input_scaler.h:5: #ifndef
REMOTING_CLIENT_TOUCH_INPUT_SCALER_H_
Please fix the include guard

[nicholss, 2017-05-23 22:00:42]

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

[nicholss, 2017-05-23 22:01:23]

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/BUILD.gn
File remoting/client/BUILD.gn (right):

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/BUILD.g...
remoting/client/BUILD.gn:59: "//remoting/client/input",
On 2017/05/23 21:28:26, Yuwei wrote:
> You may need //remoting/client/ui here to use RendererProxy.
> 
> Also given that RendererProxy is only used by GestureInterpreter and platform
> specific code, you may just move it to client/ and move the client/display
> dependency from client/ui to client/.

I added //remoting/client/display:proxy, see how that feels. I am not 100% sure
yet.

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/gesture...
File remoting/client/gesture_interpreter.cc (right):

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/gesture...
remoting/client/gesture_interpreter.cc:23: ChromotingSession* input_stub)
On 2017/05/23 21:28:26, Yuwei wrote:
> I think you'll need to change ChromotingSession to ClientInputInjector and add
> new methods there..

I did not want to change code for the move. It can be reworked after the bulk of
the files are moved.

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/input/B...
File remoting/client/input/BUILD.gn (right):

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/input/B...
remoting/client/input/BUILD.gn:16: "native_device_keymap_android.cc",
On 2017/05/23 21:28:26, Yuwei wrote:
> This seems confusing to me... If we are building this for the WebApp then we
> will compile all three of the cc files? Why compiling both _android and _ios
at
> the same time?

client had disabled src filtering but it is enabled here by default. *_android
files will only compile for android builds and same with *_ios.

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/input/d...
File remoting/client/input/direct_input_strategy.h (right):

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/input/d...
remoting/client/input/direct_input_strategy.h:15: class DirectInputStrategy :
public InputStrategy {
On 2017/05/23 21:28:26, Yuwei wrote:
> You may need the same prefix here

Explain?

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/input/i...
File remoting/client/input/input_strategy.h (right):

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/input/i...
remoting/client/input/input_strategy.h:16: class InputStrategy {
On 2017/05/23 21:28:26, Yuwei wrote:
> Could you name it GestureInputStrategy or whatever to distinguish with the
> KeyboardInputStrategy?

That is phase3. I did not want to rename anything yet. I will do it in place
next.

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/input/t...
File remoting/client/input/touch_input_scaler.h (left):

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/input/t...
remoting/client/input/touch_input_scaler.h:5: #ifndef
REMOTING_CLIENT_TOUCH_INPUT_SCALER_H_
On 2017/05/23 21:28:26, Yuwei wrote:
> Please fix the include guard

Done.

[commit-bot: I haz the power, 2017-05-23 22:02:20]

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-23 22:20:34]

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

[commit-bot: I haz the power, 2017-05-23 22:20:35]

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

[Yuwei, 2017-05-23 22:20:42]

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/BUILD.gn
File remoting/client/BUILD.gn (right):

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/BUILD.g...
remoting/client/BUILD.gn:59: "//remoting/client/input",
On 2017/05/23 22:01:23, nicholss wrote:
> On 2017/05/23 21:28:26, Yuwei wrote:
> > You may need //remoting/client/ui here to use RendererProxy.
> > 
> > Also given that RendererProxy is only used by GestureInterpreter and
platform
> > specific code, you may just move it to client/ and move the client/display
> > dependency from client/ui to client/.
> 
> I added //remoting/client/display:proxy, see how that feels. I am not 100%
sure
> yet.

TBH I don't feel its good to have a new target to just wrap the RendererProxy...

Turns out you do want /client to depend on /client/ui so that GestureInterpreter
can use FlingAnimation. That's the reason the Android trybot failed...

Then I think I'm fine with keeping RendererProxy in /client/ui

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/gesture...
File remoting/client/gesture_interpreter.cc (right):

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/gesture...
remoting/client/gesture_interpreter.cc:23: ChromotingSession* input_stub)
On 2017/05/23 22:01:23, nicholss wrote:
> On 2017/05/23 21:28:26, Yuwei wrote:
> > I think you'll need to change ChromotingSession to ClientInputInjector and
add
> > new methods there..
> 
> I did not want to change code for the move. It can be reworked after the bulk
of
> the files are moved.

Ah, yes, I think it's fine since GestureInterpreter is in /client

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/input/B...
File remoting/client/input/BUILD.gn (right):

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/input/B...
remoting/client/input/BUILD.gn:16: "native_device_keymap_android.cc",
On 2017/05/23 22:01:23, nicholss wrote:
> On 2017/05/23 21:28:26, Yuwei wrote:
> > This seems confusing to me... If we are building this for the WebApp then we
> > will compile all three of the cc files? Why compiling both _android and _ios
> at
> > the same time?
> 
> client had disabled src filtering but it is enabled here by default. *_android
> files will only compile for android builds and same with *_ios.
> 

Okay... I didn't know there is src filtering :)

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/input/d...
File remoting/client/input/direct_input_strategy.h (right):

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/input/d...
remoting/client/input/direct_input_strategy.h:15: class DirectInputStrategy :
public InputStrategy {
On 2017/05/23 22:01:23, nicholss wrote:
> On 2017/05/23 21:28:26, Yuwei wrote:
> > You may need the same prefix here
> 
> Explain?

I was talking about the GestureInputStrategy. You can ignore this if you are not
going to rename it in this CL.

[nicholss, 2017-05-23 22:46:06]

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

[commit-bot: I haz the power, 2017-05-23 22:47:18]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[nicholss, 2017-05-23 22:52:23]

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/BUILD.gn
File remoting/client/BUILD.gn (right):

https://codereview.chromium.org/2903623002/diff/40001/remoting/client/BUILD.g...
remoting/client/BUILD.gn:59: "//remoting/client/input",
On 2017/05/23 22:20:42, Yuwei wrote:
> On 2017/05/23 22:01:23, nicholss wrote:
> > On 2017/05/23 21:28:26, Yuwei wrote:
> > > You may need //remoting/client/ui here to use RendererProxy.
> > > 
> > > Also given that RendererProxy is only used by GestureInterpreter and
> platform
> > > specific code, you may just move it to client/ and move the client/display
> > > dependency from client/ui to client/.
> > 
> > I added //remoting/client/display:proxy, see how that feels. I am not 100%
> sure
> > yet.
> 
> TBH I don't feel its good to have a new target to just wrap the
RendererProxy...
> 
> Turns out you do want /client to depend on /client/ui so that
GestureInterpreter
> can use FlingAnimation. That's the reason the Android trybot failed...
> 
> Then I think I'm fine with keeping RendererProxy in /client/ui

I moved it to flat display. I don't think it is a UI component, it is part of
the renderer component in my mind.

[Yuwei, 2017-05-23 22:56:36]

LGTM after comment is resolved

https://codereview.chromium.org/2903623002/diff/100001/remoting/ios/display/B...
File remoting/ios/display/BUILD.gn (right):

https://codereview.chromium.org/2903623002/diff/100001/remoting/ios/display/B...
remoting/ios/display/BUILD.gn:35: "//remoting/client/display",
Why is this needed? Is this having a deps on itself?

[commit-bot: I haz the power, 2017-05-23 22:58:28]

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

[commit-bot: I haz the power, 2017-05-23 22:58:29]

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

[Yuwei, 2017-05-23 23:05:56]

https://codereview.chromium.org/2903623002/diff/100001/remoting/client/BUILD.gn
File remoting/client/BUILD.gn (right):

https://codereview.chromium.org/2903623002/diff/100001/remoting/client/BUILD....
remoting/client/BUILD.gn:59: "//remoting/client/display",
pnacl is unhappy again :(

Add deps only if !is_nacl?

[nicholss, 2017-05-23 23:20:47]

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

[commit-bot: I haz the power, 2017-05-23 23:22:21]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[nicholss, 2017-05-23 23:23:17]

I think the build will work now.. roll the dice!

https://codereview.chromium.org/2903623002/diff/100001/remoting/ios/display/B...
File remoting/ios/display/BUILD.gn (right):

https://codereview.chromium.org/2903623002/diff/100001/remoting/ios/display/B...
remoting/ios/display/BUILD.gn:35: "//remoting/client/display",
On 2017/05/23 22:56:35, Yuwei wrote:
> Why is this needed? Is this having a deps on itself?

This is remoting/ios/display/BUILD.gn

the iOS display module uses client/display

[Yuwei, 2017-05-24 01:31:16]

On 2017/05/23 23:23:17, nicholss wrote:
> I think the build will work now.. roll the dice!
> 
>
https://codereview.chromium.org/2903623002/diff/100001/remoting/ios/display/B...
> File remoting/ios/display/BUILD.gn (right):
> 
>
https://codereview.chromium.org/2903623002/diff/100001/remoting/ios/display/B...
> remoting/ios/display/BUILD.gn:35: "//remoting/client/display",
> On 2017/05/23 22:56:35, Yuwei wrote:
> > Why is this needed? Is this having a deps on itself?
> 
> This is remoting/ios/display/BUILD.gn
> 
> the iOS display module uses client/display

Alright.. Got confused by the name :P

[Yuwei, 2017-05-24 01:34:13]

https://codereview.chromium.org/2903623002/diff/120001/remoting/client/BUILD.gn
File remoting/client/BUILD.gn (right):

https://codereview.chromium.org/2903623002/diff/120001/remoting/client/BUILD....
remoting/client/BUILD.gn:81: "//remoting/client/ui",
Is there anything in client/ui that doesn't work with pnacl?

[commit-bot: I haz the power, 2017-05-24 02:48:05]

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

[commit-bot: I haz the power, 2017-05-24 02:48:06]

Dry run: This issue passed the CQ dry run.

[nicholss, 2017-05-24 15:41:33]

Thanks

https://codereview.chromium.org/2903623002/diff/120001/remoting/client/BUILD.gn
File remoting/client/BUILD.gn (right):

https://codereview.chromium.org/2903623002/diff/120001/remoting/client/BUILD....
remoting/client/BUILD.gn:81: "//remoting/client/ui",
On 2017/05/24 01:34:12, Yuwei wrote:
> Is there anything in client/ui that doesn't work with pnacl?

It does not really matter because the gesture_interpreter is the only thing
using it.

[nicholss, 2017-05-24 15:41:42]

The CQ bit was checked by nicholss@chromium.org

[nicholss, 2017-05-24 15:41:42]

The patchset sent to the CQ was uploaded after l-g-t-m from yuweih@chromium.org
Link to the patchset: https://codereview.chromium.org/2903623002/#ps120001
(title: "Hide gesture code from nacl.")

[commit-bot: I haz the power, 2017-05-24 15:42:00]

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-24 15:48:10]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1495640502097200,
"parent_rev": "dcc19bd2a57be365678a394563d58994a58ef555", "commit_rev":
"7666a289ebf86264fd006e1d9192b32405216094"}

[commit-bot: I haz the power, 2017-05-24 15:48:25]

Description was changed from

==========
Moving input related classes to the input folder.
==========

to

==========
Moving input related classes to the input folder.

Review-Url: https://codereview.chromium.org/2903623002
Cr-Commit-Position: refs/heads/master@{#474301}
Committed:
https://chromium.googlesource.com/chromium/src/+/7666a289ebf86264fd006e1d9192...
==========

[commit-bot: I haz the power, 2017-05-24 15:48:26]

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