Tweaked comments related to mime boundary marker generation.

Tweaked comments related to mime boundary marker generation.

BUG=575779, 575733
TBR=haraken@chromium.org

Committed: https://crrev.com/5398245c0468238ef7cb637140b23edc2a9dd37e
Cr-Commit-Position: refs/heads/master@{#368483}

[Łukasz Anforowicz, 2016-01-08 20:26:02]

Description was changed from

==========
Tweaked comments related to mime boundary marker generation.

BUG=575779 575733
==========

to

==========
Tweaked comments related to mime boundary marker generation.

BUG=575779, 575733
==========

[Łukasz Anforowicz, 2016-01-08 21:31:12]

lukasza@chromium.org changed reviewers:
+ rsleevi@chromium.org

[Łukasz Anforowicz, 2016-01-08 21:31:12]

rsleevi@, could you please take a look?  (this CL is a follow-up to
https://codereview.chromium.org/1547593002/#msg29)

https://codereview.chromium.org/1573623002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/network/FormDataEncoder.cpp (left):

https://codereview.chromium.org/1573623002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/network/FormDataEncoder.cpp:102: //
TODO(lukasza): Reuse net::GenerateMimeMultipartBoundary instead.
Hmmm... ideally we would have only 1 piece of code for generating the mime
boundary.  I understand that we don't want Blink to be able to issue network
requests without going through the browser process + because of repo history
don't even allow blink -> base dependencies.  OTOH, it sounds that some blink ->
base dependencies will be eventually allowed and therefore maybe we just need to
move GenerateMimeMultipartBoundary into base?

WDYT?  Should I remove the "reuse" TODO or leave it here to see what happens
after Blink moves around and starts allowing some Chromium dependencies (like
base)?

[Ryan Sleevi, 2016-01-08 21:34:22]

LGTM

https://codereview.chromium.org/1573623002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/network/FormDataEncoder.cpp (left):

https://codereview.chromium.org/1573623002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/network/FormDataEncoder.cpp:102: //
TODO(lukasza): Reuse net::GenerateMimeMultipartBoundary instead.
On 2016/01/08 21:31:12, Łukasz Anforowicz wrote:
> Hmmm... ideally we would have only 1 piece of code for generating the mime
> boundary.  I understand that we don't want Blink to be able to issue network
> requests without going through the browser process + because of repo history
> don't even allow blink -> base dependencies.  OTOH, it sounds that some blink
->
> base dependencies will be eventually allowed and therefore maybe we just need
to
> move GenerateMimeMultipartBoundary into base?

No, we don't want base/ to be the dumping ground for what is unquestionably a
network layer dependency :)

The story of Blink and //net is even more complicated than the //base bit,
that's why I objected to the TODO - because it's not at all clear that this is a
good / right direction, but the TODO when read naturally suggests this is the
"decided decision"

> WDYT?  Should I remove the "reuse" TODO or leave it here to see what happens
> after Blink moves around and starts allowing some Chromium dependencies (like
> base)?

Wholly remove.

[Łukasz Anforowicz, 2016-01-08 21:37:32]

Description was changed from

==========
Tweaked comments related to mime boundary marker generation.

BUG=575779, 575733
==========

to

==========
Tweaked comments related to mime boundary marker generation.

BUG=575779, 575733
TBR=haraken@chromium.org
==========

[Łukasz Anforowicz, 2016-01-08 21:39:37]

lukasza@chromium.org changed reviewers:
+ haraken@chromium.org

[Łukasz Anforowicz, 2016-01-08 21:39:37]

haraken@ - I'll just TBR further comment tweaks in
	third_party/WebKit/Source/platform/network/FormDataEncoder.cpp that were
suggested by rsleevi@.

rsleevi@ - thanks for reviewing (and catching the issues remaining in the
previous CL).

[Łukasz Anforowicz, 2016-01-08 21:39:45]

The CQ bit was checked by lukasza@chromium.org

[commit-bot: I haz the power, 2016-01-08 21:40:17]

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

[commit-bot: I haz the power, 2016-01-09 00:44:54]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-01-09 00:46:22]

Description was changed from

==========
Tweaked comments related to mime boundary marker generation.

BUG=575779, 575733
TBR=haraken@chromium.org
==========

to

==========
Tweaked comments related to mime boundary marker generation.

BUG=575779, 575733
TBR=haraken@chromium.org

Committed: https://crrev.com/5398245c0468238ef7cb637140b23edc2a9dd37e
Cr-Commit-Position: refs/heads/master@{#368483}
==========

[commit-bot: I haz the power, 2016-01-09 00:46:23]

Patchset 3 (id:??) landed as
https://crrev.com/5398245c0468238ef7cb637140b23edc2a9dd37e
Cr-Commit-Position: refs/heads/master@{#368483}

[haraken, 2016-01-09 04:14:38]

LGTM