Add QuicStrCat.

Add QuicStrCat.

Merge internal change 142661719.
Merge internal change 142781026.

BUG=

Committed: https://crrev.com/889bd6fc33dd8941005aa74ecceb59ceb4db2684
Cr-Commit-Position: refs/heads/master@{#440641}

[Fan Yang, 2016-12-21 22:05:30]

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

[commit-bot: I haz the power, 2016-12-21 22:06:30]

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, 2016-12-21 22:11:34]

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

[commit-bot: I haz the power, 2016-12-21 22:11:38]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[Fan Yang, 2016-12-22 17:13:28]

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

[commit-bot: I haz the power, 2016-12-22 17:13:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Fan Yang, 2016-12-22 17:38:37]

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

[commit-bot: I haz the power, 2016-12-22 17:38:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Fan Yang, 2016-12-22 17:52:56]

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

[commit-bot: I haz the power, 2016-12-22 17:53:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Fan Yang, 2016-12-22 18:02:04]

Description was changed from

==========
Add QuicStrCat.

Merge internal change 142661719

BUG=
==========

to

==========
Add QuicStrCat.

Merge internal change 142661719

BUG=
==========

[Fan Yang, 2016-12-22 18:02:04]

fayang@chromium.org changed reviewers:
+ rch@chromium.org

[Ryan Hamilton, 2016-12-22 18:17:30]

AWESOME! AWESOME! AWESOME! AWESOME! AWESOME! 

lgtm, mod a few nits

(and a follow up to add a test would be good)

https://codereview.chromium.org/2591143003/diff/60001/net/quic/core/crypto/cr...
File net/quic/core/crypto/crypto_framer.h (right):

https://codereview.chromium.org/2591143003/diff/60001/net/quic/core/crypto/cr...
net/quic/core/crypto/crypto_framer.h:18: #include
"net/quic/platform/api/quic_str_cat.h"
nit: Is this required? I don't think we use StrCat in this .h file.

https://codereview.chromium.org/2591143003/diff/60001/net/quic/core/quic_conn...
File net/quic/core/quic_connection.h (right):

https://codereview.chromium.org/2591143003/diff/60001/net/quic/core/quic_conn...
net/quic/core/quic_connection.h:51: #include
"net/quic/platform/api/quic_str_cat.h"
nit: Is this required? I don't think we use StrCat in this .h file.

https://codereview.chromium.org/2591143003/diff/60001/net/quic/core/quic_pack...
File net/quic/core/quic_packets.cc (right):

https://codereview.chromium.org/2591143003/diff/60001/net/quic/core/quic_pack...
net/quic/core/quic_packets.cc:12: #include
"net/quic/platform/api/quic_str_cat.h"
Is this required? I don't think we use StrCat in this file.

https://codereview.chromium.org/2591143003/diff/60001/net/quic/platform/api/q...
File net/quic/platform/api/quic_str_cat.h (right):

https://codereview.chromium.org/2591143003/diff/60001/net/quic/platform/api/q...
net/quic/platform/api/quic_str_cat.h:14: return
std::move(QuicStrCatImpl(std::forward<const Args&>(args)...));
no need to do this as part of this CL, but it'd be nice to do have a test for
QuicStrCat.

https://codereview.chromium.org/2591143003/diff/60001/net/quic/platform/impl/...
File net/quic/platform/impl/quic_str_cat_impl.h (right):

https://codereview.chromium.org/2591143003/diff/60001/net/quic/platform/impl/...
net/quic/platform/impl/quic_str_cat_impl.h:14: std::ostringstream oss;
nit: I suspect you need to #include something here.

https://codereview.chromium.org/2591143003/diff/60001/net/tools/quic/quic_htt...
File net/tools/quic/quic_http_response_cache.cc (right):

https://codereview.chromium.org/2591143003/diff/60001/net/tools/quic/quic_htt...
net/tools/quic/quic_http_response_cache.cc:341: string request_url =
QuicStrCat(request_host, request_path);
I think we should use GetKey here (and in the internal version).

https://codereview.chromium.org/2591143003/diff/60001/net/tools/quic/quic_htt...
File net/tools/quic/quic_http_response_cache.h (right):

