Split _uriEncode into JS and VM versions.

Split _uriEncode into JS and VM versions.

R=iposva@google.com, sra@google.com

Committed: https://github.com/dart-lang/sdk/commit/b13607d24751c7cfdd6138b0566ac538dca8cb49

[Lasse Reichstein Nielsen, 2015-11-04 10:15:53]

lrn@google.com changed reviewers:
+ iposva@google.com, sra@google.com

[Lasse Reichstein Nielsen, 2015-11-04 10:15:54]

https://codereview.chromium.org/1416373007/diff/1/runtime/lib/uri_patch.dart
File runtime/lib/uri_patch.dart (right):

https://codereview.chromium.org/1416373007/diff/1/runtime/lib/uri_patch.dart#...
runtime/lib/uri_patch.dart:45: }
New code replacing the RegExp test with more precision.

https://codereview.chromium.org/1416373007/diff/1/sdk/lib/_internal/js_runtim...
File sdk/lib/_internal/js_runtime/lib/core_patch.dart (right):

https://codereview.chromium.org/1416373007/diff/1/sdk/lib/_internal/js_runtim...
sdk/lib/_internal/js_runtime/lib/core_patch.dart:547: if (identical(encoding,
UTF8) &&
I added the UTF8 requirement to ensure that the encoding is compatible with the
original string. Accepting LATIN1 and ASCII works too, if you want it, but most
cases will be UTF8.

https://codereview.chromium.org/1416373007/diff/1/sdk/lib/_internal/js_runtim...
sdk/lib/_internal/js_runtime/lib/core_patch.dart:554: }
Consider lining this function unless you know the compiler always does it
anyway.

https://codereview.chromium.org/1416373007/diff/1/sdk/lib/core/uri.dart
File sdk/lib/core/uri.dart (right):

https://codereview.chromium.org/1416373007/diff/1/sdk/lib/core/uri.dart#newco...
sdk/lib/core/uri.dart:2257: bool spaceToPlus);
I made the parameters non-optional. That's what I generally do to all private
functions, leaving optional parameters only on public functions.
(I should fix _uriDecode as well).

[sra1, 2015-11-04 18:38:33]

lgtm

https://codereview.chromium.org/1416373007/diff/1/sdk/lib/_internal/js_runtim...
File sdk/lib/_internal/js_runtime/lib/core_patch.dart (right):

https://codereview.chromium.org/1416373007/diff/1/sdk/lib/_internal/js_runtim...
sdk/lib/_internal/js_runtime/lib/core_patch.dart:554: }
On 2015/11/04 10:15:53, Lasse Reichstein Nielsen wrote:
> Consider lining this function unless you know the compiler always does it
> anyway.

Inline it, it is not inlined at the moment.

[Ivan Posva, 2015-11-05 06:24:57]

LGTM -ip

https://codereview.chromium.org/1416373007/diff/1/runtime/lib/uri_patch.dart
File runtime/lib/uri_patch.dart (right):

https://codereview.chromium.org/1416373007/diff/1/runtime/lib/uri_patch.dart#...
runtime/lib/uri_patch.dart:33: if (identical(encoding, UTF8) ||
Is it really identical that you are looking for or is equal good enough?

[Lasse Reichstein Nielsen, 2015-11-05 08:06:49]

https://codereview.chromium.org/1416373007/diff/1/runtime/lib/uri_patch.dart
File runtime/lib/uri_patch.dart (right):

https://codereview.chromium.org/1416373007/diff/1/runtime/lib/uri_patch.dart#...
runtime/lib/uri_patch.dart:33: if (identical(encoding, UTF8) ||
It's the same thing since the UTF8 codec object doesn't override operator==. I
just like to be explicit.

[Lasse Reichstein Nielsen, 2015-11-05 12:00:53]

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