Redo escaping in GN.

This makes Windows escaping more correct and handles Posix shell characters better as well. Now there are completely different codepaths for Windows and Posix escaping.

I removed JSON escaping since this is no longer needed now that we no longer write GYP files.

I no longer have a separate SHELL and NINJA modes. Instead, I have pure NINJA mode for writing file names for Ninja to interpret, and NINKA_FORK mode which does NINJA escaping plus the correct platform-specific rules for the method Ninja uses for running build steps on the current system.

Includes used to always be quoted ("-I../..") which was ugly. Now they're not quoted unless necessary (which is almost never). If it requires quoting, it will do -I"foo bar" which looks a bit odd but saves a bunch of special casing in the output code.

Previously defines weren't quoted at all. Now they work like include dirs.

Removed the convert_slashes flag on PathOutput which is no longer used.

Removed some backslash special-casing in the unit tests on Windows. These are no longer necessary since we changed path output on Windows to use forward-slashes.

Fix base's StackString on GCC. Previously this was only used on Windows-specific code.

Fix mesa Windows GN build.

BUG=358764
R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/311733002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@275174 0039d316-1c4b-4281-b951-d872f2087c98

16 files changed, 304 insertions, 264 deletions

diff --git a/base/containers/stack_container.h b/base/containers/stack_container.h
index f0106d7..87fa036 100644
--- a/base/containers/stack_container.h
+++ b/base/containers/stack_container.h
@@ -90,6 +90,13 @@ class StackAllocator : public std::allocator<T> {
       : source_(NULL) {
   }
 
+  // This constructor must exist. It creates a default allocator that doesn't
+  // actually have a stack buffer. glibc's std::string() will compare the
+  // current allocator against the default-constructed allocator, so this
+  // should be fast.
+  StackAllocator() : source_(NULL) {
+  }
+
   explicit StackAllocator(Source* source) : source_(source) {
   }
 
diff --git a/third_party/mesa/BUILD.gn b/third_party/mesa/BUILD.gn
index 7c56f0e..753b46a 100644
--- a/third_party/mesa/BUILD.gn
+++ b/third_party/mesa/BUILD.gn
@@ -716,7 +716,7 @@ static_library("mesa") {
 
   if (is_win) {
     # Because we're building as a static library
-    defines += [ "_GLAPI_NO_EXPORTS" ]
+    defines = [ "_GLAPI_NO_EXPORTS" ]
   }
 
   deps = [
diff --git a/tools/gn/escape.cc b/tools/gn/escape.cc
index 1658a8a..7028a38 100644
--- a/tools/gn/escape.cc
+++ b/tools/gn/escape.cc
@@ -5,10 +5,11 @@
 #include "tools/gn/escape.h"
 
 #include "base/containers/stack_container.h"
+#include "base/logging.h"
 
 namespace {
 
-// A "1" in this lookup table means that char is valid in the shell.
+// A "1" in this lookup table means that char is valid in the Posix shell.
 const char kShellValid[0x80] = {
 // 00-1f: all are invalid
     0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
@@ -26,70 +27,150 @@ const char kShellValid[0x80] = {
 //  p  q  r  s  t  u  v  w  x  y  z  {  |  }  ~
     1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 1, 0, 0 };
 
+// Append one character to the given string, escaping it for Ninja.
+//
+// Ninja's escaping rules are very simple. We always escape colons even
+// though they're OK in many places, in case the resulting string is used on
+// the left-hand-side of a rule.
 template<typename DestString>
-void EscapeStringToString(const base::StringPiece& str,
-                          const EscapeOptions& options,
-                          DestString* dest,
-                          bool* needed_quoting) {
-  bool used_quotes = false;
+inline void NinjaEscapeChar(char ch, DestString* dest) {
+  if (ch == '$' || ch == ' ' || ch == ':')
+    dest->push_back('$');
+  dest->push_back(ch);
+}
 
-  for (size_t i = 0; i < str.size(); i++) {
-    if (str[i] == '$' && (options.mode & ESCAPE_NINJA)) {
-      // Escape dollars signs since ninja treats these specially. If we're also
-      // escaping for the shell, we need to backslash-escape that again.
-      if (options.mode & ESCAPE_SHELL)
-        dest->push_back('\\');
-      dest->push_back('$');
-      dest->push_back('$');
-    } else if (str[i] == ' ') {
-      if (options.mode & ESCAPE_NINJA) {
-        // For Ninja just escape spaces with $.
-        dest->push_back('$');
+template<typename DestString>
+void EscapeStringToString_Ninja(const base::StringPiece& str,
+                                const EscapeOptions& options,
+                                DestString* dest,
+                                bool* needed_quoting) {
+  for (size_t i = 0; i < str.size(); i++)
+    NinjaEscapeChar(str[i], dest);
+  if (needed_quoting)
+    *needed_quoting = false;
+}
+
+// Escape for CommandLineToArgvW and additionally escape Ninja characters.
+//
+// The basic algorithm is if the string doesn't contain any parse-affecting
+// characters, don't do anything (other than the Ninja processing). If it does,
+// quote the string, and backslash-escape all quotes and backslashes.
+// See:
+//   http://blogs.msdn.com/b/twistylittlepassagesallalike/archive/2011/04/23/everyone-quotes-arguments-the-wrong-way.aspx
+//   http://blogs.msdn.com/b/oldnewthing/archive/2010/09/17/10063629.aspx
+template<typename DestString>
+void EscapeStringToString_WindowsNinjaFork(const base::StringPiece& str,
+                                           const EscapeOptions& options,
+                                           DestString* dest,
+                                           bool* needed_quoting) {
+  // We assume we don't have any whitespace chars that aren't spaces.
+  DCHECK(str.find_first_of("\r\n\v\t") == std::string::npos);
+
+  if (str.find_first_of(" \"") == std::string::npos) {
+    // Simple case, don't quote.
+    EscapeStringToString_Ninja(str, options, dest, needed_quoting);
+  } else {
+    if (!options.inhibit_quoting)
+      dest->push_back('"');
+
+    for (size_t i = 0; i < str.size(); i++) {
+      // Count backslashes in case they're followed by a quote.
+      size_t backslash_count = 0;
+      while (i < str.size() && str[i] == '\\') {
+        i++;
+        backslash_count++;
       }
-      if (options.mode & ESCAPE_SHELL) {
-        // For the shell, quote the whole string.
-        if (needed_quoting)
-          *needed_quoting = true;
-        if (!options.inhibit_quoting) {
-          if (!used_quotes) {
-            used_quotes = true;
-            dest->insert(dest->begin(), '"');
-          }
-        }
+      if (i == str.size()) {
+        // Backslashes at end of string. Backslash-escape all of them since
+        // they'll be followed by a quote.
+        dest->append(backslash_count * 2, '\\');
+      } else if (str[i] == '"') {
+        // 0 or more backslashes followed by a quote. Backslash-escape the
+        // backslashes, then backslash-escape the quote.
+        dest->append(backslash_count * 2 + 1, '\\');
+        dest->push_back('"');
+      } else {
+        // Non-special Windows character, just escape for Ninja. Also, add any
+        // backslashes we read previously, these are literals.
+        dest->append(backslash_count, '\\');
+        NinjaEscapeChar(str[i], dest);
       }
-      dest->push_back(' ');
-    } else if (str[i] == '\'' && (options.mode & ESCAPE_JSON)) {
-      dest->push_back('\\');
-      dest->push_back('\'');
-#if defined(OS_WIN)
-    } else if (str[i] == '/' && options.convert_slashes) {
-      // Convert slashes on Windows if requested.
-      dest->push_back('\\');
-#else
-    } else if (str[i] == '\\' && (options.mode & ESCAPE_SHELL)) {
-      // For non-Windows shell, escape backslashes.
-      dest->push_back('\\');
-      dest->push_back('\\');
-#endif
-    } else if (str[i] == '\\' && (options.mode & ESCAPE_JSON)) {
-      dest->push_back('\\');
+    }
+
+    if (!options.inhibit_quoting)
+      dest->push_back('"');
+    if (needed_quoting)
+      *needed_quoting = true;
+  }
+}
+
+template<typename DestString>
+void EscapeStringToString_PosixNinjaFork(const base::StringPiece& str,
+                                         const EscapeOptions& options,
+                                         DestString* dest,
+                                         bool* needed_quoting) {
+  for (size_t i = 0; i < str.size(); i++) {
+    if (str[i] == '$' || str[i] == ' ') {
+      // Space and $ are special to both Ninja and the shell. '$' escape for
+      // Ninja, then backslash-escape for the shell.
       dest->push_back('\\');
-    } else if (str[i] == ':' && (options.mode & ESCAPE_NINJA)) {
+      dest->push_back('$');
+      dest->push_back(str[i]);
+    } else if (str[i] == ':') {
+      // Colon is the only other Ninja special char, which is not special to
+      // the shell.
       dest->push_back('$');
       dest->push_back(':');
-    } else if ((options.mode & ESCAPE_SHELL) &&
-               (static_cast<unsigned>(str[i]) >= 0x80 ||
-                !kShellValid[static_cast<int>(str[i])])) {
+    } else if (static_cast<unsigned>(str[i]) >= 0x80 ||
+               !kShellValid[static_cast<int>(str[i])]) {
       // All other invalid shell chars get backslash-escaped.
       dest->push_back('\\');
       dest->push_back(str[i]);
     } else {
+      // Everything else is a literal.
       dest->push_back(str[i]);
     }
   }
+}
 
-  if (used_quotes)
-    dest->push_back('"');
+template<typename DestString>
+void EscapeStringToString(const base::StringPiece& str,
+                          const EscapeOptions& options,
+                          DestString* dest,
+                          bool* needed_quoting) {
+  switch (options.mode) {
+    case ESCAPE_NONE:
+      dest->append(str.data(), str.size());
+      break;
+    case ESCAPE_NINJA:
+      EscapeStringToString_Ninja(str, options, dest, needed_quoting);
+      break;
+    case ESCAPE_NINJA_COMMAND:
+      switch (options.platform) {
+        case ESCAPE_PLATFORM_CURRENT:
+#if defined(OS_WIN)
+          EscapeStringToString_WindowsNinjaFork(str, options, dest,
+                                                needed_quoting);
+#else
+          EscapeStringToString_PosixNinjaFork(str, options, dest,
+                                              needed_quoting);
+#endif
+          break;
+        case ESCAPE_PLATFORM_WIN:
+          EscapeStringToString_WindowsNinjaFork(str, options, dest,
+                                                needed_quoting);
+          break;
+        case ESCAPE_PLATFORM_POSIX:
+          EscapeStringToString_PosixNinjaFork(str, options, dest,
+                                              needed_quoting);
+          break;
+        default:
+          NOTREACHED();
+      }
+      break;
+    default:
+      NOTREACHED();
+  }
 }
 
 }  // namespace
@@ -106,10 +187,8 @@ std::string EscapeString(const base::StringPiece& str,
 void EscapeStringToStream(std::ostream& out,
                           const base::StringPiece& str,
                           const EscapeOptions& options) {
-  // Escape to a stack buffer and then write out to the stream.
-  base::StackVector<char, 256> result;
-  result->reserve(str.size() + 4);  // Guess we'll add a couple of extra chars.
-  EscapeStringToString(str, options, &result.container(), NULL);
-  if (!result->empty())
-    out.write(&result[0], result->size());
+  base::StackString<256> escaped;
+  EscapeStringToString(str, options, &escaped.container(), NULL);
+  if (!escaped->empty())
+    out.write(escaped->data(), escaped->size());
 }
diff --git a/tools/gn/escape.h b/tools/gn/escape.h
index 1e4fa68..6f8cf9c 100644
--- a/tools/gn/escape.h
+++ b/tools/gn/escape.h
@@ -14,35 +14,42 @@ enum EscapingMode {
   ESCAPE_NONE,
 
   // Ninja string escaping.
-  ESCAPE_NINJA = 1,
+  ESCAPE_NINJA,
 
-  // Shell string escaping.
-  ESCAPE_SHELL = 2,
+  // For writing commands to ninja files.
+  ESCAPE_NINJA_COMMAND,
+};
 
-  // For writing shell commands to ninja files.
-  ESCAPE_NINJA_SHELL = ESCAPE_NINJA | ESCAPE_SHELL,
+enum EscapingPlatform {
+  // Do escaping for the current platform.
+  ESCAPE_PLATFORM_CURRENT,
 
-  ESCAPE_JSON = 4,
+  // Force escaping for the given platform.
+  ESCAPE_PLATFORM_POSIX,
+  ESCAPE_PLATFORM_WIN,
 };
 
 struct EscapeOptions {
   EscapeOptions()
       : mode(ESCAPE_NONE),
-        convert_slashes(false),
+        platform(ESCAPE_PLATFORM_CURRENT),
         inhibit_quoting(false) {
   }
 
   EscapingMode mode;
 
-  // When set, converts forward-slashes to system-specific path separators.
-  bool convert_slashes;
+  // Controls how "fork" escaping is done. You will generally want to keep the
+  // default "current" platform.
+  EscapingPlatform platform;
 
   // When the escaping mode is ESCAPE_SHELL, the escaper will normally put
   // quotes around things with spaces. If this value is set to true, we'll
   // disable the quoting feature and just add the spaces.
   //
   // This mode is for when quoting is done at some higher-level. Defaults to
-  // false.
+  // false. Note that Windows has strange behavior where the meaning of the
+  // backslashes changes according to if it is followed by a quote. The
+  // escaping rules assume that a double-quote will be appended to the result.
   bool inhibit_quoting;
 };
 
diff --git a/tools/gn/escape_unittest.cc b/tools/gn/escape_unittest.cc
index 9d0cf7a..1a79a78 100644
--- a/tools/gn/escape_unittest.cc
+++ b/tools/gn/escape_unittest.cc
@@ -12,69 +12,41 @@ TEST(Escape, Ninja) {
   EXPECT_EQ("asdf$:$ \"$$\\bar", result);
 }
 
-TEST(Escape, Shell) {
+TEST(Escape, WindowsCommand) {
   EscapeOptions opts;
-  opts.mode = ESCAPE_SHELL;
-  std::string result = EscapeString("asdf: \"$\\bar", opts, NULL);
-#if defined(OS_WIN)
-  // Windows shell doesn't escape backslashes, but it does backslash-escape
-  // quotes.
-  EXPECT_EQ("\"asdf: \\\"\\$\\bar\"", result);
-#else
-  EXPECT_EQ("\"asdf: \\\"\\$\\\\bar\"", result);
-#endif
-
-  // Some more generic shell chars.
-  result = EscapeString("a_;<*b", opts, NULL);
-  EXPECT_EQ("a_\\;\\<\\*b", result);
-}
+  opts.mode = ESCAPE_NINJA_COMMAND;
+  opts.platform = ESCAPE_PLATFORM_WIN;
 
-TEST(Escape, NinjaShell) {
-  EscapeOptions opts;
-  opts.mode = ESCAPE_NINJA_SHELL;
+  // Regular string is passed, even if it has backslashes.
+  EXPECT_EQ("foo\\bar", EscapeString("foo\\bar", opts, NULL));
 
-  // When escaping for Ninja and the shell, we would escape a $, then escape
-  // the backslash again.
-  std::string result = EscapeString("a$b", opts, NULL);
-  EXPECT_EQ("a\\$$b", result);
-}
+  // Spaces means the string is quoted, normal backslahes untouched.
+  bool needs_quoting = false;
+  EXPECT_EQ("\"foo\\$ bar\"", EscapeString("foo\\ bar", opts, &needs_quoting));
+  EXPECT_TRUE(needs_quoting);
 
-TEST(Escape, UsedQuotes) {
-  EscapeOptions shell_options;
-  shell_options.mode = ESCAPE_SHELL;
+  // Inhibit quoting.
+  needs_quoting = false;
+  opts.inhibit_quoting = true;
+  EXPECT_EQ("foo\\$ bar", EscapeString("foo\\ bar", opts, &needs_quoting));
+  EXPECT_TRUE(needs_quoting);
+  opts.inhibit_quoting = false;
 
-  EscapeOptions ninja_options;
-  ninja_options.mode = ESCAPE_NINJA;
+  // Backslashes at the end of the string get escaped.
+  EXPECT_EQ("\"foo$ bar\\\\\\\\\"", EscapeString("foo bar\\\\", opts, NULL));
 
-  EscapeOptions ninja_shell_options;
-  ninja_shell_options.mode = ESCAPE_NINJA_SHELL;
-
-  // Shell escaping with quoting inhibited.
-  bool used_quotes = false;
-  shell_options.inhibit_quoting = true;
-  EXPECT_EQ("foo bar", EscapeString("foo bar", shell_options, &used_quotes));
-  EXPECT_TRUE(used_quotes);
-
-  // Shell escaping with regular quoting.
-  used_quotes = false;
-  shell_options.inhibit_quoting = false;
-  EXPECT_EQ("\"foo bar\"",
-            EscapeString("foo bar", shell_options, &used_quotes));
-  EXPECT_TRUE(used_quotes);
+  // Backslashes preceeding quotes are escaped, and the quote is escaped.
+  EXPECT_EQ("\"foo\\\\\\\"$ bar\"", EscapeString("foo\\\" bar", opts, NULL));
+}
 
-  // Ninja shell escaping should be the same.
-  used_quotes = false;
-  EXPECT_EQ("\"foo$ bar\"",
-            EscapeString("foo bar", ninja_shell_options, &used_quotes));
-  EXPECT_TRUE(used_quotes);
+TEST(Escape, PosixCommand) {
+  EscapeOptions opts;
+  opts.mode = ESCAPE_NINJA_COMMAND;
+  opts.platform = ESCAPE_PLATFORM_POSIX;
 
-  // Ninja escaping shouldn't use quoting.
-  used_quotes = false;
-  EXPECT_EQ("foo$ bar", EscapeString("foo bar", ninja_options, &used_quotes));
-  EXPECT_FALSE(used_quotes);
+  // : and $ ninja escaped with $. Then Shell-escape backslashes and quotes.
+  EXPECT_EQ("a$:\\$ \\\"\\$$\\\\b", EscapeString("a: \"$\\b", opts, NULL));
 
-  // Used quotes not reset if it's already true.
-  used_quotes = true;
-  EscapeString("foo", ninja_options, &used_quotes);
-  EXPECT_TRUE(used_quotes);
+  // Some more generic shell chars.
+  EXPECT_EQ("a_\\;\\<\\*b", EscapeString("a_;<*b", opts, NULL));
 }
diff --git a/tools/gn/file_template.cc b/tools/gn/file_template.cc
index b626482..b00eb39 100644
--- a/tools/gn/file_template.cc
+++ b/tools/gn/file_template.cc
@@ -166,7 +166,7 @@ void FileTemplate::ApplyString(const std::string& str,
 
 void FileTemplate::WriteWithNinjaExpansions(std::ostream& out) const {
   EscapeOptions escape_options;
-  escape_options.mode = ESCAPE_NINJA_SHELL;
+  escape_options.mode = ESCAPE_NINJA_COMMAND;
   escape_options.inhibit_quoting = true;
 
   for (size_t template_i = 0;
@@ -184,8 +184,10 @@ void FileTemplate::WriteWithNinjaExpansions(std::ostream& out) const {
     for (size_t subrange_i = 0; subrange_i < t.container().size();
          subrange_i++) {
       if (t[subrange_i].type == Subrange::LITERAL) {
+        bool cur_needs_quoting = false;
         item_str.append(EscapeString(t[subrange_i].literal, escape_options,
-                                     &needs_quoting));
+                                     &cur_needs_quoting));
+        needs_quoting |= cur_needs_quoting;
       } else {
         // Don't escape this since we need to preserve the $.
         item_str.append("${");
diff --git a/tools/gn/file_template_unittest.cc b/tools/gn/file_template_unittest.cc
index cd5611c..8e3bf40 100644
--- a/tools/gn/file_template_unittest.cc
+++ b/tools/gn/file_template_unittest.cc
@@ -63,11 +63,19 @@ TEST(FileTemplate, NinjaExpansions) {
   std::ostringstream out;
   t.WriteWithNinjaExpansions(out);
 
+#if defined(OS_WIN)
   // The third argument should get quoted since it contains a space, and the
   // embedded quotes should get shell escaped.
   EXPECT_EQ(
       " -i ${source} \"--out=foo$ bar\\\"${source_name_part}\\\".o\" \"\"",
       out.str());
+#else
+  // On Posix we don't need to quote the whole thing and can escape all
+  // individual bad chars.
+  EXPECT_EQ(
+      " -i ${source} --out=foo\\$ bar\\\"${source_name_part}\\\".o \"\"",
+      out.str());
+#endif
 }
 
 TEST(FileTemplate, NinjaVariables) {
@@ -80,7 +88,7 @@ TEST(FileTemplate, NinjaVariables) {
 
   std::ostringstream out;
   EscapeOptions options;
-  options.mode = ESCAPE_NINJA_SHELL;
+  options.mode = ESCAPE_NINJA_COMMAND;
   t.WriteNinjaVariablesForSubstitution(out, "../../foo/bar.txt", options);
 
   // Just the variables used above should be written.
diff --git a/tools/gn/ninja_action_target_writer.cc b/tools/gn/ninja_action_target_writer.cc
index 1b71fce..27bd8a4 100644
--- a/tools/gn/ninja_action_target_writer.cc
+++ b/tools/gn/ninja_action_target_writer.cc
@@ -16,7 +16,7 @@ NinjaActionTargetWriter::NinjaActionTargetWriter(const Target* target,
     : NinjaTargetWriter(target, toolchain, out),
       path_output_no_escaping_(
           target->settings()->build_settings()->build_dir(),
-          ESCAPE_NONE, false) {
+          ESCAPE_NONE) {
 }
 
 NinjaActionTargetWriter::~NinjaActionTargetWriter() {
@@ -144,8 +144,7 @@ void NinjaActionTargetWriter::WriteArgsSubstitutions(
   path_output_no_escaping_.WriteFile(source_file_stream, source);
 
   EscapeOptions template_escape_options;
-  template_escape_options.mode = ESCAPE_NINJA_SHELL;
-  template_escape_options.inhibit_quoting = true;
+  template_escape_options.mode = ESCAPE_NINJA_COMMAND;
 
   args_template.WriteNinjaVariablesForSubstitution(
       out_, source_file_stream.str(), template_escape_options);
diff --git a/tools/gn/ninja_action_target_writer_unittest.cc b/tools/gn/ninja_action_target_writer_unittest.cc
index 10b5f81..9c6f2f1 100644
--- a/tools/gn/ninja_action_target_writer_unittest.cc
+++ b/tools/gn/ninja_action_target_writer_unittest.cc
@@ -29,11 +29,7 @@ TEST(NinjaActionTargetWriter, WriteOutputFilesForBuildLine) {
   std::vector<OutputFile> output_files;
   writer.WriteOutputFilesForBuildLine(output_template, source, &output_files);
 
-  std::string out_str = out.str();
-#if defined(OS_WIN)
-  std::replace(out_str.begin(), out_str.end(), '\\', '/');
-#endif
-  EXPECT_EQ(" gen/a$ bbar.h gen/bar.cc", out_str);
+  EXPECT_EQ(" gen/a$ bbar.h gen/bar.cc", out.str());
 }
 
 TEST(NinjaActionTargetWriter, WriteOutputFilesForBuildLineWithDepfile) {
@@ -57,11 +53,7 @@ TEST(NinjaActionTargetWriter, WriteOutputFilesForBuildLineWithDepfile) {
   std::vector<OutputFile> output_files;
   writer.WriteOutputFilesForBuildLine(output_template, source, &output_files);
 
-  std::string out_str = out.str();
-#if defined(OS_WIN)
-  std::replace(out_str.begin(), out_str.end(), '\\', '/');
-#endif
-  EXPECT_EQ(" gen/bar.d gen/bar.h gen/bar.cc", out_str);
+  EXPECT_EQ(" gen/bar.d gen/bar.h gen/bar.cc", out.str());
 }
 
 TEST(NinjaActionTargetWriter, WriteArgsSubstitutions) {
@@ -79,13 +71,15 @@ TEST(NinjaActionTargetWriter, WriteArgsSubstitutions) {
   FileTemplate args_template(args);
 
   writer.WriteArgsSubstitutions(SourceFile("//foo/b ar.in"), args_template);
-
-  std::string out_str = out.str();
 #if defined(OS_WIN)
-  std::replace(out_str.begin(), out_str.end(), '\\', '/');
+  EXPECT_EQ("  source = \"../../foo/b$ ar.in\"\n"
+            "  source_name_part = \"b$ ar\"\n",
+            out.str());
+#else
+  EXPECT_EQ("  source = ../../foo/b\\$ ar.in\n"
+            "  source_name_part = b\\$ ar\n",
+            out.str());
 #endif
-  EXPECT_EQ("  source = ../../foo/b$ ar.in\n  source_name_part = b$ ar\n",
-            out_str);
 }
 
 // Makes sure that we write sources as input dependencies for actions with
@@ -126,11 +120,7 @@ TEST(NinjaActionTargetWriter, ActionWithSources) {
         "build foo.out: __foo_bar___rule | obj/foo/bar.inputdeps.stamp\n"
         "\n"
         "build obj/foo/bar.stamp: stamp foo.out\n";
-    std::string out_str = out.str();
-#if defined(OS_WIN)
-    std::replace(out_str.begin(), out_str.end(), '\\', '/');
-#endif
-    EXPECT_EQ(expected_linux, out_str);
+    EXPECT_EQ(expected_linux, out.str());
   }
 
   // Windows.
@@ -145,8 +135,6 @@ TEST(NinjaActionTargetWriter, ActionWithSources) {
     NinjaActionTargetWriter writer(&target, setup.toolchain(), out);
     writer.Run();
 
-    // TODO(brettw) I think we'll need to worry about backslashes here
-    // depending if we're on actual Windows or Linux pretending to be Windows.
     const char expected_win[] =
         "rule __foo_bar___rule\n"
         "  command = C$:/python/python.exe gyp-win-tool action-wrapper environment.x86 __foo_bar___rule.$unique_name.rsp\n"
@@ -160,11 +148,7 @@ TEST(NinjaActionTargetWriter, ActionWithSources) {
         "build foo.out: __foo_bar___rule | obj/foo/bar.inputdeps.stamp\n"
         "\n"
         "build obj/foo/bar.stamp: stamp foo.out\n";
-    std::string out_str = out.str();
-#if defined(OS_WIN)
-    std::replace(out_str.begin(), out_str.end(), '\\', '/');
-#endif
-    EXPECT_EQ(expected_win, out_str);
+    EXPECT_EQ(expected_win, out.str());
   }
 }
 
@@ -214,7 +198,12 @@ TEST(NinjaActionTargetWriter, ForEach) {
     const char expected_linux[] =
         "rule __foo_bar___rule\n"
         "  command = /usr/bin/python ../../foo/script.py -i ${source} "
+            // Escaping is different between Windows and Posix.
+#if defined(OS_WIN)
             "\"--out=foo$ bar${source_name_part}.o\"\n"
+#else
+            "--out=foo\\$ bar${source_name_part}.o\n"
+#endif
         "  description = ACTION //foo:bar()\n"
         "  restat = 1\n"
         "build obj/foo/bar.inputdeps.stamp: stamp ../../foo/script.py "
@@ -241,8 +230,6 @@ TEST(NinjaActionTargetWriter, ForEach) {
 
   // Windows.
   {
-    // Note: we use forward slashes here so that the output will be the same on
-    // Linux and Windows.
     setup.build_settings()->set_python_path(base::FilePath(FILE_PATH_LITERAL(
         "C:/python/python.exe")));
     setup.settings()->set_target_os(Settings::WIN);
@@ -251,8 +238,6 @@ TEST(NinjaActionTargetWriter, ForEach) {
     NinjaActionTargetWriter writer(&target, setup.toolchain(), out);
     writer.Run();
 
-    // TODO(brettw) I think we'll need to worry about backslashes here
-    // depending if we're on actual Windows or Linux pretending to be Windows.
     const char expected_win[] =
         "rule __foo_bar___rule\n"
         "  command = C$:/python/python.exe gyp-win-tool action-wrapper "
@@ -261,7 +246,11 @@ TEST(NinjaActionTargetWriter, ForEach) {
         "  restat = 1\n"
         "  rspfile = __foo_bar___rule.$unique_name.rsp\n"
         "  rspfile_content = C$:/python/python.exe ../../foo/script.py -i "
+#if defined(OS_WIN)
             "${source} \"--out=foo$ bar${source_name_part}.o\"\n"
+#else
+            "${source} --out=foo\\$ bar${source_name_part}.o\n"
+#endif
         "build obj/foo/bar.inputdeps.stamp: stamp ../../foo/script.py "
             "../../foo/included.txt obj/foo/dep.stamp\n"
         "\n"
@@ -278,11 +267,7 @@ TEST(NinjaActionTargetWriter, ForEach) {
         "\n"
         "build obj/foo/bar.stamp: "
             "stamp input1.out input2.out obj/foo/datadep.stamp\n";
-    std::string out_str = out.str();
-#if defined(OS_WIN)
-    std::replace(out_str.begin(), out_str.end(), '\\', '/');
-#endif
-    EXPECT_EQ(expected_win, out_str);
+    EXPECT_EQ(expected_win, out.str());
   }
 }
 
@@ -322,7 +307,11 @@ TEST(NinjaActionTargetWriter, ForEachWithDepfile) {
     const char expected_linux[] =
         "rule __foo_bar___rule\n"
         "  command = /usr/bin/python ../../foo/script.py -i ${source} "
+#if defined(OS_WIN)
             "\"--out=foo$ bar${source_name_part}.o\"\n"
+#else
+            "--out=foo\\$ bar${source_name_part}.o\n"
+#endif
         "  description = ACTION //foo:bar()\n"
         "  restat = 1\n"
         "build obj/foo/bar.inputdeps.stamp: stamp ../../foo/script.py "
@@ -340,18 +329,11 @@ TEST(NinjaActionTargetWriter, ForEachWithDepfile) {
         "  depfile = gen/input2.d\n"
         "\n"
         "build obj/foo/bar.stamp: stamp input1.out input2.out\n";
-
-    std::string out_str = out.str();
-#if defined(OS_WIN)
-    std::replace(out_str.begin(), out_str.end(), '\\', '/');
-#endif
-    EXPECT_EQ(expected_linux, out_str);
+    EXPECT_EQ(expected_linux, out.str());
   }
 
   // Windows.
   {
-    // Note: we use forward slashes here so that the output will be the same on
-    // Linux and Windows.
     setup.build_settings()->set_python_path(base::FilePath(FILE_PATH_LITERAL(
         "C:/python/python.exe")));
     setup.settings()->set_target_os(Settings::WIN);
@@ -360,8 +342,6 @@ TEST(NinjaActionTargetWriter, ForEachWithDepfile) {
     NinjaActionTargetWriter writer(&target, setup.toolchain(), out);
     writer.Run();
 
-    // TODO(brettw) I think we'll need to worry about backslashes here
-    // depending if we're on actual Windows or Linux pretending to be Windows.
     const char expected_win[] =
         "rule __foo_bar___rule\n"
         "  command = C$:/python/python.exe gyp-win-tool action-wrapper "
@@ -370,7 +350,11 @@ TEST(NinjaActionTargetWriter, ForEachWithDepfile) {
         "  restat = 1\n"
         "  rspfile = __foo_bar___rule.$unique_name.rsp\n"
         "  rspfile_content = C$:/python/python.exe ../../foo/script.py -i "
+#if defined(OS_WIN)
             "${source} \"--out=foo$ bar${source_name_part}.o\"\n"
+#else
+            "${source} --out=foo\\$ bar${source_name_part}.o\n"
+#endif
         "build obj/foo/bar.inputdeps.stamp: stamp ../../foo/script.py "
             "../../foo/included.txt\n"
         "\n"
@@ -388,10 +372,6 @@ TEST(NinjaActionTargetWriter, ForEachWithDepfile) {
         "  depfile = gen/input2.d\n"
         "\n"
         "build obj/foo/bar.stamp: stamp input1.out input2.out\n";
-    std::string out_str = out.str();
-#if defined(OS_WIN)
-    std::replace(out_str.begin(), out_str.end(), '\\', '/');
-#endif
-    EXPECT_EQ(expected_win, out_str);
+    EXPECT_EQ(expected_win, out.str());
   }
 }
diff --git a/tools/gn/ninja_binary_target_writer.cc b/tools/gn/ninja_binary_target_writer.cc
index b645624..9cfbdfb 100644
--- a/tools/gn/ninja_binary_target_writer.cc
+++ b/tools/gn/ninja_binary_target_writer.cc
@@ -17,7 +17,7 @@ namespace {
 // Returns the proper escape options for writing compiler and linker flags.
 EscapeOptions GetFlagOptions() {
   EscapeOptions opts;
-  opts.mode = ESCAPE_NINJA;
+  opts.mode = ESCAPE_NINJA_COMMAND;
 
   // Some flag strings are actually multiple flags that expect to be just
   // added to the command line. We assume that quoting is done by the
@@ -29,7 +29,7 @@ EscapeOptions GetFlagOptions() {
 
 struct DefineWriter {
   DefineWriter() {
-    options.mode = ESCAPE_SHELL;
+    options.mode = ESCAPE_NINJA_COMMAND;
   }
 
   void operator()(const std::string& s, std::ostream& out) const {
@@ -41,33 +41,20 @@ struct DefineWriter {
 };
 
 struct IncludeWriter {
-  IncludeWriter(PathOutput& path_output,
-                const NinjaHelper& h)
+  IncludeWriter(PathOutput& path_output, const NinjaHelper& h)
       : helper(h),
-        path_output_(path_output),
-        old_inhibit_quoting_(path_output.inhibit_quoting()) {
-    // Inhibit quoting since we'll put quotes around the whole thing ourselves.
-    // Since we're writing in NINJA escaping mode, this won't actually do
-    // anything, but I think we may need to change to shell-and-then-ninja
-    // escaping for this in the future.
-    path_output_.set_inhibit_quoting(true);
+        path_output_(path_output) {
   }
   ~IncludeWriter() {
-    path_output_.set_inhibit_quoting(old_inhibit_quoting_);
   }
 
   void operator()(const SourceDir& d, std::ostream& out) const {
-    out << " \"-I";
-    // It's important not to include the trailing slash on directories or on
-    // Windows it will be a backslash and the compiler might think we're
-    // escaping the quote!
+    out << " -I";
     path_output_.WriteDir(out, d, PathOutput::DIR_NO_LAST_SLASH);
-    out << "\"";
   }
 
   const NinjaHelper& helper;
   PathOutput& path_output_;
-  bool old_inhibit_quoting_;  // So we can put the PathOutput back.
 };
 
 Toolchain::ToolType GetToolTypeForTarget(const Target* target) {
@@ -268,8 +255,8 @@ void NinjaBinaryTargetWriter::WriteLinkerFlags(
   if (!all_lib_dirs.empty()) {
     // Since we're passing these on the command line to the linker and not
     // to Ninja, we need to do shell escaping.
-    PathOutput lib_path_output(
-        path_output_.current_dir(), ESCAPE_NINJA_SHELL, false);
+    PathOutput lib_path_output(path_output_.current_dir(),
+                               ESCAPE_NINJA_COMMAND);
     for (size_t i = 0; i < all_lib_dirs.size(); i++) {
       out_ << " " << tool.lib_dir_prefix;
       lib_path_output.WriteDir(out_, all_lib_dirs[i],
@@ -291,7 +278,7 @@ void NinjaBinaryTargetWriter::WriteLibs(const Toolchain::Tool& tool) {
 
   // Libraries that have been recursively pushed through the dependency tree.
   EscapeOptions lib_escape_opts;
-  lib_escape_opts.mode = ESCAPE_NINJA_SHELL;
+  lib_escape_opts.mode = ESCAPE_NINJA_COMMAND;
   const OrderedSet<std::string> all_libs = target_->all_libs();
   const std::string framework_ending(".framework");
   for (size_t i = 0; i < all_libs.size(); i++) {
diff --git a/tools/gn/ninja_build_writer.cc b/tools/gn/ninja_build_writer.cc
index f903904..007089d 100644
--- a/tools/gn/ninja_build_writer.cc
+++ b/tools/gn/ninja_build_writer.cc
@@ -39,7 +39,7 @@ std::string GetSelfInvocationCommand(const BuildSettings* build_settings) {
   cmdline.AppendSwitch("-q");  // Don't write output.
 
   EscapeOptions escape_shell;
-  escape_shell.mode = ESCAPE_SHELL;
+  escape_shell.mode = ESCAPE_NINJA_COMMAND;
 #if defined(OS_WIN)
   // The command line code quoting varies by platform. We have one string,
   // possibly with spaces, that we want to quote. The Windows command line
@@ -82,7 +82,7 @@ NinjaBuildWriter::NinjaBuildWriter(
       default_toolchain_targets_(default_toolchain_targets),
       out_(out),
       dep_out_(dep_out),
-      path_output_(build_settings->build_dir(), ESCAPE_NINJA, false),
+      path_output_(build_settings->build_dir(), ESCAPE_NINJA),
       helper_(build_settings) {
 }
 
diff --git a/tools/gn/ninja_target_writer.cc b/tools/gn/ninja_target_writer.cc
index 778e983..a67c3fa 100644
--- a/tools/gn/ninja_target_writer.cc
+++ b/tools/gn/ninja_target_writer.cc
@@ -26,9 +26,7 @@ NinjaTargetWriter::NinjaTargetWriter(const Target* target,
       target_(target),
       toolchain_(toolchain),
       out_(out),
-      path_output_(settings_->build_settings()->build_dir(),
-                   ESCAPE_NINJA,
-                   false),
+      path_output_(settings_->build_settings()->build_dir(), ESCAPE_NINJA),
       helper_(settings_->build_settings()) {
 }
 
diff --git a/tools/gn/ninja_toolchain_writer.cc b/tools/gn/ninja_toolchain_writer.cc
index 44d2586..99087c6 100644
--- a/tools/gn/ninja_toolchain_writer.cc
+++ b/tools/gn/ninja_toolchain_writer.cc
@@ -23,9 +23,7 @@ NinjaToolchainWriter::NinjaToolchainWriter(
       toolchain_(toolchain),
       targets_(targets),
       out_(out),
-      path_output_(settings_->build_settings()->build_dir(),
-                   ESCAPE_NINJA,
-                   false),
+      path_output_(settings_->build_settings()->build_dir(), ESCAPE_NINJA),
       helper_(settings->build_settings()) {
 }
 
diff --git a/tools/gn/path_output.cc b/tools/gn/path_output.cc
index 4e71f9b..7e739b6 100644
--- a/tools/gn/path_output.cc
+++ b/tools/gn/path_output.cc
@@ -9,9 +9,7 @@
 #include "tools/gn/output_file.h"
 #include "tools/gn/string_utils.h"
 
-PathOutput::PathOutput(const SourceDir& current_dir,
-                       EscapingMode escaping,
-                       bool convert_slashes)
+PathOutput::PathOutput(const SourceDir& current_dir, EscapingMode escaping)
     : current_dir_(current_dir) {
   CHECK(current_dir.is_source_absolute())
       << "Currently this only supports writing to output directories inside "
@@ -20,11 +18,6 @@ PathOutput::PathOutput(const SourceDir& current_dir,
   inverse_current_dir_ = InvertDir(current_dir_);
 
   options_.mode = escaping;
-  options_.convert_slashes = convert_slashes;
-  options_.inhibit_quoting = false;
-
-  if (convert_slashes)
-    ConvertPathToSystem(&inverse_current_dir_);
 }
 
 PathOutput::~PathOutput() {
@@ -91,12 +84,9 @@ void PathOutput::WriteFile(std::ostream& out,
 void PathOutput::WriteSourceRelativeString(
     std::ostream& out,
     const base::StringPiece& str) const {
-  if (options_.mode == ESCAPE_SHELL) {
+  if (options_.mode == ESCAPE_NINJA_COMMAND) {
     // Shell escaping needs an intermediate string since it may end up
-    // quoting the whole thing. On Windows, the slashes may already be
-    // converted to backslashes in inverse_current_dir_, but we assume that on
-    // Windows the escaper won't try to then escape the preconverted
-    // backslashes and will just pass them, so this is fine.
+    // quoting the whole thing.
     std::string intermediate;
     intermediate.reserve(inverse_current_dir_.size() + str.size());
     intermediate.assign(inverse_current_dir_.c_str(),
diff --git a/tools/gn/path_output.h b/tools/gn/path_output.h
index 33227d4..ba4a5f6 100644
--- a/tools/gn/path_output.h
+++ b/tools/gn/path_output.h
@@ -33,9 +33,7 @@ class PathOutput {
     DIR_NO_LAST_SLASH,
   };
 
-  PathOutput(const SourceDir& current_dir,
-             EscapingMode escaping,
-             bool convert_slashes);
+  PathOutput(const SourceDir& current_dir, EscapingMode escaping);
   ~PathOutput();
 
   // Read-only since inverse_current_dir_ is computed depending on this.
@@ -43,19 +41,10 @@ class PathOutput {
 
   const SourceDir& current_dir() const { return current_dir_; }
 
-  // When true, converts slashes to the system-type path separators (on
-  // Windows, this is a backslash, this is a NOP otherwise).
-  //
-  // Read-only since inverse_current_dir_ is computed depending on this.
-  bool convert_slashes_to_system() const { return options_.convert_slashes; }
-
-  // When the output escaping is ESCAPE_SHELL, the escaper will normally put
-  // quotes around suspect things. If this value is set to true, we'll disable
-  // the quoting feature. This means that in ESCAPE_SHELL mode, strings with
-  // spaces in them qon't be quoted. This mode is for when quoting is done at
-  // some higher-level. Defaults to false.
+  // Getter/setters for flags inside the escape options.
   bool inhibit_quoting() const { return options_.inhibit_quoting; }
   void set_inhibit_quoting(bool iq) { options_.inhibit_quoting = iq; }
+  void set_escape_platform(EscapingPlatform p) { options_.platform = p; }
 
   void WriteFile(std::ostream& out, const SourceFile& file) const;
   void WriteFile(std::ostream& out, const OutputFile& file) const;
diff --git a/tools/gn/path_output_unittest.cc b/tools/gn/path_output_unittest.cc
index 49f29c9..389f96a 100644
--- a/tools/gn/path_output_unittest.cc
+++ b/tools/gn/path_output_unittest.cc
@@ -11,7 +11,7 @@
 
 TEST(PathOutput, Basic) {
   SourceDir build_dir("//out/Debug/");
-  PathOutput writer(build_dir, ESCAPE_NONE, false);
+  PathOutput writer(build_dir, ESCAPE_NONE);
   {
     // Normal source-root path.
     std::ostringstream out;
@@ -52,7 +52,7 @@ TEST(PathOutput, Basic) {
 // Same as basic but the output dir is the root.
 TEST(PathOutput, BasicInRoot) {
   SourceDir build_dir("//");
-  PathOutput writer(build_dir, ESCAPE_NONE, false);
+  PathOutput writer(build_dir, ESCAPE_NONE);
   {
     // Normal source-root path.
     std::ostringstream out;
@@ -69,7 +69,7 @@ TEST(PathOutput, BasicInRoot) {
 
 TEST(PathOutput, NinjaEscaping) {
   SourceDir build_dir("//out/Debug/");
-  PathOutput writer(build_dir, ESCAPE_NINJA, false);
+  PathOutput writer(build_dir, ESCAPE_NINJA);
   {
     // Spaces and $ in filenames.
     std::ostringstream out;
@@ -84,63 +84,85 @@ TEST(PathOutput, NinjaEscaping) {
   }
 }
 
-TEST(PathOutput, ShellEscaping) {
+TEST(PathOutput, NinjaForkEscaping) {
   SourceDir build_dir("//out/Debug/");
-  PathOutput writer(build_dir, ESCAPE_SHELL, false);
+  PathOutput writer(build_dir, ESCAPE_NINJA_COMMAND);
+
+  // Spaces in filenames should get quoted on Windows.
+  writer.set_escape_platform(ESCAPE_PLATFORM_WIN);
   {
-    // Spaces in filenames should get quoted.
     std::ostringstream out;
     writer.WriteFile(out, SourceFile("//foo/foo bar.cc"));
-    EXPECT_EQ("\"../../foo/foo bar.cc\"", out.str());
+    EXPECT_EQ("\"../../foo/foo$ bar.cc\"", out.str());
+  }
+
+  // Spaces in filenames should get escaped on Posix.
+  writer.set_escape_platform(ESCAPE_PLATFORM_POSIX);
+  {
+    std::ostringstream out;
+    writer.WriteFile(out, SourceFile("//foo/foo bar.cc"));
+    EXPECT_EQ("../../foo/foo\\$ bar.cc", out.str());
+  }
+
+  // Quotes should get blackslash-escaped on Windows and Posix.
+  writer.set_escape_platform(ESCAPE_PLATFORM_WIN);
+  {
+    std::ostringstream out;
+    writer.WriteFile(out, SourceFile("//foo/\"foobar\".cc"));
+    // Our Windows code currently quotes the whole thing in this case for
+    // code simplicity, even though it's strictly unnecessary. This might
+    // change in the future.
+    EXPECT_EQ("\"../../foo/\\\"foobar\\\".cc\"", out.str());
   }
+  writer.set_escape_platform(ESCAPE_PLATFORM_POSIX);
   {
-    // Quotes should get blackslash-escaped.
     std::ostringstream out;
     writer.WriteFile(out, SourceFile("//foo/\"foobar\".cc"));
     EXPECT_EQ("../../foo/\\\"foobar\\\".cc", out.str());
   }
+
+
+  // Backslashes should get escaped on non-Windows and preserved on Windows.
+  writer.set_escape_platform(ESCAPE_PLATFORM_WIN);
   {
-    // Backslashes should get escaped on non-Windows and preserved on Windows.
     std::ostringstream out;
     writer.WriteFile(out, SourceFile("//foo\\bar.cc"));
-#if defined(OS_WIN)
     EXPECT_EQ("../../foo\\bar.cc", out.str());
-#else
-    EXPECT_EQ("../../foo\\\\bar.cc", out.str());
-#endif
   }
-}
-
-TEST(PathOutput, SlashConversion) {
-  SourceDir build_dir("//out/Debug/");
-  PathOutput writer(build_dir, ESCAPE_NINJA, true);
+  writer.set_escape_platform(ESCAPE_PLATFORM_POSIX);
   {
     std::ostringstream out;
-    writer.WriteFile(out, SourceFile("//foo/bar.cc"));
-#if defined(OS_WIN)
-    EXPECT_EQ("..\\..\\foo\\bar.cc", out.str());
-#else
-    EXPECT_EQ("../../foo/bar.cc", out.str());
-#endif
+    writer.WriteFile(out, SourceFile("//foo\\bar.cc"));
+    EXPECT_EQ("../../foo\\\\bar.cc", out.str());
   }
 }
 
 TEST(PathOutput, InhibitQuoting) {
   SourceDir build_dir("//out/Debug/");
-  PathOutput writer(build_dir, ESCAPE_SHELL, false);
+  PathOutput writer(build_dir, ESCAPE_NINJA_COMMAND);
   writer.set_inhibit_quoting(true);
+
+  writer.set_escape_platform(ESCAPE_PLATFORM_WIN);
   {
     // We should get unescaped spaces in the output with no quotes.
     std::ostringstream out;
     writer.WriteFile(out, SourceFile("//foo/foo bar.cc"));
-    EXPECT_EQ("../../foo/foo bar.cc", out.str());
+    EXPECT_EQ("../../foo/foo$ bar.cc", out.str());
+  }
+
+  writer.set_escape_platform(ESCAPE_PLATFORM_POSIX);
+  {
+    // Escapes the space.
+    std::ostringstream out;
+    writer.WriteFile(out, SourceFile("//foo/foo bar.cc"));
+    EXPECT_EQ("../../foo/foo\\$ bar.cc", out.str());
   }
 }
 
 TEST(PathOutput, WriteDir) {
   {
     SourceDir build_dir("//out/Debug/");
-    PathOutput writer(build_dir, ESCAPE_NINJA, false);
+    PathOutput writer(build_dir, ESCAPE_NINJA);
     {
       std::ostringstream out;
       writer.WriteDir(out, SourceDir("//foo/bar/"),
@@ -191,6 +213,8 @@ TEST(PathOutput, WriteDir) {
     // Output inside current dir.
     {
       std::ostringstream out;
+
+
       writer.WriteDir(out, SourceDir("//out/Debug/"),
                       PathOutput::DIR_INCLUDE_LAST_SLASH);
       EXPECT_EQ("./", out.str());
@@ -216,7 +240,7 @@ TEST(PathOutput, WriteDir) {
   }
   {
     // Empty build dir writer.
-    PathOutput root_writer(SourceDir("//"), ESCAPE_NINJA, false);
+    PathOutput root_writer(SourceDir("//"), ESCAPE_NINJA);
     {
       std::ostringstream out;
       root_writer.WriteDir(out, SourceDir("//"),