https://codereview.chromium.org/2591143003/diff/60001/net/tools/quic/quic_htt...
net/tools/quic/quic_http_response_cache.h:21: #include
"net/quic/platform/api/quic_str_cat.h"
nit: is this required?

https://codereview.chromium.org/2591143003/diff/60001/net/tools/quic/quic_sim...
File net/tools/quic/quic_simple_server_stream.cc (right):

https://codereview.chromium.org/2591143003/diff/60001/net/tools/quic/quic_sim...
net/tools/quic/quic_simple_server_stream.cc:19: #include
"net/quic/platform/api/quic_str_cat.h"
nit: is this needed?

[commit-bot: I haz the power, 2016-12-22 19:27:21]

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

[commit-bot: I haz the power, 2016-12-22 19:27:22]

Dry run: This issue passed the CQ dry run.

[Fan Yang, 2016-12-22 20:59:27]

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

[commit-bot: I haz the power, 2016-12-22 20:59:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Fan Yang, 2016-12-22 21:02:18]

On 2016/12/22 18:17:30, Ryan Hamilton wrote:
> AWESOME! AWESOME! AWESOME! AWESOME! AWESOME! 
> 
> lgtm, mod a few nits
> 
> (and a follow up to add a test would be good)
> 
>
https://codereview.chromium.org/2591143003/diff/60001/net/quic/core/crypto/cr...
> File net/quic/core/crypto/crypto_framer.h (right):
> 
>
https://codereview.chromium.org/2591143003/diff/60001/net/quic/core/crypto/cr...
> net/quic/core/crypto/crypto_framer.h:18: #include
> "net/quic/platform/api/quic_str_cat.h"
> nit: Is this required? I don't think we use StrCat in this .h file.
> 
>
https://codereview.chromium.org/2591143003/diff/60001/net/quic/core/quic_conn...
> File net/quic/core/quic_connection.h (right):
> 
>
https://codereview.chromium.org/2591143003/diff/60001/net/quic/core/quic_conn...
> net/quic/core/quic_connection.h:51: #include
> "net/quic/platform/api/quic_str_cat.h"
> nit: Is this required? I don't think we use StrCat in this .h file.
> 
>
https://codereview.chromium.org/2591143003/diff/60001/net/quic/core/quic_pack...
> File net/quic/core/quic_packets.cc (right):
> 
>
https://codereview.chromium.org/2591143003/diff/60001/net/quic/core/quic_pack...
> net/quic/core/quic_packets.cc:12: #include
> "net/quic/platform/api/quic_str_cat.h"
> Is this required? I don't think we use StrCat in this file.
> 
>
https://codereview.chromium.org/2591143003/diff/60001/net/quic/platform/api/q...
> File net/quic/platform/api/quic_str_cat.h (right):
> 
>
https://codereview.chromium.org/2591143003/diff/60001/net/quic/platform/api/q...
> net/quic/platform/api/quic_str_cat.h:14: return
> std::move(QuicStrCatImpl(std::forward<const Args&>(args)...));
> no need to do this as part of this CL, but it'd be nice to do have a test for
> QuicStrCat.
> 
>
https://codereview.chromium.org/2591143003/diff/60001/net/quic/platform/impl/...
> File net/quic/platform/impl/quic_str_cat_impl.h (right):
> 
>
https://codereview.chromium.org/2591143003/diff/60001/net/quic/platform/impl/...
> net/quic/platform/impl/quic_str_cat_impl.h:14: std::ostringstream oss;
> nit: I suspect you need to #include something here.
> 
>
https://codereview.chromium.org/2591143003/diff/60001/net/tools/quic/quic_htt...
> File net/tools/quic/quic_http_response_cache.cc (right):
> 
>
https://codereview.chromium.org/2591143003/diff/60001/net/tools/quic/quic_htt...
> net/tools/quic/quic_http_response_cache.cc:341: string request_url =
> QuicStrCat(request_host, request_path);
> I think we should use GetKey here (and in the internal version).
> 
>
https://codereview.chromium.org/2591143003/diff/60001/net/tools/quic/quic_htt...
> File net/tools/quic/quic_http_response_cache.h (right):
> 
>
https://codereview.chromium.org/2591143003/diff/60001/net/tools/quic/quic_htt...
> net/tools/quic/quic_http_response_cache.h:21: #include
> "net/quic/platform/api/quic_str_cat.h"
> nit: is this required?
> 
>
https://codereview.chromium.org/2591143003/diff/60001/net/tools/quic/quic_sim...
> File net/tools/quic/quic_simple_server_stream.cc (right):
> 
>
https://codereview.chromium.org/2591143003/diff/60001/net/tools/quic/quic_sim...
> net/tools/quic/quic_simple_server_stream.cc:19: #include
> "net/quic/platform/api/quic_str_cat.h"
> nit: is this needed?

