Added a test that currently is able to print to pdf.

chrome/browser/printing/print_preview_pdf_generated_browsertest.cc

Index: chrome/browser/printing/print_preview_pdf_generated_browsertest.cc
diff --git a/chrome/browser/printing/print_preview_pdf_generated_browsertest.cc b/chrome/browser/printing/print_preview_pdf_generated_browsertest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..7948a718728a96a24d9fe43219f2ea2e04ccedf5
--- /dev/null
+++ b/chrome/browser/printing/print_preview_pdf_generated_browsertest.cc
@@ -0,0 +1,621 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <cstdio>
+#include <iostream>
+#include <limits>
+#include <string>
+#include <vector>
+
+#include "base/file_util.h"
+#include "base/files/file.h"
+#include "base/files/file_path.h"
+#include "base/files/scoped_temp_dir.h"
+#include "base/format_macros.h"
+#include "base/logging.h"
+#include "base/md5.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/numerics/safe_conversions.h"
+#include "base/path_service.h"
+#include "base/run_loop.h"
+#include "base/scoped_native_library.h"
+#include "base/strings/string16.h"
+#include "base/strings/string_split.h"
+#include "base/strings/string_util.h"
+#include "base/strings/utf_string_conversions.h"
+#include "chrome/app/chrome_command_ids.h"
+#include "chrome/browser/net/referrer.h"
+#include "chrome/browser/printing/print_preview_dialog_controller.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_commands.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/webui/print_preview/print_preview_handler.h"
+#include "chrome/browser/ui/webui/print_preview/print_preview_ui.h"
+#include "chrome/browser/ui/webui/print_preview/sticky_settings.h"
+#include "chrome/common/chrome_paths.h"
+#include "chrome/common/print_messages.h"
+#include "chrome/common/url_constants.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_ui_message_handler.h"
+#include "content/public/common/page_transition_types.h"
+#include "content/public/test/browser_test_utils.h"
+#include "content/public/test/test_navigation_observer.h"
+#include "content/public/test/test_utils.h"
+#include "net/base/filename_util.h"
+#include "printing/pdf_render_settings.h"
+#include "printing/units.h"
+#include "ui/events/keycodes/keyboard_codes.h"
+#include "ui/gfx/codec/png_codec.h"
+#include "ui/gfx/geometry/rect.h"
+#include "url/gurl.h"
+#include "ipc/ipc_message_macros.h"
+
+#if defined(OS_POSIX)

[ivandavid, 2014/07/03 03:12:03]

Is this an OK solution to the problem of getline needing different stream types
depending on the platform? I think it is, however if there is a better way I
will implement that one. Also, is there a specific header I have to include to
make this work?

[Lei Zhang, 2014/07/08 01:11:36]

Can you just use the same pattern as the blink layout test runner? I don't
recall they having to do this.

[ivandavid, 2014/07/08 23:24:19]

I'm going to keep this implementation for now until I run I start testing the
program on windows. Then if it doesn't work, I'll change it, even though it
should work.

