Properly send URL parameters in all requests

Properly send URL parameters in all requests

https://codereview.webrtc.org/2825313002/
added this only to connection URL but it's needed everywhere

BUG=webrtc:7548
Review-Url: https://codereview.webrtc.org/2858493003
Cr-Commit-Position: refs/heads/master@{#17981}
Committed: https://chromium.googlesource.com/external/webrtc/+/9c7e56498b26aa1e88b86b037e7f5a7d5ec3962d

[oprypin_webrtc, 2017-05-02 11:04:07]

Description was changed from

==========
Properly send URL parameters in all requests

BUG=webrtc:7548
==========

to

==========
Properly send URL parameters in all requests

BUG=webrtc:7548
==========

[oprypin_webrtc, 2017-05-02 11:04:07]

oprypin@webrtc.org changed reviewers:
+ kjellander@webrtc.org, magjed@webrtc.org

[oprypin_webrtc, 2017-05-02 11:20:53]

Description was changed from

==========
Properly send URL parameters in all requests

BUG=webrtc:7548
==========

to

==========
Properly send URL parameters in all requests

https://codereview.webrtc.org/2825313002/
added this only to connection URL but it's needed everywhere

BUG=webrtc:7548
==========

[kjellander_webrtc, 2017-05-02 11:52:56]

https://codereview.webrtc.org/2858493003/diff/1/webrtc/examples/androidapp/sr...
File webrtc/examples/androidapp/src/org/appspot/apprtc/WebSocketRTCClient.java
(right):

https://codereview.webrtc.org/2858493003/diff/1/webrtc/examples/androidapp/sr...
webrtc/examples/androidapp/src/org/appspot/apprtc/WebSocketRTCClient.java:145:
if (connectionParameters.urlParameters != null) {
I think the code duplication pattern calls for introducing a private helper
function to call instead.

[oprypin_webrtc, 2017-05-02 12:15:03]

https://codereview.webrtc.org/2858493003/diff/1/webrtc/examples/androidapp/sr...
File webrtc/examples/androidapp/src/org/appspot/apprtc/WebSocketRTCClient.java
(right):

https://codereview.webrtc.org/2858493003/diff/1/webrtc/examples/androidapp/sr...
webrtc/examples/androidapp/src/org/appspot/apprtc/WebSocketRTCClient.java:145:
if (connectionParameters.urlParameters != null) {
On 2017/05/02 11:52:56, kjellander_webrtc wrote:
> I think the code duplication pattern calls for introducing a private helper
> function to call instead.

Done.

[kjellander_webrtc, 2017-05-02 12:40:11]

lgtm

[magjed_webrtc, 2017-05-02 13:14:43]

https://codereview.webrtc.org/2858493003/diff/20001/webrtc/examples/androidap...
File webrtc/examples/androidapp/src/org/appspot/apprtc/WebSocketRTCClient.java
(right):

https://codereview.webrtc.org/2858493003/diff/20001/webrtc/examples/androidap...
webrtc/examples/androidapp/src/org/appspot/apprtc/WebSocketRTCClient.java:135: +
getQueryString(connectionParameters);
It looks like it would be easier to assign "" to connectionParameters instead of
null when you don't have any arguments and remove the if-checks here.

[oprypin_webrtc, 2017-05-02 13:17:38]

https://codereview.webrtc.org/2858493003/diff/20001/webrtc/examples/androidap...
File webrtc/examples/androidapp/src/org/appspot/apprtc/WebSocketRTCClient.java
(right):

https://codereview.webrtc.org/2858493003/diff/20001/webrtc/examples/androidap...
webrtc/examples/androidapp/src/org/appspot/apprtc/WebSocketRTCClient.java:135: +
getQueryString(connectionParameters);
On 2017/05/02 13:14:43, magjed_webrtc wrote:
> It looks like it would be easier to assign "" to connectionParameters instead
of
> null when you don't have any arguments and remove the if-checks here.

It's not just a null check - it's also about the "?". It's arguably nicer to not
require it when passing connectionParameters.

[magjed_webrtc, 2017-05-03 07:58:41]

lgtm

https://codereview.webrtc.org/2858493003/diff/20001/webrtc/examples/androidap...
File webrtc/examples/androidapp/src/org/appspot/apprtc/WebSocketRTCClient.java
(right):

https://codereview.webrtc.org/2858493003/diff/20001/webrtc/examples/androidap...
webrtc/examples/androidapp/src/org/appspot/apprtc/WebSocketRTCClient.java:135: +
getQueryString(connectionParameters);
On 2017/05/02 13:17:37, oprypin_webrtc wrote:
> On 2017/05/02 13:14:43, magjed_webrtc wrote:
> > It looks like it would be easier to assign "" to connectionParameters
instead
> of
> > null when you don't have any arguments and remove the if-checks here.
> 
> It's not just a null check - it's also about the "?". It's arguably nicer to
not
> require it when passing connectionParameters.

Ok, let's do it this way.

[oprypin_webrtc, 2017-05-03 08:31:00]

The CQ bit was checked by oprypin@webrtc.org

[commit-bot: I haz the power, 2017-05-03 08:31:12]

CQ is trying da patch.

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

[commit-bot: I haz the power, 2017-05-03 08:58:40]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1493800260909840,
"parent_rev": "bee4ff8d825eb7d3fabd1a6fd5bb1a0c494f329d", "commit_rev":
"9c7e56498b26aa1e88b86b037e7f5a7d5ec3962d"}

[commit-bot: I haz the power, 2017-05-03 08:58:44]

Description was changed from

==========
Properly send URL parameters in all requests

https://codereview.webrtc.org/2825313002/
added this only to connection URL but it's needed everywhere

BUG=webrtc:7548
==========

to

==========
Properly send URL parameters in all requests

https://codereview.webrtc.org/2825313002/
added this only to connection URL but it's needed everywhere

BUG=webrtc:7548

Review-Url: https://codereview.webrtc.org/2858493003
Cr-Commit-Position: refs/heads/master@{#17981}
Committed:
https://chromium.googlesource.com/external/webrtc/+/9c7e56498b26aa1e88b86b037...
==========

[commit-bot: I haz the power, 2017-05-03 08:58:45]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/external/webrtc/+/9c7e56498b26aa1e88b86b037...