[dart:io][windows] Use WriteFile instead of _write

This CL changes File::Write on Windows to call directly to WriteFile()
instead of using _write(). This avoids a number of complexities:
1. Don't need to bother with text vs. binary mode.
2. Don't need to check both errno and GetLastError if _write() fails.
3. Don't need to convert to a wchar_t* for console output since we've
   already set the code page to UTF8.

fixes #29101

R=fschneider@google.com

Review-Url: https://codereview.chromium.org/2761673002 .

runtime/bin/file.cc

@@ -76,22 +76,6 @@ void FUNCTION_NAME(File_SetPointer)(Dart_NativeArguments args) {
 }
 
 
-void FUNCTION_NAME(File_SetTranslation)(Dart_NativeArguments args) {
-  File* file = GetFile(args);
-  ASSERT(file != NULL);
-  int64_t translation;
-  Dart_Handle status = Dart_GetNativeIntegerArgument(args, 1, &translation);
-  if (Dart_IsError(status)) {
-    Dart_PropagateError(status);
-  }
-  if ((translation != File::kText) && (translation != File::kBinary)) {
-    Dart_ThrowException(DartUtils::NewDartArgumentError(
-        "The argument must be a FileTranslation mode"));
-  }
-  file->SetTranslation(static_cast<File::DartFileTranslation>(translation));
-}
-
-
 void FUNCTION_NAME(File_Open)(Dart_NativeArguments args) {
   const char* filename =
       DartUtils::GetStringValue(Dart_GetNativeArgument(args, 0));

runtime/bin/file.h

@@ -60,13 +60,6 @@ class File : public ReferenceCounted<File> {
     kDartWriteOnlyAppend = 4
   };
 
-  // These values have to be kept in sync with the values of
-  // _FileTranslation.text and _FileTranslation.binary in file_impl.dart
-  enum DartFileTranslation {
-    kText = 0,
-    kBinary = 1,
-  };
-
   enum Type { kIsFile = 0, kIsDirectory = 1, kIsLink = 2, kDoesNotExist = 3 };
 
   enum Identical { kIdentical = 0, kDifferent = 1, kError = 2 };
@@ -142,10 +135,6 @@ class File : public ReferenceCounted<File> {
   // Set the byte position in the file.
   bool SetPosition(int64_t position);
 
-  // Set the translation mode of the file. This is currently a no-op unless the
-  // file is for a terminal on Windows.
-  void SetTranslation(DartFileTranslation translation);
-
   // Truncate (or extend) the file to the given length in bytes.
   bool Truncate(int64_t length);
 

runtime/bin/file_android.cc

@@ -153,11 +153,6 @@ bool File::SetPosition(int64_t position) {
 }
 
 
-// There is no difference between binary and text translation modes on this
-// platform, so this operation is a no-op.
-void File::SetTranslation(DartFileTranslation translation) {}
-
-
 bool File::Truncate(int64_t length) {
   ASSERT(handle_->fd() >= 0);
   return TEMP_FAILURE_RETRY(ftruncate(handle_->fd(), length) != -1);

runtime/bin/file_fuchsia.cc

@@ -133,11 +133,6 @@ bool File::SetPosition(int64_t position) {
 }
 
 
-// There is no difference between binary and text translation modes on this
-// platform, so this operation is a no-op.
-void File::SetTranslation(DartFileTranslation translation) {}
-
-
 bool File::Truncate(int64_t length) {
   ASSERT(handle_->fd() >= 0);
   return NO_RETRY_EXPECTED(ftruncate(handle_->fd(), length) != -1);

runtime/bin/file_linux.cc

@@ -152,11 +152,6 @@ bool File::SetPosition(int64_t position) {
 }
 
 
-// There is no difference between binary and text translation modes on this
-// platform, so this operation is a no-op.
-void File::SetTranslation(DartFileTranslation translation) {}
-
-
 bool File::Truncate(int64_t length) {
   ASSERT(handle_->fd() >= 0);
   return TEMP_FAILURE_RETRY(ftruncate64(handle_->fd(), length) != -1);

runtime/bin/file_macos.cc

@@ -154,11 +154,6 @@ bool File::SetPosition(int64_t position) {
 }
 
 
-// There is no difference between binary and text translation modes on this
-// platform, so this operation is a no-op.
-void File::SetTranslation(DartFileTranslation translation) {}
-
-
 bool File::Truncate(int64_t length) {
   ASSERT(handle_->fd() >= 0);
   return TEMP_FAILURE_RETRY(ftruncate(handle_->fd(), length)) != -1;

runtime/bin/file_patch.dart

@@ -69,7 +69,6 @@ class _RandomAccessFileOpsImpl extends NativeFieldWrapperClass1
   length() native "File_Length";
   flush() native "File_Flush";
   lock(int lock, int start, int end) native "File_Lock";
-  setTranslation(int translation) native "File_SetTranslation";
 }
 
 class _WatcherPath {

runtime/bin/file_unsupported.cc

@@ -24,12 +24,6 @@ void FUNCTION_NAME(File_SetPointer)(Dart_NativeArguments args) {
 }
 
 
-void FUNCTION_NAME(File_SetTranslation)(Dart_NativeArguments args) {
-  Dart_ThrowException(
-      DartUtils::NewInternalError("File is not supported on this platform"));
-}
-
-
 void FUNCTION_NAME(File_Open)(Dart_NativeArguments args) {
   Dart_ThrowException(
       DartUtils::NewInternalError("File is not supported on this platform"));

runtime/bin/file_win.cc

@@ -26,48 +26,13 @@ namespace bin {
 
 class FileHandle {
  public:
-  explicit FileHandle(int fd)
-      : fd_(fd), real_fd_(-1), binary_(true), is_atty_(false) {}
+  explicit FileHandle(int fd) : fd_(fd) {}
   ~FileHandle() {}
   int fd() const { return fd_; }
   void set_fd(int fd) { fd_ = fd; }
 
-  int real_fd() const {
-    ASSERT(is_atty_);
-    return real_fd_;
-  }
-  void set_real_fd(int real_fd) {
-    ASSERT(is_atty_);
-    real_fd_ = real_fd;
-  }
-
-  bool binary() const { return binary_; }
-  void SetBinary(bool binary) {
-    ASSERT(fd_ >= 0);
-    if (binary) {
-      // Setting the mode to _O_TEXT is needed first to reset _write to allow
-      // an odd number of bytes, which setting to _O_BINARY alone doesn't
-      // accomplish.
-      if (binary != binary_) {
-        _setmode(fd_, _O_TEXT);
-      }
-      _setmode(fd_, _O_BINARY);
-    } else {
-      // Only allow non-binary modes if we're attached to a terminal.
-      ASSERT(_isatty(fd_));
-      _setmode(fd_, _O_WTEXT);
-    }
-    binary_ = binary;
-  }
-
-  bool is_atty() const { return is_atty_; }
-  void set_is_atty(bool is_atty) { is_atty_ = is_atty; }
-
  private:
   int fd_;
-  int real_fd_;
-  bool binary_;
-  bool is_atty_;
 
   DISALLOW_COPY_AND_ASSIGN(FileHandle);
 };
@@ -84,13 +49,7 @@ File::~File() {
 
 void File::Close() {
   ASSERT(handle_->fd() >= 0);
-  int closing_fd;
-  if (handle_->is_atty()) {
-    close(handle_->fd());
-    closing_fd = handle_->real_fd();
-  } else {
-    closing_fd = handle_->fd();
-  }
+  int closing_fd = handle_->fd();
   if ((closing_fd == _fileno(stdout)) || (closing_fd == _fileno(stderr))) {
     int fd = _open("NUL", _O_WRONLY);
     ASSERT(fd >= 0);
@@ -173,15 +132,33 @@ int64_t File::Read(void* buffer, int64_t num_bytes) {
 int64_t File::Write(const void* buffer, int64_t num_bytes) {
   int fd = handle_->fd();
   ASSERT(fd >= 0);
-  if (handle_->binary()) {
-    return _write(fd, buffer, num_bytes);
-  } else {
-    // If we've done _setmode(fd, _O_WTEXT) then _write() expects
-    // a buffer of wchar_t with an even unmber of bytes.
-    Utf8ToWideScope wide(reinterpret_cast<const char*>(buffer), num_bytes);
-    ASSERT((wide.size_in_bytes() % 2) == 0);
-    return _write(fd, wide.wide(), wide.size_in_bytes());
+  HANDLE handle = reinterpret_cast<HANDLE>(_get_osfhandle(fd));
+  DWORD written = 0;
+  BOOL result = WriteFile(handle, buffer, num_bytes, &written, NULL);
+  if (!result) {
+    return -1;
   }
+  DWORD mode;
+  int64_t bytes_written = written;
+  if (GetConsoleMode(handle, &mode)) {
+    // If `handle` is for a console, then `written` may refer to the number of
+    // characters printed to the screen rather than the number of bytes of the
+    // buffer that were actually consumed. To compute the number of bytes that
+    // were actually consumed, we convert the buffer to a wchar_t using the
+    // console's current code page, filling as many characters as were
+    // printed, and then convert that many characters back to the encoding for
+    // the code page, which gives the number of bytes of `buffer` used to
+    // generate the characters that were printed.
+    wchar_t* wide = new wchar_t[written];
+    int cp = GetConsoleOutputCP();
+    MultiByteToWideChar(cp, 0, reinterpret_cast<const char*>(buffer), -1, wide,
+                        written);
+    int buffer_len =
+        WideCharToMultiByte(cp, 0, wide, written, NULL, 0, NULL, NULL);
+    delete wide;
+    bytes_written = buffer_len;
+  }
+  return bytes_written;
 }
 
 
@@ -218,17 +195,6 @@ bool File::SetPosition(int64_t position) {
 }
 
 
-void File::SetTranslation(DartFileTranslation translation) {
-  ASSERT(handle_->fd() >= 0);
-  // Only allow setting the translation mode if we're attached to a terminal.
-  // TODO(zra): Is this restriction needed? Is it already handled correctly
-  // by _write()?
-  if (handle_->is_atty()) {
-    handle_->SetBinary(translation == kBinary);
-  }
-}
-
-
 bool File::Truncate(int64_t length) {
   ASSERT(handle_->fd() >= 0);
   return _chsize_s(handle_->fd(), length) == 0;
@@ -342,22 +308,8 @@ File* File::OpenStdio(int fd) {
     default:
       UNREACHABLE();
   }
-  FileHandle* handle;
-  if (_isatty(stdio_fd)) {
-    // We _dup these fds to avoid different Isoaltes racing on calls to
-    // _setmode() and _write() on the same file descriptor. That is, a call to
-    // _setmode() followed by a call to _write() on the same file descriptor is
-    // not atomic. When the corresponding Dart File object is closed, these
-    // dup'd fds will be closed.
-    int stdio_fd_dup = _dup(stdio_fd);
-    handle = new FileHandle(stdio_fd_dup);
-    handle->set_is_atty(true);
-    handle->set_real_fd(stdio_fd);
-  } else {
-    handle = new FileHandle(stdio_fd);
-  }
-  handle->SetBinary(true);
-  return new File(handle);
+  _setmode(stdio_fd, _O_BINARY);
+  return new File(new FileHandle(stdio_fd));
 }
 
 

runtime/bin/io_natives.cc

@@ -45,7 +45,6 @@ namespace bin {
   V(File_WriteFrom, 4)                                                         \
   V(File_Position, 1)                                                          \
   V(File_SetPosition, 2)                                                       \
-  V(File_SetTranslation, 2)                                                    \
   V(File_Truncate, 2)                                                          \
   V(File_Length, 1)                                                            \
   V(File_LengthFromPath, 1)                                                    \

runtime/bin/process_patch.dart

@@ -257,7 +257,7 @@ class _ProcessImpl extends _ProcessImplNativeWrapper implements Process {
 
     if (mode != ProcessStartMode.DETACHED) {
       // stdin going to process.
-      _stdin = new _StdSocketSink(new _Socket._writePipe());
+      _stdin = new _StdSink(new _Socket._writePipe());
       _stdin._sink._owner = this;
       // stdout coming from process.
       _stdout = new _StdStream(new _Socket._readPipe());
@@ -516,7 +516,7 @@ class _ProcessImpl extends _ProcessImplNativeWrapper implements Process {
   List<String> _environment;
   ProcessStartMode _mode;
   // Private methods of Socket are used by _in, _out, and _err.
-  _StdSocketSink _stdin;
+  _StdSink _stdin;
   _StdStream _stdout;
   _StdStream _stderr;
   Socket _exitHandler;

sdk/lib/io/file_impl.dart

@@ -617,22 +617,6 @@ abstract class _RandomAccessFileOps {
   length();
   flush();
   lock(int lock, int start, int end);
-  setTranslation(int translation);
-}
-
-/**
- * The translation mode of a File.
- *
- * Whether the data written to a file should be interpreted as text
- * or binary data. This distinction is only meaningful on platforms that
- * recognize a difference, in particular on Windows.
- */
-enum _FileTranslation {
-  /// Data should be interpreted as text.
-  text,
-
-  /// Data should be interpreted as binary data.
-  binary,
 }
 
 class _RandomAccessFile implements RandomAccessFile {
@@ -646,12 +630,9 @@ class _RandomAccessFile implements RandomAccessFile {
   _FileResourceInfo _resourceInfo;
   _RandomAccessFileOps _ops;
 
-  _FileTranslation _translation;
-
   _RandomAccessFile(int pointer, this.path) {
     _ops = new _RandomAccessFileOps(pointer);
     _resourceInfo = new _FileResourceInfo(this);
-    _translation = _FileTranslation.binary;
     _maybeConnectHandler();
   }
 
@@ -1052,15 +1033,6 @@ class _RandomAccessFile implements RandomAccessFile {
     }
   }
 
-  _FileTranslation get translation => _translation;
-
-  void set translation(_FileTranslation translation) {
-    if (_translation != translation) {
-      _ops.setTranslation(translation.index);
-      _translation = translation;
-    }
-  }
-
   bool closed = false;
 
   // Calling this function will increase the reference count on the native

sdk/lib/io/stdio.dart

@@ -183,7 +183,7 @@ class Stdin extends _StdStream implements Stream<List<int>> {
  * This class can also be used to check whether `stdout` or `stderr` is
  * connected to a terminal and query some terminal properties.
  */
-class Stdout extends _StdFileSink implements IOSink {
+class Stdout extends _StdSink implements IOSink {
   final int _fd;
   IOSink _nonBlocking;
 
@@ -300,42 +300,30 @@ class _StdConsumer implements StreamConsumer<List<int>> {
   }
 }
 
-class _StdSinkHelper implements IOSink {
+class _StdSink implements IOSink {
   final IOSink _sink;
   final bool _isTranslatable;
 
-  _StdSinkHelper(this._sink, this._isTranslatable);
+  _StdSink(this._sink);
 
   Encoding get encoding => _sink.encoding;
   void set encoding(Encoding encoding) {
     _sink.encoding = encoding;
   }
 
-  void set _translation(_FileTranslation t) {
-    if (_isTranslatable) {
-      _IOSinkImpl sink = _sink;
-      _StdConsumer target = sink._target;
-      target._file.translation = t;
-    }
-  }
-
   void write(object) {
-    _translation = _FileTranslation.text;
     _sink.write(object);
   }
 
   void writeln([object = ""]) {
-    _translation = _FileTranslation.text;
     _sink.writeln(object);
   }
 
   void writeAll(objects, [sep = ""]) {
-    _translation = _FileTranslation.text;
     _sink.writeAll(objects, sep);
   }
 
   void add(List<int> data) {
-    _translation = _FileTranslation.binary;
     _sink.add(data);
   }
 
@@ -344,29 +332,15 @@ class _StdSinkHelper implements IOSink {
   }
 
   void writeCharCode(int charCode) {
-    _translation = _FileTranslation.text;
     _sink.writeCharCode(charCode);
   }
 
-  Future addStream(Stream<List<int>> stream) {
-    _translation = _FileTranslation.binary;
-    return _sink.addStream(stream);
-  }
-
+  Future addStream(Stream<List<int>> stream) => _sink.addStream(stream);
   Future flush() => _sink.flush();
   Future close() => _sink.close();
   Future get done => _sink.done;
 }
 
-class _StdFileSink extends _StdSinkHelper {
-  // The target of `sink` is expected to be a _StdConsumer.
-  _StdFileSink(IOSink sink) : super(sink, true);
-}
-
-class _StdSocketSink extends _StdSinkHelper {
-  _StdSocketSink(IOSink sink) : super(sink, false);
-}
-
 /// The type of object a standard IO stream is attached to.
 class StdioType {
   static const StdioType TERMINAL = const StdioType._("terminal");