+#define STDIN_STREAM std::cin
+#elif defined(OS_WIN)
+#define STDIN_STREAM std::wcin
+#endif
+
+using content::WebContents;
+using content::WebContentsObserver;
+
+// Message refers to the 'UILoaded' message from print_preview.js.
+// It gets sent either from onPreviewGenerationDone or from
+// onManipulateSettings_() in print_preview.js
+enum State {
+  // Waiting for the first message so the program can select Save as PDF
+  kWaitingToSendSaveAsPdf = 0,
+  // Waiting for the second message so the test can set the layout
+  kWaitingToSendLayoutSettings = 1,
+  // Waiting for the third message so the test can set the page numbers
+  kWaitingToSendPageNumbers = 2,
+  // Waiting for the forth message so the test can set the headers checkbox
+  kWaitingToSendHeadersAndFooters = 3,
+  // Waiting for the fifth message so the test can set the background checkbox
+  kWaitingToSendBackgroundColorsAndImages = 4,
+  // Waiting for the sixth message so the test can set the margins combobox
+  kWaitingToSendMargins = 5,
+  // Waiting for the final message so the program can save to PDF.
+  kWaitingForFinalMessage = 6,
+};
+
+struct PrintPreviewSettings {
+  PrintPreviewSettings(bool is_portrait,
+                       std::string page_numbers,
+                       bool headers_and_footers,
+                       bool background_colors_and_images,
+                       printing::MarginType margins,
+                       bool is_already_pdf)
+      : is_portrait(is_portrait),
+        page_numbers(page_numbers),
+        headers_and_footers(headers_and_footers),
+        background_colors_and_images(background_colors_and_images),
+        margins(margins),
+        is_already_pdf(is_already_pdf) {}
+
+  PrintPreviewSettings(const PrintPreviewSettings& settings)
+      : is_portrait(settings.is_portrait),
+        page_numbers(settings.page_numbers),
+        headers_and_footers(settings.headers_and_footers),
+        background_colors_and_images(settings.background_colors_and_images),
+        margins(settings.margins),
+        is_already_pdf(settings.is_already_pdf) {}

[Dan Beam, 2014/07/07 23:08:11]

do you need to explicitly declare these constructors?  if you delete L87-106
does the code still work?

[ivandavid, 2014/07/08 01:07:41]

The first one does, however the copy constructor doesn't need to be there. I
forgot c++ provides one by default and I didn't need to override it because
there are no pointers.

[ivandavid, 2014/07/08 01:07:41]

Done.

+
+  bool is_portrait;
+  std::string page_numbers;
+  bool headers_and_footers;
+  bool background_colors_and_images;
+  printing::MarginType margins;
+  bool is_already_pdf;
+};
+
+// Observes the print preview webpage. Once it observes the
+// PreviewPageCount message, will send a sequence of commands
+// to the print preview dialog and change the settings of the
+// preview dialog.
+class PrintPreviewObserver : public WebContentsObserver {
+ public:
+  PrintPreviewObserver(WebContents* dialog,
+                       Browser* browser)

[Lei Zhang, 2014/07/08 01:11:37]

Fits on the previous line. Not sure why you moved it.

[ivandavid, 2014/07/08 23:24:20]

Done.

[ivandavid, 2014/07/08 23:24:21]

I added an extra parameter, but then removed it, then forgot to move this back
up.

+      : WebContentsObserver(dialog),
+        browser_(browser),
+        state_(kWaitingToSendSaveAsPdf) {}
+
+  virtual ~PrintPreviewObserver() {}
+
+  // Sets closure for the observer so that it can end the loop.
+  void set_quit_closure(const base::Closure &closure) {
+    closure_ = closure;
+  }
+
+  // Actually stops the message_loop so that the test can proceed.
+  void EndLoop() {
+    base::MessageLoop::current()->PostTask(FROM_HERE, closure_);
+  }
+
+  // This method must always return false. If it ever returns true,
+  // it will cause the test to hang. This is because the
+  // PrintPreviewMessageHandler should still handle all of the messages to
+  // progress the print preview process.
+  bool OnMessageReceived(const IPC::Message& message) OVERRIDE {
+    IPC_BEGIN_MESSAGE_MAP(PrintPreviewObserver, message)
+      IPC_MESSAGE_HANDLER(PrintHostMsg_DidGetPreviewPageCount,
+                          OnDidGetPreviewPageCount)
+      IPC_MESSAGE_UNHANDLED(break;)
+    IPC_END_MESSAGE_MAP();
+    return false;
+  }
+
+  // Gets the web contents for the print preview dialog so that
+  // the UI and other elements can be accessed.
+  WebContents* GetDialog() {
+    WebContents* tab = browser_->tab_strip_model()->GetActiveWebContents();
+    printing::PrintPreviewDialogController* dialog_controller =
+        printing::PrintPreviewDialogController::GetInstance();
+    WebContents* web_contents =
+        dialog_controller->GetPrintPreviewForContents(tab);
+    return web_contents;
+  }
+
+  // Gets the PrintPreviewUI so that certain elements can be accessed.
+  PrintPreviewUI* GetUI() {
+    return static_cast<PrintPreviewUI*>(
+        GetDialog()->GetWebUI()->GetController());
+  }
+
+  // Calls a javascript function that will change the print preview settings,
+  // such as the layout, the margins, page numbers, etc.
+  void ManipulatePreviewSettings() {
+    base::DictionaryValue script_argument;
+
+    if (state_ == kWaitingToSendSaveAsPdf) {
+      script_argument.SetBoolean("selectSaveAsPdfDestination", true);
+      state_ = settings_->is_already_pdf ?
+               kWaitingToSendPageNumbers :
+               kWaitingToSendLayoutSettings;
+    } else if (state_ == kWaitingToSendLayoutSettings) {
+      script_argument.SetBoolean("layoutSettings.portrait",
+                                  settings_->is_portrait);
+      state_ = kWaitingToSendPageNumbers;
+    } else if (state_ == kWaitingToSendPageNumbers) {
+      script_argument.SetString("pageRange", settings_->page_numbers);
+      state_ = settings_->is_already_pdf ?
+               kWaitingForFinalMessage :
+               kWaitingToSendHeadersAndFooters;
+    } else if (state_ == kWaitingToSendHeadersAndFooters) {
+      script_argument.SetBoolean("headersAndFooters",
+                                  settings_->headers_and_footers);
+      state_ = kWaitingToSendBackgroundColorsAndImages;
+    } else if (state_ == kWaitingToSendBackgroundColorsAndImages) {
+      script_argument.SetBoolean("backgroundColorsAndImages",
+                                   settings_->background_colors_and_images);
+      state_ = kWaitingToSendMargins;
+    } else if (state_ == kWaitingToSendMargins) {
+      script_argument.SetInteger("margins", settings_->margins);
+      state_ = kWaitingForFinalMessage;
+    } else if (state_ == kWaitingForFinalMessage) {
+      EndLoop();

[Lei Zhang, 2014/07/08 01:11:36]

Return after this? Your code should have failed the DCHECK on line 204 below.

[ivandavid, 2014/07/08 23:24:20]

Done.

+    }
+
+    DCHECK(!script_argument.empty());
+    GetUI()->web_ui()->CallJavascriptFunction(
+        "onManipulateSettingsForTest", script_argument);
+  }
+
+  // Function to set the print preview settings and save them so they
+  // can be sent later. Currently only used in the constructor. Will be
+  // used when creating a test and take command line arguments.
+  // |page_numbers| is a comma separated page range.
+  //    Example: "1-5,9" will print pages 1 through 5 and page 9.
+  //    The pages specified must be less than or equal to the maximum
+  //    page number. An empty string seems to be valid input, however
+  //    further testing will be required to see if that is actually
+  //    true.
+  void SetPrintPreviewSettings(const PrintPreviewSettings& settings) {
+    settings_.reset(new PrintPreviewSettings(settings));
+  }
+
+ private:
+  // Called when the observer gets the IPC message stating that the
+  // page count is ready.
+  // Due to forward declaration problem, the definition of the function
+  // must be separated from the declaration.
+  void OnDidGetPreviewPageCount(
+      const PrintHostMsg_DidGetPreviewPageCount_Params &params);
+
+  void DidCloneToNewWebContents(WebContents* old_web_contents,
+                                WebContents* new_web_contents)
+                                OVERRIDE {

[Dan Beam, 2014/07/07 23:08:11]

indent off on OVERRIDE

[ivandavid, 2014/07/08 01:07:41]

Done.

[Lei Zhang, 2014/07/08 01:11:36]

Or just put it on the previous line...

[ivandavid, 2014/07/08 23:24:20]

Done.

+    Observe(new_web_contents);
+  }
+
+  void WebContentsDestroyed() OVERRIDE {
+    EndLoop();
+  }
+
+  Browser* browser_;
+  base::Closure closure_;
+  scoped_ptr<PrintPreviewSettings> settings_;
+  State state_;
+
+  DISALLOW_COPY_AND_ASSIGN(PrintPreviewObserver);
+};
+
+// Listens for messages from the print preview ui. Its a different
+// set of messages, which is why two different classes are needed for this.
+// When it gets the "UILoadedForTest" message, it prompts the observer to
+// send a new message. When it gets the "UIFailedLoadingForTest" it just
+// calls FAIL() and the test stops.
+class UIDoneLoadingMessageHandler : public content::WebUIMessageHandler {
+ public:
+  explicit UIDoneLoadingMessageHandler(PrintPreviewObserver* observer) :

[Dan Beam, 2014/07/07 23:08:11]

: on next line

[ivandavid, 2014/07/08 01:07:41]

Done.

+      observer_(observer) {}
+
+  virtual ~UIDoneLoadingMessageHandler() {}
+
+  // When a setting has been set succesfully, this is called and
+  // the observer_ is told to send the next setting to be set.
+  void HandleDone(const base::ListValue* /* args */) {
+    observer_->ManipulatePreviewSettings();

[Dan Beam, 2014/07/07 23:08:12]

can |observer_| be NULL?

[ivandavid, 2014/07/08 01:07:41]

No it shouldn't. I added an ASSERT_TRUE to make sure its not.

[ivandavid, 2014/07/08 01:07:41]

Done.

+  }
+
+  // Ends the test because a setting was not set successfully,
+  // therefore, the test shouldn't continue.
+  void HandleFailure(const base::ListValue* /* args */) {
+    FAIL();
+  }
+
+  // Sets up this class to listen for the UILoadedForTest and
+  // UIFailedLoadingForTest messages. These messages are sent
+  // by print_preview.js. On UILoadedForTest, a settings has
+  // been successfully set and its effects on the pdf have been finalized.
+  // On UIFaieldLoadingForTest a setting has not been successfully set
+  // and the test should fail.
+  void RegisterMessages() OVERRIDE {
+    web_ui()->RegisterMessageCallback(
+    "UILoadedForTest",
+    base::Bind(&UIDoneLoadingMessageHandler::HandleDone,
+               base::Unretained(this)));

[Dan Beam, 2014/07/07 23:08:11]

indent off

[ivandavid, 2014/07/08 01:07:41]

Done.

[Lei Zhang, 2014/07/08 01:11:36]

That is, indentation is off on line 280 and downwards.

[ivandavid, 2014/07/08 23:24:20]

Done.

+
+    web_ui()->RegisterMessageCallback(
+    "UIFailedLoadingForTest",
+    base::Bind(&UIDoneLoadingMessageHandler::HandleFailure,
+               base::Unretained(this)));

[Dan Beam, 2014/07/07 23:08:12]

indent off

[ivandavid, 2014/07/08 01:07:41]

Done.

+  }
+
+ private:
+  PrintPreviewObserver* observer_;
+
+  DISALLOW_COPY_AND_ASSIGN(UIDoneLoadingMessageHandler);
+};
+
+// This function needs to be forward declared.

