Build remoting for PNaCl

Build remoting for PNaCl

BUG=276739
R=jamiewalch@chromium.org, rsleevi@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=271351

[Jamie, 2014-04-14 22:23:41]

https://codereview.chromium.org/234023003/diff/60001/remoting/client/plugin/c...
File remoting/client/plugin/chromoting_instance.cc (left):

https://codereview.chromium.org/234023003/diff/60001/remoting/client/plugin/c...
remoting/client/plugin/chromoting_instance.cc:283:
net::EnableSSLServerSockets();
Did you intend to remove this? If so, please add something to the CL description
explaining why it's no longer needed, even for non-NaCl builds.

https://codereview.chromium.org/234023003/diff/60001/remoting/client/plugin/c...
File remoting/client/plugin/chromoting_instance.cc (right):

https://codereview.chromium.org/234023003/diff/60001/remoting/client/plugin/c...
remoting/client/plugin/chromoting_instance.cc:225:
jingle_glue::JingleThreadWrapper::EnsureForCurrentMessageLoop();
Is this run on the same thread as it would have been in its previous locations
(ConnectWithConfig, et al)?

https://codereview.chromium.org/234023003/diff/60001/remoting/client/plugin/c...
remoting/client/plugin/chromoting_instance.cc:290: // work in NaCl at the moment
so the check fails in NaCl builds.
Do we want this to work for non-extensions, or is it a necessary compromise
because the API isn't implemented? If the latter, please add a TODO and a bug to
track support for PB_URLUtil_Dev in NaCl.

https://codereview.chromium.org/234023003/diff/60001/remoting/client/plugin/c...
remoting/client/plugin/chromoting_instance.cc:367: return;
Is this required for this CL, or an unrelated bug-fix?

https://codereview.chromium.org/234023003/diff/60001/remoting/protocol/ssl_hm...
File remoting/protocol/ssl_hmac_channel_authenticator.cc (right):

https://codereview.chromium.org/234023003/diff/60001/remoting/protocol/ssl_hm...
remoting/protocol/ssl_hmac_channel_authenticator.cc:68: // Client plugin uses
only client SSL sockets.
s/Client plugin/NaCl client/?

https://codereview.chromium.org/234023003/diff/60001/remoting/protocol/ssl_hm...
File remoting/protocol/ssl_hmac_channel_authenticator.h (left):

https://codereview.chromium.org/234023003/diff/60001/remoting/protocol/ssl_hm...
remoting/protocol/ssl_hmac_channel_authenticator.h:92:
scoped_ptr<net::CertVerifier> cert_verifier_;
Part of this CL, or could it be separate?

https://codereview.chromium.org/234023003/diff/60001/remoting/remoting_webapp...
File remoting/remoting_webapp.gypi (right):

https://codereview.chromium.org/234023003/diff/60001/remoting/remoting_webapp...
remoting/remoting_webapp.gypi:16: 'client_plugin_type%': 'native',
Is this used anywhere?

[Sergey Ulanov, 2014-04-14 23:16:51]

https://codereview.chromium.org/234023003/diff/60001/remoting/client/plugin/c...
File remoting/client/plugin/chromoting_instance.cc (left):

https://codereview.chromium.org/234023003/diff/60001/remoting/client/plugin/c...
remoting/client/plugin/chromoting_instance.cc:283:
net::EnableSSLServerSockets();
On 2014/04/14 22:23:42, Jamie wrote:
> Did you intend to remove this? If so, please add something to the CL
description
> explaining why it's no longer needed, even for non-NaCl builds.

Yes, this is intentional. There is no reason to initialize server ssl sockets on
the client because we always use client SSL sockets there. I'll move this to a
separate CL.

https://codereview.chromium.org/234023003/diff/60001/remoting/client/plugin/c...
File remoting/client/plugin/chromoting_instance.cc (right):

https://codereview.chromium.org/234023003/diff/60001/remoting/client/plugin/c...
remoting/client/plugin/chromoting_instance.cc:225:
jingle_glue::JingleThreadWrapper::EnsureForCurrentMessageLoop();
On 2014/04/14 22:23:42, Jamie wrote:
> Is this run on the same thread as it would have been in its previous locations
> (ConnectWithConfig, et al)?

Yes. All code in this file is executed on the main plugin thread.

https://codereview.chromium.org/234023003/diff/60001/remoting/client/plugin/c...
remoting/client/plugin/chromoting_instance.cc:290: // work in NaCl at the moment
so the check fails in NaCl builds.
On 2014/04/14 22:23:42, Jamie wrote:
> Do we want this to work for non-extensions, or is it a necessary compromise
> because the API isn't implemented? If the latter, please add a TODO and a bug
to
> track support for PB_URLUtil_Dev in NaCl.

We don't need this check for NaCl plugin. Pepper plugin is built in into chrome,
so we need to prevent it from being used from drive-by web pages. NaCl plugin is
packaged with the webapp, so other pages/apps will not have access to it, and
there is no reason to verify it here. Beside that NaCl plugins don't get any
permissions that the page doesn't already have (while the built-in plugin get
access to sockets API implicitly), so there is no reason to check the origin
here even if the plugin was somehow available to drive-by web.

https://codereview.chromium.org/234023003/diff/60001/remoting/client/plugin/c...
remoting/client/plugin/chromoting_instance.cc:367: return;
On 2014/04/14 22:23:42, Jamie wrote:
> Is this required for this CL, or an unrelated bug-fix?

