Print Preview: Add test to ensure only 1 preview is started at init

chrome/test/data/webui/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.
 
 GEN('#include "base/feature_list.h"');
 GEN('#include "chrome/common/chrome_features.h"');
 
 /**
  * Test fixture for print preview WebUI testing.
  * @constructor
@@ skipping 1235 matching lines @@
   // appears.
   var advancedSettingsCloseButton = $('advanced-settings').
       querySelector('.close-button');
   checkElementDisplayed(advancedSettingsCloseButton, true);
   checkElementDisplayed($('advanced-settings').
       querySelector('.search-box-area'), true);
 
   this.waitForAnimationToEnd('more-settings');
 });
 
+// Test that initialization with saved destination only issues one call
+// to startPreview.
+TEST_F('PrintPreviewWebUITest', 'TestInitIssuesOneRequest', function() {
+  // Load in a bunch of recent destinations with non null capabilities.
+  this.initialSettings_.serializedAppStateStr_ =

[dpapad, 2017/01/13 19:55:10]

Let's simplify how serializedAppStateStr_ is populated. I already gave a
suggestion at a different CL, see my comment at
https://codereview.chromium.org/2606043004/diff/120001/chrome/test/data/webui....

[rbpotter, 2017/01/13 21:25:01]

Done.

+      '{"version":2,"recentDestinations":[{"id":"ID1", "origin":"local",' +
+        '"account":"",' +
+        '"capabilities":' + JSON.stringify(getCddTemplate("ID1")) +
+        ', "name":"", "extensionId":"",' +
+            '"extensionName":""},' +
+      '{"id":"ID2", "origin":"local","account":"",' +
+        '"capabilities":' + JSON.stringify(getCddTemplate("ID2")) +
+        ', "name":"", "extensionId":"","extensionName":""},' +
+      '{"id":"ID3", "origin":"local","account":"",' +
+        '"capabilities":' + JSON.stringify(getCddTemplate("ID3")) +
+        ', "name":"", "extensionId":"", "extensionName":""}]}';
+  this.setCapabilities(getCddTemplate('ID1'));
+  this.setCapabilities(getCddTemplate('ID2'));
+  this.setCapabilities(getCddTemplate('ID3'));
+
+  var previewGenerator = mock(print_preview.PreviewGenerator);
+  printPreview.previewArea_.previewGenerator_ = previewGenerator.proxy();
+
+  // Initializing print preview initializes documentInfo. This triggers

[dpapad, 2017/01/13 19:55:10]

This wall of text is a bit hard to read. Can you add some bullet points to make
it easier?

 - 2 calls for ...
 - 3 calls for ...
 ...
 Total 18....

[rbpotter, 2017/01/13 21:25:02]

Done. Let me know if it is still unclear or should be trimmed down.

[dpapad, 2017/01/13 21:37:40]

The text is fine now, I can read it with less effort.

But there is one thing I still don't understand. The CL description says that
there is only 1 preview at init, but there are 18 calls to requestPreview. That
confuses me.

Is there a requestPreview() and startPreview() and if so, what is their
difference?

[rbpotter, 2017/01/13 22:52:19]

Yes. requestPreview() checks a number of conditions (if the preview will have
changed due to the ticket item changes, if the ticket store has been
initialized, and if a destination is selected) to see if a preview request
should actually be sent. If so, it calls startPreview() which is handled by the
native layer and sends the preview request. So in this case all but one request
is filtered out, so only one call is made to startPreview(). The bug occurred
because after a destination had been set and everything was initialized, another
destination selection event was fired, and because the destination changed,
requestPreview called startPreview again.

Ideally it would be better to look at the number of calls to startPreview but
the native layer cannot be replaced with a mock here because it is needed to
send various events in setInitialSettings. 

+  // TicketItem.CHANGE events from all 10 ticket items that depend on
+  // documentInfo. 2 additional TicketItem.CHANGE events are dispatched by the
+  // headerFooter ticket item since it issues a change event each time
+  // marginsType and customMargins do. A documentInfo.CHANGE event is also
+  // dispatched. All these events trigger a call to requestPreview = 13 total.
+  // When the initial destination is selected, this triggers a
+  // CAPABILITIES_CHANGE event and 4 TicketItem.CHANGE events from the 4 ticket
+  // items that depend on destinationStore = 5 additional requestPreview calls.
+  // These 18 calls result in 1 startPreview call as the destination must be
+  // initialized before it will be called.
+  // This checks that only 1 CAPABILITIES_CHANGE event is issued in print
+  // preview initialization so that we do not end up with 2 startPreview calls.
+  // If more events are triggered, there will be additional calls to
+  // requestPreview that will trigger additional calls to startPreview.
+  // Multiple startPreview creates a race condition: see crbug.com/666595.
+  previewGenerator.expects(exactly(18)).requestPreview();
+
+  this.setInitialSettings();
+  testDone();
+});