Thanks a lot, Ryan. PTAL.

[Fan Yang, 2016-12-22 21:03:05]

Description was changed from

==========
Add QuicStrCat.

Merge internal change 142661719

BUG=
==========

to

==========
Add QuicStrCat.

Merge internal change 142661719.
Merge internal change 142781026.

BUG=
==========

[commit-bot: I haz the power, 2016-12-22 21:58:45]

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

[commit-bot: I haz the power, 2016-12-22 21:58:46]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[Ryan Hamilton, 2016-12-22 23:33:53]

LGTM!

https://codereview.chromium.org/2591143003/diff/120001/net/quic/platform/api/...
File net/quic/platform/api/quic_str_cat_test.cc (right):

https://codereview.chromium.org/2591143003/diff/120001/net/quic/platform/api/...
net/quic/platform/api/quic_str_cat_test.cc:21: const uint ui = 4;
I think this is not a valid type on windows.

[Fan Yang, 2016-12-23 15:21:30]

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

[commit-bot: I haz the power, 2016-12-23 15:21:39]

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, 2016-12-23 15:23:31]

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

[commit-bot: I haz the power, 2016-12-23 15:23:31]

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

[Fan Yang, 2016-12-23 15:37:55]

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

[commit-bot: I haz the power, 2016-12-23 15:38:05]

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, 2016-12-23 16:39:17]

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

[commit-bot: I haz the power, 2016-12-23 16:39:18]

Dry run: This issue passed the CQ dry run.

[Fan Yang, 2016-12-23 17:05:29]

The CQ bit was checked by fayang@chromium.org

[Fan Yang, 2016-12-23 17:05:29]

The patchset sent to the CQ was uploaded after l-g-t-m from rch@chromium.org
Link to the patchset: https://codereview.chromium.org/2591143003/#ps150001
(title: "sync")

[commit-bot: I haz the power, 2016-12-23 17:05:40]

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, 2016-12-23 17:09:48]

CQ is committing da patch.

Bot data: {"patchset_id": 150001, "attempt_start_ts": 1482512729532000,
"parent_rev": "0881291d3e28465fd2a88c865835e5a8f3b085e9", "commit_rev":
"c3b556ff72c9a4fb47fc5ec8f6129c87080e56be"}

[commit-bot: I haz the power, 2016-12-23 17:10:19]

Description was changed from

==========
Add QuicStrCat.

Merge internal change 142661719.
Merge internal change 142781026.

BUG=
==========

to

==========
Add QuicStrCat.

Merge internal change 142661719.
Merge internal change 142781026.

BUG=

Review-Url: https://codereview.chromium.org/2591143003
==========

[commit-bot: I haz the power, 2016-12-23 17:10:20]

Committed patchset #9 (id:150001)

[commit-bot: I haz the power, 2016-12-23 17:12:31]

Description was changed from

==========
Add QuicStrCat.

Merge internal change 142661719.
Merge internal change 142781026.

BUG=

Review-Url: https://codereview.chromium.org/2591143003
==========

to

==========
Add QuicStrCat.

Merge internal change 142661719.
Merge internal change 142781026.

BUG=

Committed: https://crrev.com/889bd6fc33dd8941005aa74ecceb59ceb4db2684
Cr-Commit-Position: refs/heads/master@{#440641}
==========

[commit-bot: I haz the power, 2016-12-23 17:12:32]

Patchset 9 (id:??) landed as
https://crrev.com/889bd6fc33dd8941005aa74ecceb59ceb4db2684
Cr-Commit-Position: refs/heads/master@{#440641}