[snapshot] only create snapshot files as last step in mksnapshot.

R=leszeks@chromium.org
BUG=chromium:633159

Review-Url: https://codereview.chromium.org/2767903002
Cr-Commit-Position: refs/heads/master@{#44015}

src/snapshot/mksnapshot.cc

@@ -19,23 +19,21 @@ using namespace v8;
 
 class SnapshotWriter {
  public:
-  SnapshotWriter() : fp_(NULL), startup_blob_file_(NULL) {}
+  SnapshotWriter() : snapshot_cpp_path_(NULL), snapshot_blob_path_(NULL) {}
 
-  ~SnapshotWriter() {
-    if (fp_) fclose(fp_);
-    if (startup_blob_file_) fclose(startup_blob_file_);
+  void SetSnapshotFile(const char* snapshot_cpp_file) {
+    snapshot_cpp_path_ = snapshot_cpp_file;
   }
 
-  void SetSnapshotFile(const char* snapshot_file) {
-    if (snapshot_file != NULL) fp_ = GetFileDescriptorOrDie(snapshot_file);
-  }
-
-  void SetStartupBlobFile(const char* startup_blob_file) {
-    if (startup_blob_file != NULL)
-      startup_blob_file_ = GetFileDescriptorOrDie(startup_blob_file);
+  void SetStartupBlobFile(const char* snapshot_blob_file) {
+    snapshot_blob_path_ = snapshot_blob_file;
   }
 
   void WriteSnapshot(v8::StartupData blob) const {
+    // TODO(crbug/633159): if we crash before the files have been fully created,
+    // we end up with a corrupted snapshot file. The build step would succeed,
+    // but the build target is unusable. Ideally we would write out temporary
+    // files and only move them to the final destination as last step.
     i::Vector<const i::byte> blob_vector(
         reinterpret_cast<const i::byte*>(blob.data), blob.raw_size);
     MaybeWriteSnapshotFile(blob_vector);
@@ -44,59 +42,67 @@ class SnapshotWriter {
 
  private:
   void MaybeWriteStartupBlob(const i::Vector<const i::byte>& blob) const {
-    if (!startup_blob_file_) return;
+    if (!snapshot_blob_path_) return;
 
-    size_t written = fwrite(blob.begin(), 1, blob.length(), startup_blob_file_);
+    FILE* fp = GetFileDescriptorOrDie(snapshot_blob_path_);
+    size_t written = fwrite(blob.begin(), 1, blob.length(), fp);
+    fclose(fp);
     if (written != static_cast<size_t>(blob.length())) {
       i::PrintF("Writing snapshot file failed.. Aborting.\n");
+      remove(snapshot_blob_path_);
       exit(1);
     }
   }
 
   void MaybeWriteSnapshotFile(const i::Vector<const i::byte>& blob) const {
-    if (!fp_) return;
+    if (!snapshot_cpp_path_) return;
+
+    FILE* fp = GetFileDescriptorOrDie(snapshot_cpp_path_);
+
+    WriteFilePrefix(fp);
+    WriteData(fp, blob);
+    WriteFileSuffix(fp);
 
-    WriteFilePrefix();
-    WriteData(blob);
-    WriteFileSuffix();
+    fclose(fp);
   }
 
-  void WriteFilePrefix() const {
-    fprintf(fp_, "// Autogenerated snapshot file. Do not edit.\n\n");
-    fprintf(fp_, "#include \"src/v8.h\"\n");
-    fprintf(fp_, "#include \"src/base/platform/platform.h\"\n\n");
-    fprintf(fp_, "#include \"src/snapshot/snapshot.h\"\n\n");
-    fprintf(fp_, "namespace v8 {\n");
-    fprintf(fp_, "namespace internal {\n\n");
+  static void WriteFilePrefix(FILE* fp) {
+    fprintf(fp, "// Autogenerated snapshot file. Do not edit.\n\n");
+    fprintf(fp, "#include \"src/v8.h\"\n");
+    fprintf(fp, "#include \"src/base/platform/platform.h\"\n\n");
+    fprintf(fp, "#include \"src/snapshot/snapshot.h\"\n\n");
+    fprintf(fp, "namespace v8 {\n");
+    fprintf(fp, "namespace internal {\n\n");
   }
 
-  void WriteFileSuffix() const {
-    fprintf(fp_, "const v8::StartupData* Snapshot::DefaultSnapshotBlob() {\n");
-    fprintf(fp_, "  return &blob;\n");
-    fprintf(fp_, "}\n\n");
-    fprintf(fp_, "}  // namespace internal\n");
-    fprintf(fp_, "}  // namespace v8\n");
+  static void WriteFileSuffix(FILE* fp) {
+    fprintf(fp, "const v8::StartupData* Snapshot::DefaultSnapshotBlob() {\n");
+    fprintf(fp, "  return &blob;\n");
+    fprintf(fp, "}\n\n");
+    fprintf(fp, "}  // namespace internal\n");
+    fprintf(fp, "}  // namespace v8\n");
   }
 
-  void WriteData(const i::Vector<const i::byte>& blob) const {
-    fprintf(fp_, "static const byte blob_data[] = {\n");
-    WriteSnapshotData(blob);
-    fprintf(fp_, "};\n");
-    fprintf(fp_, "static const int blob_size = %d;\n", blob.length());
-    fprintf(fp_, "static const v8::StartupData blob =\n");
-    fprintf(fp_, "{ (const char*) blob_data, blob_size };\n");
+  static void WriteData(FILE* fp, const i::Vector<const i::byte>& blob) {
+    fprintf(fp, "static const byte blob_data[] = {\n");
+    WriteSnapshotData(fp, blob);
+    fprintf(fp, "};\n");
+    fprintf(fp, "static const int blob_size = %d;\n", blob.length());
+    fprintf(fp, "static const v8::StartupData blob =\n");
+    fprintf(fp, "{ (const char*) blob_data, blob_size };\n");
   }
 
-  void WriteSnapshotData(const i::Vector<const i::byte>& blob) const {
+  static void WriteSnapshotData(FILE* fp,
+                                const i::Vector<const i::byte>& blob) {
     for (int i = 0; i < blob.length(); i++) {
-      if ((i & 0x1f) == 0x1f) fprintf(fp_, "\n");
-      if (i > 0) fprintf(fp_, ",");
-      fprintf(fp_, "%u", static_cast<unsigned char>(blob.at(i)));
+      if ((i & 0x1f) == 0x1f) fprintf(fp, "\n");
+      if (i > 0) fprintf(fp, ",");
+      fprintf(fp, "%u", static_cast<unsigned char>(blob.at(i)));
     }
-    fprintf(fp_, "\n");
+    fprintf(fp, "\n");
   }
 
-  FILE* GetFileDescriptorOrDie(const char* filename) {
+  static FILE* GetFileDescriptorOrDie(const char* filename) {
     FILE* fp = base::OS::FOpen(filename, "wb");
     if (fp == NULL) {
       i::PrintF("Unable to open file \"%s\" for writing.\n", filename);
@@ -105,8 +111,8 @@ class SnapshotWriter {
     return fp;
   }
 
-  FILE* fp_;
-  FILE* startup_blob_file_;
+  const char* snapshot_cpp_path_;
+  const char* snapshot_blob_path_;
 };
 
 char* GetExtraCode(char* filename, const char* description) {