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

chrome/browser/resources/print_preview/print_preview.js

 // Copyright (c) 2012 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.
 
 // TODO(rltoscano): Move data/* into print_preview.data namespace
 
 var localStrings = new LocalStrings(templateData);
 
 <include src="component.js"/>
 
@@ skipping 267 matching lines @@
           print_preview.NativeLayer.EventType.SETTINGS_INVALID,
           this.onSettingsInvalid_.bind(this));
       this.tracker.add(
           this.nativeLayer_,
           print_preview.NativeLayer.EventType.DISABLE_SCALING,
           this.onDisableScaling_.bind(this));
       this.tracker.add(
           this.nativeLayer_,
           print_preview.NativeLayer.EventType.PRIVET_PRINT_FAILED,
           this.onPrivetPrintFailed_.bind(this));
-
+      this.tracker.add(
+          this.nativeLayer_,
+          print_preview.NativeLayer.EventType.MANIPULATE_SETTINGS_FOR_TEST,
+          this.onManipulateSettingsForTest_.bind(this));
 
       this.tracker.add(
           $('system-dialog-link'),
           'click',
           this.openSystemPrintDialog_.bind(this));
       this.tracker.add(
           $('cloud-print-dialog-link'),
           'click',
           this.onCloudPrintDialogLinkClick_.bind(this));
       this.tracker.add(
@@ skipping 385 matching lines @@
       this.isPreviewGenerationInProgress_ = true;
     },
 
     /**
      * Called when the preview area's preview generation is complete.
      * @private
      */
     onPreviewGenerationDone_: function() {
       this.isPreviewGenerationInProgress_ = false;
       this.printHeader_.isPrintButtonEnabled = true;
+      this.nativeLayer_.previewReadyForTest();
       this.printIfReady_();
     },
 
     /**
      * Called when the preview area's preview failed to load.
      * @private
      */
     onPreviewGenerationFail_: function() {
       this.isPreviewGenerationInProgress_ = false;
       this.printHeader_.isPrintButtonEnabled = false;
@@ skipping 179 matching lines @@
      * @private
      */
     onPrivetPrintFailed_: function(event) {
       console.error('Privet printing failed with error code ' +
                     event.httpError);
       this.printHeader_.setErrorMessage(
         localStrings.getString('couldNotPrint'));
     },
 
     /**
+     * Called when the print preview settings need to be changed for testing.
+     * @param {Event} event Event object that contains the option that is to
+     *     be changed and what to set that option.
+     * @private
+     */
+    onManipulateSettingsForTest_: function(event) {
+      if ('selectSaveAsPdfDestination' in event.settings) {

[Dan Beam, 2014/06/21 02:44:11]

one strategy for splitting this up

if ('selectSaveAsPdfDestination' in event.settings)
  this.selectSaveAsPdfDestination_(event);
else if ('layoutSettings' in event.settings)
  this.clickLayoutSettings_();
else if ('pageRange' in event.settings)
  this.setPageRange_(event);

[Dan Beam, 2014/06/21 02:44:11]

another strategy would be to simply make different methods for all this... is
there any reason you *need* to go through onManipulateSettingsForTest_ for all
this code?

[Aleksey Shlyapnikov, 2014/06/23 19:34:46]

I would support keeping native_layer.js involvement at minimum and do not have
any logic in it. This is test code, it would be great to keep it as small and
'one piece' as possible.
Removing unnecessary "!element" checks and returning early will help to improve
readability of this code.

[ivandavid, 2014/06/24 18:49:52]

I felt that keeping all the code in one function is better for the reasons
Aleksey stated. Having it in one spot makes it difficult to read and makes it a
huge function, at least its obvious where all the test related functionality is
(save for the message passing parts, like the part in
onPrintPreviewGenerationDone_).

+        var destinations = this.destinationStore_.destinations();
+        var didSet = false;
+        for (var i = 0; i < destinations.length; i++) {
+          if (destinations[i].id == print_preview.Destination.GooglePromotedId.
+              SAVE_AS_PDF) {

[Aleksey Shlyapnikov, 2014/06/20 22:13:49]

           if (destinations[i].id ==
               print_preview.Destination.GooglePromotedId.SAVE_AS_PDF) {

[ivandavid, 2014/06/24 18:49:52]

Done.

+            this.destinationStore_.selectDestination(destinations[i]);
+            didSet = true;
+            break;

[Dan Beam, 2014/06/21 02:44:11]

return;

[ivandavid, 2014/06/24 18:49:52]

Done.

+          }
+        }
+        if (!didSet)

[Dan Beam, 2014/06/21 02:44:11]

^ remove didSet

[ivandavid, 2014/06/24 18:49:52]

Done.

+          this.nativeLayer_.previewFailedForTest();

[Aleksey Shlyapnikov, 2014/06/20 22:13:50]

var pdfDestination = null;
...
pdfDestination = destinations[i];
break;
...
if (pdfDestination)
  this.destinationStore_.selectDestination(pdfDestination);
else
  this.nativeLayer_.previewFailedForTest();

is easier to understand.

[ivandavid, 2014/06/24 18:49:52]

Done.

+      } else if ('layoutSettings' in event.settings) {

[Aleksey Shlyapnikov, 2014/06/20 22:13:50]

I know that one liners do not require brackets, but don't you think reading this
code with if and else from the different branches next to each other and with
the same indent is confusing? I'd add brackets around one liner if here for the
sake of readability anyway.

[Dan Beam, 2014/06/21 02:27:51]

an overwhelming majority of chrome/browser/resources disagrees with this

[ivandavid, 2014/06/24 18:49:52]

After removing all the unnecessary !element sort of statements (like 921 for
example), I think I was able to get rid of this problem.
Done.

+        var element = document.querySelector(
+            event.settings.layoutSettings.portrait ?
+            '.layout-settings-portrait-radio' :
+            '.layout-settings-landscape-radio');

[Aleksey Shlyapnikov, 2014/06/20 22:13:50]

4 more spaces indent for these two lines.

+        if (element) {

[Dan Beam, 2014/06/21 02:44:11]

if (!element) {
  this.nativeLayer_.previewFailedForTest();
  return;
}

if (element.clicked)
  this.nativeLayer_.previewReadyForTest();
else
  element.click();

+          if (element.clicked)
+            this.nativeLayer_.previewReadyForTest();
+          else
+            element.click();
+        } else
+          this.nativeLayer_.previewFailedForTest();

[Aleksey Shlyapnikov, 2014/06/20 22:13:49]

I think if one part of the statement has braces the other one should have them
too, even if it's a one liner. Otherwise it's harder to grasp the structure of
the code.

[Dan Beam, 2014/06/21 02:27:51]

yes, should've mentioned this ^

[ivandavid, 2014/06/24 18:49:52]

Done.

+
+      } else if ('pageRange' in event.settings) {
+        var textbox = document.querySelector('.page-settings-custom-input');
+        if (textbox) {

[Dan Beam, 2014/06/21 02:44:11]

if (!textbox) {
  this.nativeLayer_.previewFailedForTest();
  return;
}

... less if indent ...

[Aleksey Shlyapnikov, 2014/06/23 19:34:46]

I don't see my extensive comment on the same matter in this revision, so just in
case, these "!element" checks are not necessary, they can be removed altogether
to save indentation and code. All the elements under test always exist, you
should just query and use them.

[ivandavid, 2014/06/24 18:49:52]

Done.

[ivandavid, 2014/06/24 18:49:52]

I think I finally understand what you are saying and attempted to apply your
suggestion through the code. I think I did it correctly.

+          if (textbox.value == event.settings.pageRange)
+            this.nativeLayer_.previewReadyForTest();
+          else {
+            // Assuming that the string format is correct for now.
+            // TODO: Will need a way to validate it just so the test doesn't
+            //    hang mysteriously if the string isn't correct.

[Aleksey Shlyapnikov, 2014/06/20 22:13:49]

No indent is required.

[ivandavid, 2014/06/24 18:49:52]

I just got rid of the comment.

+            textbox.value = event.settings.pageRange;
+
+            // Triggers the re-rendering of the preview area as
+            // just setting the value isn't sufficient.
+            var radio = document.querySelector('.page-settings-custom-radio');
+            if (radio)

[Dan Beam, 2014/06/21 02:44:11]

if (!radio) { ... usual failure stuff + return ... }

+              radio.click();
+            else
+              this.nativeLayer_.previewFailedForTest();
+          }
+        } else
+          this.nativeLayer_.previewFailedForTest();
+      } else if ('headersAndFooters' in event.settings) {
+        var checkbox = document.querySelector(
+            '.header-footer-checkbox');

[Aleksey Shlyapnikov, 2014/06/20 22:13:49]

Fits one line.

[ivandavid, 2014/06/24 18:49:52]

Done.

[ivandavid, 2014/06/24 18:49:52]

Done.

[ivandavid, 2014/06/24 18:49:52]

Done.

+        if (checkbox) {
+          if (event.settings.headersAndFooters == checkbox.checked)
+            this.nativeLayer_.previewReadyForTest();
+          else
+            checkbox.click();
+        } else
+          this.nativeLayer_.previewFailedForTest();
+      } else if ('backgroundColorsAndImages' in event.settings) {
+        var checkbox = document.querySelector('.css-background-checkbox');
+        if (checkbox) {
+          if (event.settings.backgroundColorsAndImages == checkbox.checked)
+            this.nativeLayer_.previewReadyForTest();
+          else
+            checkbox.click();
+        } else
+          this.nativeLayer_.previewFailedForTest();
+      } else if ('margins' in event.settings) {
+        var element = document.querySelector('.margin-settings-select');
+        if (element) {
+          if (event.settings.margins == element.selectedIndex)
+            this.nativeLayer_.previewReadyForTest();
+          else if (event.settings.margins >= 0 &&
+                     event.settings.margins < element.length) {

[Aleksey Shlyapnikov, 2014/06/20 22:13:50]

Fix indent.

[ivandavid, 2014/06/24 18:49:52]

Done.

+            element.selectedIndex = event.settings.margins;
+            this.marginSettings_.onSelectChange_();
+          } else
+            this.nativeLayer_.previewFailedForTest();
+        } else
+          this.nativeLayer_.previewFailedForTest();
+      }

[Dan Beam, 2014/06/21 02:44:11]

feel free to return early or break this method into smaller methods for
readability (left some examples)

+    },
+
+    /**
      * Called when the open-cloud-print-dialog link is clicked. Opens the Google
      * Cloud Print web dialog.
      * @private
      */
     onCloudPrintDialogLinkClick_: function() {
       assert(this.uiState_ == PrintPreview.UiState_.READY,
              'Opening Google Cloud Print dialog when not in ready state: ' +
                  this.uiState_);
       setIsVisible($('cloud-print-dialog-throbber'), true);
       this.setIsEnabled_(false);
@@ skipping 131 matching lines @@
 <include src="search/recent_destination_list.js"/>
 <include src="search/destination_list_item.js"/>
 <include src="search/destination_search.js"/>
 <include src="search/search_box.js"/>
 <include src="search/fedex_tos.js"/>
 
 window.addEventListener('DOMContentLoaded', function() {
   printPreview = new print_preview.PrintPreview();
   printPreview.initialize();
 });