[Dan Beam, 2014/07/07 23:08:11]

I don't think you need this forward if you move UIDoneLoadingMessageHandler
above PrintPreviewObserver

[ivandavid, 2014/07/08 01:07:41]

If I did that, I would have had another forward declaration problem. However, I
found a way around all this. I made UIDoneLoadingMessageHandler a nested class
within PrintPreviewObserver, since its the only class that will use it. It fixes
the forward declaration problem.

[ivandavid, 2014/07/08 01:07:42]

Done.

+void PrintPreviewObserver::OnDidGetPreviewPageCount(
+    const PrintHostMsg_DidGetPreviewPageCount_Params &params) {
+  WebContents* web_contents = GetDialog();
+  PrintPreviewUI* ui = GetUI();
+  ASSERT_TRUE(ui);
+  ASSERT_TRUE(ui->web_ui());
+  Observe(web_contents);
+  ASSERT_TRUE(web_contents);
+  ui->web_ui()->AddMessageHandler(new UIDoneLoadingMessageHandler(this));
+  ui->web_ui()->CallJavascriptFunction("onEnableManipulateSettingsForTest");
+}
+
+class PrintPreviewPdfGeneratedBrowserTest : public InProcessBrowserTest {
+ public:
+  PrintPreviewPdfGeneratedBrowserTest() {}
+  virtual ~PrintPreviewPdfGeneratedBrowserTest() {}
+
+  // Navigates to the web page given, then initiates print preview
+  // and waits for all the settings to be set.
+  void NavigateAndPreview(const base::FilePath::StringType& file_name,
+                          PrintPreviewSettings settings) {
+    print_preview_observer_->SetPrintPreviewSettings(settings);
+    base::FilePath path(file_name);
+    GURL gurl = net::FilePathToFileURL(path);
+
+    ui_test_utils::NavigateToURL(browser(), gurl);
+
+    base::RunLoop loop;
+    print_preview_observer_->set_quit_closure(loop.QuitClosure());
+    chrome::Print(browser());
+    loop.Run();
+  }
+
+  // Prints the webpage to pdf, after the settings have been set.
+  void Print() {
+    ASSERT_FALSE(pdf_file_save_path_.empty());
+    base::RunLoop loop;
+    print_preview_observer_->set_quit_closure(loop.QuitClosure());
+    print_preview_observer_->GetUI()->handler_->FileSelected(
+        pdf_file_save_path_, 0, NULL);
+    loop.Run();
+  }
+
+  // Converts the pdf to a png file, so that the layout test can
+  // do an image diff on this image and a reference image.
+  void PdfToPng() {

[Lei Zhang, 2014/07/08 01:11:36]

PdfToPng() is over 100 lines long. Consider breaking it up into smaller pieces.

[ivandavid, 2014/07/08 23:24:20]

Done.

[ivandavid, 2014/07/08 23:24:21]

I turned lines 344-366 into a different setup function that is called at the
beginning of the test. Then I made lines 443 to 466 into another function also.
The function comes out to about 90 lines now. Unfortunately its still large, but
a lot of the code can't really be split into different functions.

+    // Get the library functions to get the pdf info.
+    base::ScopedNativeLibrary pdf_lib;
+    base::FilePath pdf_module_path;
+
+    ASSERT_TRUE(PathService::Get(chrome::FILE_PDF_PLUGIN, &pdf_module_path));
+    ASSERT_TRUE(base::PathExists(pdf_module_path));
+    pdf_lib.Reset(base::LoadNativeLibrary(pdf_module_path, NULL));
+
+    ASSERT_TRUE(pdf_lib.is_valid());
+    pdf_to_bitmap_func_ =
+        reinterpret_cast<PDFPageToBitmap>(
+            pdf_lib.GetFunctionPointer("RenderPDFPageToBitmap"));
+
+    pdf_doc_info_func_ =
+        reinterpret_cast<GetPDFDocInfoProc>(
+            pdf_lib.GetFunctionPointer("GetPDFDocInfo"));
+
+    pdf_page_size_func_ =
+        reinterpret_cast<GetPDFPageSizeByIndexProc>(
+            pdf_lib.GetFunctionPointer("GetPDFPageSizeByIndex"));
+
+    ASSERT_TRUE(pdf_to_bitmap_func_);
+    ASSERT_TRUE(pdf_doc_info_func_);
+    ASSERT_TRUE(pdf_page_size_func_);
+
+    std::string data;
+    int num_pages;
+    ASSERT_TRUE(base::ReadFileToString(pdf_file_save_path_, &data));
+    ASSERT_TRUE(pdf_doc_info_func_(data.data(),
+                                   data.size(),
+                                   &num_pages,

[Lei Zhang, 2014/07/08 01:11:36]

ASSERT |num_pages| is positive just in case.

[ivandavid, 2014/07/08 23:24:20]

Done.

+                                   NULL));
+    std::vector<uint8_t> bitmap_data;
+    double min_width = std::numeric_limits<double>::max();
+    double total_height = 0;
+
+    for (int i = 0; i < num_pages; i++) {
+      double height, width;

[Lei Zhang, 2014/07/08 01:11:36]

Since we generally think of sizes as WxH, maybe declare and access them that
order when the ordering doesn't matter otherwise.

[ivandavid, 2014/07/08 23:24:21]

Done. I didn't realize how annoying this was until I decided to read it again.

+      ASSERT_TRUE(pdf_page_size_func_(
+          data.data(), data.size(), i, &width, &height));
+
+      // Can't seem to access printing/units.h due to linking difficulties.
+      // This will do for now.

[Dan Beam, 2014/07/07 23:08:11]

i think you simply need to add PRINTING_EXPORT to the constants in
printing/units.h

[ivandavid, 2014/07/08 01:07:41]

I actually had problems accessing the function I needed, not the constants. I
added PRINTING_EXPORT to the function and now it works.

[ivandavid, 2014/07/08 01:07:41]

Done.

+
+      const double kPointsPerInch = 72.0;
+      const double kPixelsPerInch = 96.0;
+
+      height = height * kPixelsPerInch / kPointsPerInch;

[Lei Zhang, 2014/07/08 01:11:36]

Use printing::ConvertUnit() ?

[ivandavid, 2014/07/08 23:24:20]

Done.

+      width = width * kPixelsPerInch / kPointsPerInch;
+      bool autorotate = false;

[Lei Zhang, 2014/07/08 01:11:36]

Can you explain why we need to rotate landscape images?

[ivandavid, 2014/07/08 23:24:20]

Added a comment.

+
+      if (height < width) {
+        double temp = height;

[Lei Zhang, 2014/07/08 01:11:36]

use std::swap()

[ivandavid, 2014/07/08 23:24:20]

Done.

+        height = width;
+        width = temp;
+        autorotate = true;
+      }
+
+      total_height += height;
+
+      if (width < min_width)
+        min_width = width;
+
+      gfx::Rect rect(width, height);
+      printing::PdfRenderSettings settings(rect, 300, true);

[Lei Zhang, 2014/07/08 01:11:36]

What is 300? Is it the DPI? Make it a constant please.

[ivandavid, 2014/07/08 23:24:21]

Done.

+
+      // Detecting overflow. If a * b > max_size then there is overflow.
+      // So just check if a > max_size / b.
+      // Need to check both the cases in which GetArea() overflows and where
+      // GetArea() * 4 overflows. If the latter is just checked, then it is
+      // possible that if GetArea() overflows, the latter case will pass by
+      // virtue of it overflowing to a value of 1 or something like that.
+      // Also, for the first comparison, height is promoted to size_t.
+      size_t max_size = std::numeric_limits<size_t>::max();

[ivandavid, 2014/07/03 03:12:03]

I think I did this correctly. I feel like my reasoning is correct about having
to do two overflow checks, however I am not sure.

+      if (static_cast<size_t>(settings.area().size().width()) >

[Lei Zhang, 2014/07/08 01:11:36]

Can't you just call area().width() and skip the size() call in the moddile?

[Lei Zhang, 2014/07/08 01:11:36]

Why do you need to cast this to a size_t?

[ivandavid, 2014/07/08 23:24:20]

settings.area().width() is a signed type, however we are comparing it to an
unsigned type, size_t. Therefore it has to be cast so it can compile. 

However, now that I noticed this, I think comparing it to size_t would be
incorrect, since if it overflowed, it could be negative, then when we cast it
would no longer be causing some weird stuff to happen. I'll have to look into
this more.

[ivandavid, 2014/07/08 23:24:21]

Done.

+          max_size / settings.area().size().height() ||
+          static_cast<size_t>(settings.area().size().GetArea()) >
+          max_size / 4) {
+        LOG(ERROR) << "OVERFLOW DETECTED: IMAGE WIDTH OR HEIGHT IS TOO LARGE!";

[Lei Zhang, 2014/07/08 01:11:36]

You can just FAIL() << "oh noes!";

[ivandavid, 2014/07/08 23:24:20]

Done.

+        FAIL();
+      }
+
+      std::vector<uint8_t> page_bitmap_data(
+          4 * settings.area().size().GetArea());
+
+      ASSERT_TRUE(pdf_to_bitmap_func_(data.data(),
+                                      data.size(),
+                                      i,
+                                      page_bitmap_data.data(),
+                                      settings.area().size().width(),
+                                      settings.area().size().height(),
+                                      settings.dpi(),
+                                      settings.dpi(),
+                                      autorotate));
+
+      bitmap_data.insert(bitmap_data.end(),
+                         page_bitmap_data.begin(),
+                         page_bitmap_data.end());
+    }
+
+    std::string hash_input(bitmap_data.begin(), bitmap_data.end());
+    base::MD5Sum(static_cast<void*>(const_cast<char*>(hash_input.data())),

[Lei Zhang, 2014/07/08 01:11:37]

Do you need the const_cast? base::MD5Sum() takes a const void*

[ivandavid, 2014/07/08 23:24:20]

Done.

+                 hash_input.size(),
+                 &hash_);
+
+    gfx::Rect png_rect(min_width, total_height);

[Lei Zhang, 2014/07/08 01:11:36]

If you start with a 2 page PDF file source, and the PDFs are 8x11 and 7x11. This
would end up being 7x22. Doesn't that mean there's a 1x11 portion that'll get
cut off?

[ivandavid, 2014/07/08 23:24:20]

Done. I made it so that the bitmap gets filled in with a blank space so it keeps
the proportions. So, in the example you gave, the 7x11 will be expanded to an
8x11 image. However, rather than stretching the image, in the 1x11 spot, there
will just be white space, preserving the size of the image.

+
+    std::string comment_title("tEXtchecksum\x00");
+    gfx::PNGCodec::Comment hash_comment(
+        comment_title,
+        base::MD5DigestToBase16(hash_));
+
+    std::vector<gfx::PNGCodec::Comment> comments;
+    comments.push_back(hash_comment);
+
+    ASSERT_TRUE(
+        gfx::PNGCodec::Encode(static_cast<unsigned char*>(
+            bitmap_data.data()),
+            gfx::PNGCodec::FORMAT_BGRA,
+            png_rect.size(),
+            png_rect.size().width() * sizeof(uint32_t),
+            false,
+            comments,
+            &output_));
+  }
+
+  // Sends the png image to the layout test framework for comparison.
+  void SendPng() {
+    // Send image header & hash_
+    printf("Content-Type: image/png\n");
+    printf("ActualHash: %s\n", base::MD5DigestToBase16(hash_).data());
+    printf("Content-Length: %" PRIuS "\n", output_.size());
+
+    for (size_t i = 0; i < output_.size(); ++i)
+      printf("%c", output_[i]);
+
+    printf("#EOF\n");
+    fflush(stdout);
+    fprintf(stderr, "#EOF\n");
+    fflush(stderr);
+  }
+
+  // Duplicates the tab that was created when the browser opened.
+  // This is done, so that the observer can listen to the duplicated
+  // tab as soon as possible and start listening for messages related to
+  // print preview.
+  void DuplicateTab() {
+    WebContents* tab =
+        browser()->tab_strip_model()->GetActiveWebContents();
+    ASSERT_TRUE(tab);
+
+    print_preview_observer_.reset(
+        new PrintPreviewObserver(tab, browser()));

[Lei Zhang, 2014/07/08 01:11:36]

This fits on the previous line just fine before...

[ivandavid, 2014/07/08 23:24:20]

Done.

+    chrome::DuplicateTab(browser());
+
+    WebContents* initiator =
+        browser()->tab_strip_model()->GetActiveWebContents();
+    ASSERT_TRUE(initiator);
+    ASSERT_NE(tab, initiator);
+  }
+
+  // Resets the test so that another web page can be printing.
+  // Deletes the duplicate tab as it isn't needed anymore.
+  void Clean() {

[Dan Beam, 2014/07/07 23:08:11]

nit: Reset()

[ivandavid, 2014/07/08 01:07:41]

Done.

+    output_.clear();
+    ASSERT_EQ(browser()->tab_strip_model()->count(), 2);
+    chrome::CloseTab(browser());
+    ASSERT_EQ(browser()->tab_strip_model()->count(), 1);
+  }
+
+  // Creates a temporary directory to store the file that will be used for
+  // stdin to accept input. Also sets up the path to save the pdf file
+  // that will be printed. Everything is cleaned up automatically once
+  // the test ends.
+  void SetupStdinAndSavePath() {
+    printf("#READY\n");

[Dan Beam, 2014/07/07 23:08:11]

what is this doing?

[ivandavid, 2014/07/08 01:07:41]

See comment on line 615.

+    fflush(stdout);
+
+    ASSERT_TRUE(tmp_dir_.CreateUniqueTempDir());
+    ASSERT_TRUE(base::CreateTemporaryFileInDir(tmp_dir_.path(), &tmp_path_));
+    ASSERT_TRUE(freopen(tmp_path_.value().c_str(), "r", stdin));
+
+    pdf_file_save_path_ = tmp_dir_.path().AppendASCII("dummy.pdf");
+
+    printf("StdinPath: %s\n#EOF\n", tmp_path_.value().c_str());
+    fflush(stdout);
+  }
+
+ private:
+  scoped_ptr<PrintPreviewObserver> print_preview_observer_;
+  base::FilePath pdf_file_save_path_;
+
+  typedef bool (*PDFPageToBitmap) (const void * pdf_buffer,

[Dan Beam, 2014/07/07 23:08:10]

nit: ) ( -> )(

[Lei Zhang, 2014/07/08 01:11:36]

and void * -> void*

[Lei Zhang, 2014/07/08 01:11:36]

And name it PDFPageToBitmapProc to be consistent with the other two function
pointer types.

[ivandavid, 2014/07/08 23:24:21]

Done.

+                                   int pdf_buffer_size,
+                                   int page_number,
+                                   void* bitmap_buffer,
+                                   int bitmap_width,
+                                   int bitmap_height,
+                                   int dpi_x,
+                                   int dpi_y,
+                                   bool autorotate);
+
+  typedef bool (*GetPDFDocInfoProc)(const void* pdf_buffer,
+                                    int buffer_size,
+                                    int* pages_count,
+                                    double* max_page_width);
+
+  typedef bool (*GetPDFPageSizeByIndexProc)(const void* pdf_buffer,
+                                            int buffer_size,
+                                            int index,
+                                            double* width,
+                                            double* height);
+
+  PDFPageToBitmap pdf_to_bitmap_func_;

[Lei Zhang, 2014/07/08 01:11:36]

Document your member variables unless they are completely obvious. I'd group the
three foo_func_ ones together with 1 comment.

[ivandavid, 2014/07/08 23:24:20]

Done.

+  GetPDFDocInfoProc pdf_doc_info_func_;
+  GetPDFPageSizeByIndexProc pdf_page_size_func_;
+  std::vector<unsigned char> output_;
+  base::MD5Digest hash_;
+  base::ScopedTempDir tmp_dir_;
+  base::FilePath tmp_path_;
+
+  DISALLOW_COPY_AND_ASSIGN(PrintPreviewPdfGeneratedBrowserTest);
+};
+
+IN_PROC_BROWSER_TEST_F(PrintPreviewPdfGeneratedBrowserTest,
+                       MANUAL_DummyTest) {

[ivandavid, 2014/07/03 03:12:03]

Now a manual test since it should only be run with the layout test framework.

[Dan Beam, 2014/07/07 23:08:11]

what's the point of a manual test like this?

[ivandavid, 2014/07/08 01:07:41]

It was a suggestion by Dirk Pranke that he made in a design document for the
layout test modifications that I am making. I shared the design document with
you. If you click comments after opening the doc, you'll see it.

+  // What this code is supposed to do: Setup communication with the

[Dan Beam, 2014/07/07 23:08:10]

make into an ordered list:
- much organized
- so pretty
- wow

[ivandavid, 2014/07/08 01:07:41]

Done.

+  // layout test framework, print webpage to a pdf, convert that
+  // pdf to a png, then send that png file to the layout test framework,
+  // where the framework will do an image diff.
+

[Dan Beam, 2014/07/07 23:08:11]

remove \n

[ivandavid, 2014/07/08 01:07:41]

Done.

+  SetupStdinAndSavePath();
+
+  // There is no way to determine how many tests are to be run
+  // ahead of time without a ton of modifications to the layout
+  // test framework. However, whenever it is done with a set of tests,
+  // it calls SIGKILL on the browser test that ran those set of tests.
+  // Thus, this while loop will end once the layout test framework
+  // decides to actually kill this process. For this to work,
+  // the test must be run with '--single_process'. There must be an
+  // underscore, not a dash, as that is something different.
+  while (true) {
+    DuplicateTab();
+
+    base::FilePath::StringType cmd;
+    std::getline(STDIN_STREAM, cmd);
+    if (cmd.empty()) {
+      while (STDIN_STREAM.eof()) {
+        STDIN_STREAM.clear();
+        std::getline(STDIN_STREAM, cmd);
+        if (!cmd.empty()) {
+          break;
+        }
+      }
+    }
+
+    // TODO(ivandavid): Get settings from file in resources.

[ivandavid, 2014/07/03 03:12:03]

I will do this once I start actually writing tests and figure out what settings
I might want to add, on top of these settings, such as DPI settings if the image
is too large and eats up too much memory.

+    PrintPreviewSettings settings(
+        true,
+        "",
+        false,
+        false,
+        printing::DEFAULT_MARGINS,
+        cmd.find(".pdf") != base::FilePath::StringType::npos);
+
+    std::vector<base::FilePath::StringType> cmd_arguments;
+    base::SplitString(cmd, '\'', &cmd_arguments);
+    base::FilePath::StringType test_name(cmd_arguments[0]);
+    NavigateAndPreview(test_name, settings);
+    Print();
+    PdfToPng();
+
+    printf("#EOF\n");

[Dan Beam, 2014/07/07 23:08:12]

what is this doing?

[ivandavid, 2014/07/08 01:07:41]

All of the printf("#EOF\n") statements are for layout tests. I added a bunch of
comments explaining why these are there in general and I also documented each
instance of a call to printf("#EOF\n") to explain which specific content block
has been sent.

+    fflush(stdout);
+
+    SendPng();
+    Clean();
+  }
+}