Improvements in --debug-print switch implementation.

printing/printed_document.cc

Index: printing/printed_document.cc
diff --git a/printing/printed_document.cc b/printing/printed_document.cc
index 7c2392d064dc0e0562a5d783cbaf145ca71633c3..09c8fddf4e5b9945a587a5e9da2f10e7cce15973 100644
--- a/printing/printed_document.cc
+++ b/printing/printed_document.cc
@@ -13,12 +13,17 @@
 #include "base/files/file_path.h"
 #include "base/i18n/file_util_icu.h"
 #include "base/i18n/time_formatting.h"
+#include "base/json/json_writer.h"
 #include "base/lazy_instance.h"
 #include "base/message_loop/message_loop.h"
+#include "base/numerics/safe_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/time/time.h"
+#include "base/values.h"
 #include "printing/page_number.h"
+#include "printing/print_settings_conversion.h"
 #include "printing/printed_page.h"
 #include "printing/printed_pages_source.h"
 #include "printing/units.h"
@@ -26,24 +31,15 @@
 #include "ui/gfx/font.h"
 #include "ui/gfx/text_elider.h"
 
-namespace {
-
-struct PrintDebugDumpPath {
-  PrintDebugDumpPath()
-    : enabled(false) {
-  }
+namespace printing {
 
-  bool enabled;
-  base::FilePath debug_dump_path;
-};
+namespace {
 
-base::LazyInstance<PrintDebugDumpPath> g_debug_dump_info =
+base::LazyInstance<base::FilePath> g_debug_dump_info =
     LAZY_INSTANCE_INITIALIZER;
 
 }  // namespace
 
-namespace printing {
-
 PrintedDocument::PrintedDocument(const PrintSettings& settings,
                                  PrintedPagesSource* source,
                                  int cookie)
@@ -85,20 +81,19 @@ void PrintedDocument::SetPage(int page_number,
       mutable_.first_page = page_number;
 #endif
   }
-  DebugDump(*page.get());
 }
 
-bool PrintedDocument::GetPage(int page_number,
-                              scoped_refptr<PrintedPage>* page) {
-  base::AutoLock lock(lock_);
-  PrintedPages::const_iterator itr = mutable_.pages_.find(page_number);
-  if (itr != mutable_.pages_.end()) {
-    if (itr->second.get()) {
-      *page = itr->second;
-      return true;
-    }
+scoped_refptr<PrintedPage> PrintedDocument::GetPage(int page_number) {
+  scoped_refptr<PrintedPage> page;
+  {
+    base::AutoLock lock(lock_);
+    PrintedPages::const_iterator itr = mutable_.pages_.find(page_number);
+    if (itr != mutable_.pages_.end())
+      page = itr->second;
   }
-  return false;
+  if (page)
+    DebugDump(*page);
+  return page;
 }
 
 bool PrintedDocument::IsComplete() const {
@@ -173,34 +168,54 @@ int PrintedDocument::expected_page_count() const {
 }
 
 void PrintedDocument::DebugDump(const PrintedPage& page) {
-  if (!g_debug_dump_info.Get().enabled)
+  if (g_debug_dump_info.Get().empty())
     return;
 
-  base::string16 filename;
-  filename += name();
-  filename += base::ASCIIToUTF16("_");
+  base::string16 filename = name();
   filename += base::ASCIIToUTF16(
-      base::StringPrintf("%02d", page.page_number()));
+      base::StringPrintf("_%04d", page.page_number()));
 #if defined(OS_WIN)
-  filename += base::ASCIIToUTF16("_.emf");
-  page.metafile()->SaveTo(
-      g_debug_dump_info.Get().debug_dump_path.Append(filename));
+  page.metafile()->SaveTo(CreateDebugDumpPath(filename, "emf"));
 #else  // OS_WIN
-  filename += base::ASCIIToUTF16("_.pdf");
-  page.metafile()->SaveTo(
-      g_debug_dump_info.Get().debug_dump_path.Append(
-          base::UTF16ToUTF8(filename)));
+  page.metafile()->SaveTo(CreateDebugDumpPath(filename, "pdf"));
 #endif  // OS_WIN
+
+  base::DictionaryValue job_settings;
+  PrintSettingsToJobSettingsDebug(settings(), &job_settings);
+  std::string settings;
+  base::JSONWriter::WriteWithOptions(
+      &job_settings, base::JSONWriter::OPTIONS_PRETTY_PRINT, &settings);
+  base::WriteFile(CreateDebugDumpPath(filename, "json"),
+                  settings.data(),
+                  base::checked_cast<int>(settings.size()));
 }
 
 void PrintedDocument::set_debug_dump_path(
     const base::FilePath& debug_dump_path) {
-  g_debug_dump_info.Get().enabled = !debug_dump_path.empty();
-  g_debug_dump_info.Get().debug_dump_path = debug_dump_path;
+  g_debug_dump_info.Get() = debug_dump_path;
 }
 
-const base::FilePath& PrintedDocument::debug_dump_path() {
-  return g_debug_dump_info.Get().debug_dump_path;
+base::FilePath PrintedDocument::CreateDebugDumpPath(
+    const base::string16& document_name,
+    const std::string& extension) {
+  if (g_debug_dump_info.Get().empty())
+    return base::FilePath();
+  // Create a filename.
+  base::string16 filename;
+  base::Time now(base::Time::Now());
+  filename = base::TimeFormatShortDateAndTime(now);

[Lei Zhang, 2014/06/11 20:03:06]

Might be clear to StringPrintf() this.

[Vitaly Buka (NO REVIEWS), 2014/06/11 21:32:57]

You mean document name?
We have not StringPrintf for string16 on Android, and document name is string16

+  filename += base::ASCIIToUTF16("_");
+  filename += document_name;
+  filename += base::ASCIIToUTF16(".");
+  filename += base::ASCIIToUTF16(extension);
+  base::FilePath::StringType system_filename;
+#if defined(OS_WIN)
+  system_filename = filename;
+#else   // OS_WIN
+  system_filename = base::UTF16ToUTF8(filename);
+#endif  // OS_WIN
+  file_util::ReplaceIllegalCharactersInPath(&system_filename, '_');
+  return g_debug_dump_info.Get().Append(system_filename);
 }
 
 PrintedDocument::Mutable::Mutable(PrintedPagesSource* source)