It fixes a bug that affects only NaCl build (I didn't try to figure out why it
doesn't crash the pepper plugin).

https://codereview.chromium.org/234023003/diff/60001/remoting/protocol/ssl_hm...
File remoting/protocol/ssl_hmac_channel_authenticator.cc (right):

https://codereview.chromium.org/234023003/diff/60001/remoting/protocol/ssl_hm...
remoting/protocol/ssl_hmac_channel_authenticator.cc:68: // Client plugin uses
only client SSL sockets.
On 2014/04/14 22:23:42, Jamie wrote:
> s/Client plugin/NaCl client/?

The comment is correct. Updated it to make it clear why this code needs to be
disabled for NaCl.

https://codereview.chromium.org/234023003/diff/60001/remoting/protocol/ssl_hm...
File remoting/protocol/ssl_hmac_channel_authenticator.h (left):

https://codereview.chromium.org/234023003/diff/60001/remoting/protocol/ssl_hm...
remoting/protocol/ssl_hmac_channel_authenticator.h:92:
scoped_ptr<net::CertVerifier> cert_verifier_;
On 2014/04/14 22:23:42, Jamie wrote:
> Part of this CL, or could it be separate?

Can be separate: https://codereview.chromium.org/238133002

https://codereview.chromium.org/234023003/diff/60001/remoting/remoting_webapp...
File remoting/remoting_webapp.gypi (right):

https://codereview.chromium.org/234023003/diff/60001/remoting/remoting_webapp...
remoting/remoting_webapp.gypi:16: 'client_plugin_type%': 'native',
On 2014/04/14 22:23:42, Jamie wrote:
> Is this used anywhere?

Done.

[Jamie, 2014-04-14 23:43:35]

lgtm

[Sergey Ulanov, 2014-04-16 17:46:41]

The CQ bit was checked by sergeyu@chromium.org

[commit-bot: I haz the power, 2014-04-16 17:47:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/234023003/110001

[commit-bot: I haz the power, 2014-04-16 18:53:06]

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

[commit-bot: I haz the power, 2014-04-16 18:53:07]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_rel

[Sergey Ulanov, 2014-04-16 19:02:24]

The CQ bit was checked by sergeyu@chromium.org

[commit-bot: I haz the power, 2014-04-16 19:02:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/234023003/120001

[Sergey Ulanov, 2014-04-16 19:08:09]

The CQ bit was unchecked by sergeyu@chromium.org

[Sergey Ulanov, 2014-04-29 21:06:21]

The CQ bit was checked by sergeyu@chromium.org

[commit-bot: I haz the power, 2014-04-29 21:07:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/234023003/170001

[commit-bot: I haz the power, 2014-04-29 22:12:06]

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

[commit-bot: I haz the power, 2014-04-29 22:12:06]

Try jobs failed on following builders:
  win_chromium_rel on tryserver.chromium

[Sergey Ulanov, 2014-04-29 23:07:32]

The CQ bit was checked by sergeyu@chromium.org

[commit-bot: I haz the power, 2014-04-29 23:10:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/234023003/170001

[commit-bot: I haz the power, 2014-04-30 00:25:53]

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

[commit-bot: I haz the power, 2014-04-30 00:25:54]

Try jobs failed on following builders:
  win_chromium_rel on tryserver.chromium

[Sergey Ulanov, 2014-04-30 01:34:54]

Committed patchset #6 manually as r267030 (presubmit successful).

[Sergey Ulanov, 2014-04-30 05:12:08]

reverted in r267064 due to intermittent compilation issues:
http://build.chromium.org/p/chromium/builders/Linux/builds/49401/steps/compil...

[Sergey Ulanov, 2014-05-08 17:47:05]

The CQ bit was checked by sergeyu@chromium.org

[commit-bot: I haz the power, 2014-05-08 17:52:00]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sergeyu@chromium.org/234023003/170001

[commit-bot: I haz the power, 2014-05-08 22:29:33]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-08 22:38:01]

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

[commit-bot: I haz the power, 2014-05-08 22:38:01]

Try jobs failed on following builders:
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)

[Sergey Ulanov, 2014-05-16 17:13:22]

The CQ bit was checked by sergeyu@chromium.org

[commit-bot: I haz the power, 2014-05-16 17:14:08]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sergeyu@chromium.org/234023003/190001

[commit-bot: I haz the power, 2014-05-16 20:48:49]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)

[Sergey Ulanov, 2014-05-16 23:05:34]

+rsleevi in TBR for the trivial change in net/net_nacl.gyp

[Sergey Ulanov, 2014-05-16 23:05:39]

The CQ bit was checked by sergeyu@chromium.org

[Ryan Sleevi, 2014-05-16 23:11:06]

lgtm

[commit-bot: I haz the power, 2014-05-16 23:32:19]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sergeyu@chromium.org/234023003/210001

[commit-bot: I haz the power, 2014-05-17 11:40:20]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)

[commit-bot: I haz the power, 2014-05-17 21:27:03]

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

[commit-bot: I haz the power, 2014-05-17 21:27:04]

Try jobs failed on following builders:
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)

[Sergey Ulanov, 2014-05-19 06:23:10]

Committed patchset #8 manually as r271343 (presubmit successful).

[Sergey Ulanov, 2014-05-19 06:48:55]

The CQ bit was checked by sergeyu@chromium.org

[commit-bot: I haz the power, 2014-05-19 06:49:08]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sergeyu@chromium.org/234023003/230001

[Sergey Ulanov, 2014-05-19 08:11:37]

Committed patchset #9 manually as r271351 (presubmit successful).