Fix ConnectionToClient to connect stubs only after authentication.

Fix ConnectionToClient to connect stubs only after authentication.

Previously ConnectionToClient required stubs to be created before the
client is authenticated. This is no longer necessary. Fixed it so
set_*_stub() methods can be called only after authentication. Also
removed auth input filters from ClientSession as they are no longer
useful.

BUG=448838
Committed: https://crrev.com/1f8cd500a5335940fee5a8c317d52f090102c131
Cr-Commit-Position: refs/heads/master@{#316299}

[Sergey Ulanov, 2015-02-06 23:14:26]

sergeyu@chromium.org changed reviewers:
+ wez@chromium.org

[Wez, 2015-02-07 00:40:43]

lgtm

https://codereview.chromium.org/902613004/diff/20001/remoting/host/client_ses...
File remoting/host/client_session.cc (left):

https://codereview.chromium.org/902613004/diff/20001/remoting/host/client_ses...
remoting/host/client_session.cc:391:
disable_clipboard_filter_.set_enabled(false);
Why remove these?  The ensure that we don't process further input even if the
connection somehow manages to generate more from the event channel.

https://codereview.chromium.org/902613004/diff/20001/remoting/host/client_ses...
File remoting/host/client_session.h (right):

https://codereview.chromium.org/902613004/diff/20001/remoting/host/client_ses...
remoting/host/client_session.h:239: bool is_authenticated_;
nit: Add a comment to clarify that this is true if the connection has
successfully authenticated, so it's obvious that it's not a flag indicating
whether the connection _should_ bother doing authentication. :)

https://codereview.chromium.org/902613004/diff/20001/remoting/host/client_ses...
File remoting/host/client_session_unittest.cc (left):

https://codereview.chromium.org/902613004/diff/20001/remoting/host/client_ses...
remoting/host/client_session_unittest.cc:346: TEST_F(ClientSessionTest,
ClipboardStubFilter) {
We should have some disable-inputs tests - perhaps just repurpose these?

https://codereview.chromium.org/902613004/diff/20001/remoting/protocol/connec...
File remoting/protocol/connection_to_client.h (right):

https://codereview.chromium.org/902613004/diff/20001/remoting/protocol/connec...
remoting/protocol/connection_to_client.h:95: // Set/get the stubs which will
handle messages we receive from the client.
These methods only set, not get

https://codereview.chromium.org/902613004/diff/20001/remoting/protocol/connec...
remoting/protocol/connection_to_client.h:96: // These stubs must be set in
EventHandler::OnConnectionAuthenticated().
These must be called from ... ?

[Sergey Ulanov, 2015-02-13 19:35:27]

New patchsets have been uploaded after l-g-t-m from wez@chromium.org

[Sergey Ulanov, 2015-02-13 19:40:21]

https://codereview.chromium.org/902613004/diff/20001/remoting/host/client_ses...
File remoting/host/client_session.cc (left):

https://codereview.chromium.org/902613004/diff/20001/remoting/host/client_ses...
remoting/host/client_session.cc:391:
disable_clipboard_filter_.set_enabled(false);
On 2015/02/07 00:40:43, Wez wrote:
> Why remove these?  The ensure that we don't process further input even if the
> connection somehow manages to generate more from the event channel.

InputInjector is deleted 5 lines later so these are not really useful.

https://codereview.chromium.org/902613004/diff/20001/remoting/host/client_ses...
File remoting/host/client_session.h (right):

https://codereview.chromium.org/902613004/diff/20001/remoting/host/client_ses...
remoting/host/client_session.h:239: bool is_authenticated_;
On 2015/02/07 00:40:43, Wez wrote:
> nit: Add a comment to clarify that this is true if the connection has
> successfully authenticated, so it's obvious that it's not a flag indicating
> whether the connection _should_ bother doing authentication. :)

Done.

https://codereview.chromium.org/902613004/diff/20001/remoting/host/client_ses...
File remoting/host/client_session_unittest.cc (left):

https://codereview.chromium.org/902613004/diff/20001/remoting/host/client_ses...
remoting/host/client_session_unittest.cc:346: TEST_F(ClientSessionTest,
ClipboardStubFilter) {
On 2015/02/07 00:40:43, Wez wrote:
> We should have some disable-inputs tests - perhaps just repurpose these?

Done.

https://codereview.chromium.org/902613004/diff/20001/remoting/protocol/connec...
File remoting/protocol/connection_to_client.h (right):

https://codereview.chromium.org/902613004/diff/20001/remoting/protocol/connec...
remoting/protocol/connection_to_client.h:95: // Set/get the stubs which will
handle messages we receive from the client.
On 2015/02/07 00:40:43, Wez wrote:
> These methods only set, not get

Done.

https://codereview.chromium.org/902613004/diff/20001/remoting/protocol/connec...
remoting/protocol/connection_to_client.h:96: // These stubs must be set in
EventHandler::OnConnectionAuthenticated().
On 2015/02/07 00:40:43, Wez wrote:
> These must be called from ... ?

Done.

[Sergey Ulanov, 2015-02-13 19:40:37]

The CQ bit was checked by sergeyu@chromium.org

[commit-bot: I haz the power, 2015-02-13 19:41:21]

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

[commit-bot: I haz the power, 2015-02-13 19:45:10]

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

[commit-bot: I haz the power, 2015-02-13 19:45:12]

Try jobs failed on following builders:
  ios_dbg_simulator on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ng...)
  ios_rel_device_ninja_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)

[Sergey Ulanov, 2015-02-13 20:42:32]

The CQ bit was checked by sergeyu@chromium.org

[commit-bot: I haz the power, 2015-02-13 20:43:17]

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

[commit-bot: I haz the power, 2015-02-13 21:46:02]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-02-13 21:47:49]

Patchset 4 (id:??) landed as
https://crrev.com/1f8cd500a5335940fee5a8c317d52f090102c131
Cr-Commit-Position: refs/heads/master@{#316299}