Print Preview: Making margin selection sticky (part 2/2)

chrome/browser/ui/webui/print_preview_handler_unittest.cc

+// Copyright (c) 2011 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 "base/command_line.h"
+#include "base/json/json_writer.h"
+#include "base/values.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/printing/background_printing_manager.h"
+#include "chrome/browser/printing/print_preview_tab_controller.h"
+#include "chrome/browser/ui/browser_list.h"
+#include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h"
+#include "chrome/browser/ui/webui/print_preview_handler.h"
+#include "chrome/browser/ui/webui/print_preview_ui.h"
+#include "chrome/common/chrome_switches.h"
+#include "chrome/test/base/browser_with_test_window_test.h"
+#include "printing/page_size_margins.h"
+#include "printing/print_job_constants.h"
+
+typedef BrowserWithTestWindowTest PrintPreviewHandlerTest;
+
+// When users hit print in the print preview tab, the print preview tab hides
+// and the focus should return to the initiator tab.
+TEST_F(PrintPreviewHandlerTest, HandlePrintTest) {

[kmadhusu, 2011/11/11 23:39:40]

Your test case is fine. But you can add more information to it. 

I expected the following sequence:
1. Open a initiator tab.
2. Create a preview tab.
3. Call HandlePrint().
4. Delete the hidden preview tab.
5. Preview the initiator tab again.
6. Query/Test the last used margin type and custom margins value. 

You can create 3 test cases using this sequence:

Test case 1:
User set default margins during the first print request.
Expect the default margin type in the next preview tab.

Test case 2:
User set custom margins during the first print request.
Expect the same custom margins type in the next preview tab.

Test case 3:
User set custom margins during the first print request.
User set default margins during the next print request.
During the next preview request, the last used margin type should be default but
we would still retain the last used margin settings. 

[dpapad, 2011/11/14 16:31:55]

Done.

+#if !defined(GOOGLE_CHROME_BUILD) || defined(OS_CHROMEOS)
+  CommandLine::ForCurrentProcess()->AppendSwitch(switches::kEnablePrintPreview);
+#endif
+  ASSERT_TRUE(browser());
+  BrowserList::SetLastActive(browser());
+  ASSERT_TRUE(BrowserList::GetLastActive());
+
+  browser()->NewTab();
+  EXPECT_EQ(1, browser()->tab_count());
+  TabContentsWrapper* initiator_tab =
+      browser()->GetSelectedTabContentsWrapper();
+  ASSERT_TRUE(initiator_tab);
+
+  scoped_refptr<printing::PrintPreviewTabController>
+      controller(new printing::PrintPreviewTabController());
+  ASSERT_TRUE(controller.get());
+
+  TabContentsWrapper* preview_tab =
+      controller->GetOrCreatePreviewTab(initiator_tab);
+  ASSERT_TRUE(preview_tab);
+  EXPECT_EQ(2, browser()->tab_count());
+
+  PrintPreviewUI* preview_ui =
+      static_cast<PrintPreviewUI*>(preview_tab->web_ui());
+  ASSERT_TRUE(preview_ui);
+
+  // Set the minimal dummy settings to make the HandlePrint() code happy.
+  DictionaryValue settings;
+  settings.SetBoolean(printing::kSettingPreviewModifiable, true);
+  settings.SetInteger(printing::kSettingColor, printing::COLOR);
+  settings.SetBoolean(printing::kSettingPrintToPDF, false);
+  settings.SetInteger(printing::kSettingMarginsType, printing::CUSTOM_MARGINS);
+
+  // Creating custom margins dictionary and nesting it in |settings|.
+  std::string custom_margins_root(printing::kSettingMarginsCustom);
+  custom_margins_root.append(".");
+  std::stringstream ss;

[kmadhusu, 2011/11/11 23:39:40]

Any special reason to use the stringstream?

You can use the Set() member function of DictionaryValue class to add a
dictionary to a DictionaryValue.

[dpapad, 2011/11/14 16:31:55]

Done. I was following the instructions on the top of values.h to create nested
DictionaryValues. Set() works as well and it is less code.

+  ss << custom_margins_root << printing::kSettingMarginTop;
+  settings.SetDouble(ss.str(), 25.5);

[Lei Zhang, 2011/11/11 22:56:30]

make the values constants?

[dpapad, 2011/11/14 16:31:55]

Done.

+  ss.str(std::string());
+  ss << custom_margins_root << printing::kSettingMarginRight;
+  settings.SetDouble(ss.str(), 26.5);
+  ss.str(std::string());
+  ss << custom_margins_root << printing::kSettingMarginBottom;
+  settings.SetDouble(ss.str(), 27.5);
+  ss.str(std::string());
+  ss << custom_margins_root << printing::kSettingMarginLeft;
+  settings.SetDouble(ss.str(), 28.5);
+
+  // Put |settings| in to |args| as a JSON string.
+  std::string json_string;
+  base::JSONWriter::Write(&settings, false, &json_string);
+  ListValue args;
+  args.Append(new base::StringValue(json_string)); // |args| takes ownership.

[Lei Zhang, 2011/11/11 22:56:30]

nit: 2 spaces

[dpapad, 2011/11/14 16:31:55]

Done.

+  preview_ui->handler_->HandlePrint(&args);
+  EXPECT_EQ(1, browser()->tab_count());
+
+  // Checking that sticky settings were saved correctly.
+  EXPECT_EQ(PrintPreviewHandler::last_used_color_model_, printing::COLOR);
+  EXPECT_EQ(PrintPreviewHandler::last_used_margins_type_,
+            printing::CUSTOM_MARGINS);
+  EXPECT_EQ(PrintPreviewHandler::last_used_page_size_margins_->margin_top,
+            25.5);
+  EXPECT_EQ(PrintPreviewHandler::last_used_page_size_margins_->margin_right,
+            26.5);
+  EXPECT_EQ(PrintPreviewHandler::last_used_page_size_margins_->margin_bottom,
+            27.5);
+  EXPECT_EQ(PrintPreviewHandler::last_used_page_size_margins_->margin_left,
+            28.5);
+
+  printing::BackgroundPrintingManager* bg_printing_manager =

[Lei Zhang, 2011/11/11 22:56:30]

Might want a comment to explain why this is here and why you are deleting
|preview_tab|.

[dpapad, 2011/11/14 16:31:55]

Done.

+      g_browser_process->background_printing_manager();
+  ASSERT_TRUE(bg_printing_manager->HasPrintPreviewTab(preview_tab));
+
+  delete preview_tab;
+}