Now starting AppRTC server up in "developer mode".

Now starting AppRTC server up in "developer mode".

We don't have a way to click on the new join confirmation buttons from
Firefox; therefore we'll start the server in a special mode where the
join buttons are disabled.

We can't do this with an URL parameter since the purpose of the buttons
is to prevent embedding services to get users to auto-join.

B=None
R=kjellander@chromium.org

Committed: https://chromium.googlesource.com/chromium/deps/webrtc/webrtc.DEPS/+/293859

[phoglund_chromium, 2015-01-29 13:00:13]

phoglund@chromium.org changed reviewers:
+ kjellander@chromium.org

[phoglund_chromium, 2015-01-29 13:00:14]

https://codereview.chromium.org/872143005/diff/1/build_apprtc_closure.py
File build_apprtc_closure.py (right):

https://codereview.chromium.org/872143005/diff/1/build_apprtc_closure.py#newc...
build_apprtc_closure.py:30: sys.stdout.write(line)
I realized print put extra newlines in the file. Not a big deal, but this is
prettier.

https://codereview.chromium.org/872143005/diff/1/copy_apprtc.py
File copy_apprtc.py (right):

https://codereview.chromium.org/872143005/diff/1/copy_apprtc.py#newcode34
copy_apprtc.py:34: def _CopyApprtcToTargetDir(target_dir, apprtc_subdir):
The more I think about it, this should really be a build action. I'll consider
making it that later.

[kjellander_chromium, 2015-01-29 13:11:15]

lgtm with a nit and a question.

https://codereview.chromium.org/872143005/diff/1/copy_apprtc.py
File copy_apprtc.py (right):

https://codereview.chromium.org/872143005/diff/1/copy_apprtc.py#newcode34
copy_apprtc.py:34: def _CopyApprtcToTargetDir(target_dir, apprtc_subdir):
On 2015/01/29 13:00:14, phoglund wrote:
> The more I think about it, this should really be a build action. I'll consider
> making it that later.

Yeah, our hooks starts growing out of control here. I guess we should put
together a buildbot script that does all the stuff that's happening in the
hooks, as a separate step (preferably with annotated output so buildbot will
parse it into even more steps).

https://codereview.chromium.org/872143005/diff/1/copy_apprtc.py#newcode38
copy_apprtc.py:38: os.system('rmdir /s /q %s' % target_dir)
Will we catch errors if this fails? It seems like we can check the exit code:
https://docs.python.org/2/library/os.html#os.system
Even if we can't recover, it would make debugging easier if we know a removal
failed (i.e. we print an error message).

https://codereview.chromium.org/872143005/diff/1/copy_apprtc.py#newcode51
copy_apprtc.py:51: def Main():
nit: the Chromium style guide says the main function should be named all
lowercase.

[phoglund_chromium, 2015-01-29 13:46:24]

https://codereview.chromium.org/872143005/diff/1/copy_apprtc.py
File copy_apprtc.py (right):

https://codereview.chromium.org/872143005/diff/1/copy_apprtc.py#newcode34
copy_apprtc.py:34: def _CopyApprtcToTargetDir(target_dir, apprtc_subdir):
On 2015/01/29 13:11:15, kjellander wrote:
> On 2015/01/29 13:00:14, phoglund wrote:
> > The more I think about it, this should really be a build action. I'll
consider
> > making it that later.
> 
> Yeah, our hooks starts growing out of control here. I guess we should put
> together a buildbot script that does all the stuff that's happening in the
> hooks, as a separate step (preferably with annotated output so buildbot will
> parse it into even more steps).

Hm, yeah. The downloading and syncing stuff should happen in the buildbot code
and the "building" (i.e. moving and modifying the apprtc code, building closure)
should be a gyp action. I can try to look into that later.

https://codereview.chromium.org/872143005/diff/1/copy_apprtc.py#newcode38
copy_apprtc.py:38: os.system('rmdir /s /q %s' % target_dir)
On 2015/01/29 13:11:15, kjellander wrote:
> Will we catch errors if this fails? It seems like we can check the exit code:
> https://docs.python.org/2/library/os.html#os.system
> Even if we can't recover, it would make debugging easier if we know a removal
> failed (i.e. we print an error message).

Yeah. Actually I realize I can remove this because I've fixed so that the long
paths never get generated (at least for now). We'll run into trouble anyway in
other places if the paths are allowed to get too long, so this special case
doensn't add anything.

https://codereview.chromium.org/872143005/diff/1/copy_apprtc.py#newcode51
copy_apprtc.py:51: def Main():
On 2015/01/29 13:11:15, kjellander wrote:
> nit: the Chromium style guide says the main function should be named all
> lowercase.

Done.

[phoglund_chromium, 2015-01-29 13:47:22]

Committed patchset #2 (id:20001) manually as r293859 (presubmit successful).