Remove hack http client and use the one in dart:io

Remove hack http client and use the one in dart:io

R=asiva@google.com

Committed: https://code.google.com/p/dart/source/detail?r=23496

[siva, 2013-05-31 21:02:02]

lgtm

https://codereview.chromium.org/15736023/diff/3001/runtime/bin/builtin.dart
File runtime/bin/builtin.dart (right):

https://codereview.chromium.org/15736023/diff/3001/runtime/bin/builtin.dart#n...
runtime/bin/builtin.dart:6: import 'dart:io';
I hope we won't get into any bootstrapping issues with somebody
adding an import 'http:....' inside sdk/lib/io/io.dart

https://codereview.chromium.org/15736023/diff/3001/runtime/bin/dartutils.cc
File runtime/bin/dartutils.cc (right):

https://codereview.chromium.org/15736023/diff/3001/runtime/bin/dartutils.cc#n...
runtime/bin/dartutils.cc:266: ASSERT(buffer_len != NULL);
ASSERT(!Dart_HasLivePorts());

https://codereview.chromium.org/15736023/diff/3001/runtime/bin/dartutils.cc#n...
runtime/bin/dartutils.cc:280: }
Can the get field of _httpRequestStatusString  be done only under
if (responseCode != 200) {
   ...
}

https://codereview.chromium.org/15736023/diff/3001/runtime/bin/dartutils.cc#n...
runtime/bin/dartutils.cc:285: }
This get field code could be moved below the
if (responseCode != 200) check.

https://codereview.chromium.org/15736023/diff/3001/runtime/bin/dartutils.cc#n...
runtime/bin/dartutils.cc:401: }
Are these functions PathFromUri, DomainFromUri still needed?

https://codereview.chromium.org/15736023/diff/3001/runtime/bin/dartutils.cc#n...
runtime/bin/dartutils.cc:538: 
blank lines?

https://codereview.chromium.org/15736023/diff/3001/runtime/bin/dartutils.cc#n...
runtime/bin/dartutils.cc:539: Dart_Handle DartUtils::LoadScriptHttp(const char*
script_uri,
I think this should probably be
(Dart_Handle script_uri, Dart_Handle builtin_lib)

see comment in previous CL.

[Cutch, 2013-05-31 21:38:34]

https://codereview.chromium.org/15736023/diff/3001/runtime/bin/dartutils.cc
File runtime/bin/dartutils.cc (right):

https://codereview.chromium.org/15736023/diff/3001/runtime/bin/dartutils.cc#n...
runtime/bin/dartutils.cc:266: ASSERT(buffer_len != NULL);
On 2013/05/31 21:02:02, siva wrote:
> ASSERT(!Dart_HasLivePorts());

Done.

https://codereview.chromium.org/15736023/diff/3001/runtime/bin/dartutils.cc#n...
runtime/bin/dartutils.cc:280: }
On 2013/05/31 21:02:02, siva wrote:
> Can the get field of _httpRequestStatusString  be done only under
> if (responseCode != 200) {
>    ...
> }

Done.

https://codereview.chromium.org/15736023/diff/3001/runtime/bin/dartutils.cc#n...
runtime/bin/dartutils.cc:285: }
On 2013/05/31 21:02:02, siva wrote:
> This get field code could be moved below the
> if (responseCode != 200) check.

Done.

https://codereview.chromium.org/15736023/diff/3001/runtime/bin/dartutils.cc#n...
runtime/bin/dartutils.cc:401: }
On 2013/05/31 21:02:02, siva wrote:
> Are these functions PathFromUri, DomainFromUri still needed?

Removed.

https://codereview.chromium.org/15736023/diff/3001/runtime/bin/dartutils.cc#n...
runtime/bin/dartutils.cc:538: 
On 2013/05/31 21:02:02, siva wrote:
> blank lines?

Done.

https://codereview.chromium.org/15736023/diff/3001/runtime/bin/dartutils.cc#n...
runtime/bin/dartutils.cc:539: Dart_Handle DartUtils::LoadScriptHttp(const char*
script_uri,
On 2013/05/31 21:02:02, siva wrote:
> I think this should probably be
> (Dart_Handle script_uri, Dart_Handle builtin_lib)
> 
> see comment in previous CL.

Done. Will finish the cleanup in the next CL.

[Cutch, 2013-05-31 22:05:09]

Committed patchset #3 manually as r23496 (presubmit successful).

[Søren Gjesse, 2013-06-03 09:20:00]

The handling of the client response body stream needs to be fixed.

https://codereview.chromium.org/15736023/diff/9001/runtime/bin/builtin.dart
File runtime/bin/builtin.dart (right):

https://codereview.chromium.org/15736023/diff/9001/runtime/bin/builtin.dart#n...
runtime/bin/builtin.dart:35: response.listen((List<int> responseData) {
This is not sufficient to handle the response body. onData can be called several
times followed by onDone. You can use the HttpBodyHandler to do the work:

client.getUrl(...)
    .then((HttpClientRequest response) => response.close())
    .then(HttpBodyHandler.processResponse)
    .then((HttpClientResponseBody body) {
      if (body.type == "text") {
        // The mime type indicated text and body.body is the decoded string
      } else if (body.type == "binary") {
        // The mime type indicated binary and body.body is List<int> of the
whole body.
      } else {
        // Other body types.
      }
    });

https://codereview.chromium.org/15736023/diff/9001/runtime/bin/dartutils.cc
File runtime/bin/dartutils.cc (right):

https://codereview.chromium.org/15736023/diff/9001/runtime/bin/dartutils.cc#n...
runtime/bin/dartutils.cc:329: bufferLen);
By using the HttpBodyHandler the supported charsets (currently only us-ascii,
iso-8859-1 and utf-8) of text/ mimetype will be decoded into a string in Dart
HTTP code.

[Cutch, 2013-06-03 17:38:31]

On 2013/06/03 09:20:00, Søren Gjesse wrote:
> The handling of the client response body stream needs to be fixed.
> 
> https://codereview.chromium.org/15736023/diff/9001/runtime/bin/builtin.dart
> File runtime/bin/builtin.dart (right):
> 
>
https://codereview.chromium.org/15736023/diff/9001/runtime/bin/builtin.dart#n...
> runtime/bin/builtin.dart:35: response.listen((List<int> responseData) {
> This is not sufficient to handle the response body. onData can be called
several
> times followed by onDone. You can use the HttpBodyHandler to do the work:
> 
> client.getUrl(...)
>     .then((HttpClientRequest response) => response.close())
>     .then(HttpBodyHandler.processResponse)
>     .then((HttpClientResponseBody body) {
>       if (body.type == "text") {
>         // The mime type indicated text and body.body is the decoded string
>       } else if (body.type == "binary") {
>         // The mime type indicated binary and body.body is List<int> of the
> whole body.
>       } else {
>         // Other body types.
>       }
>     });
> 
> https://codereview.chromium.org/15736023/diff/9001/runtime/bin/dartutils.cc
> File runtime/bin/dartutils.cc (right):
> 
>
https://codereview.chromium.org/15736023/diff/9001/runtime/bin/dartutils.cc#n...
> runtime/bin/dartutils.cc:329: bufferLen);
> By using the HttpBodyHandler the supported charsets (currently only us-ascii,
> iso-8859-1 and utf-8) of text/ mimetype will be decoded into a string in Dart
> HTTP code.

https://codereview.chromium.org/16305011