Print Preview: Store preview data in the print preview handler....

chrome/browser/dom_ui/print_preview_handler.h

-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// 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.
 
 #ifndef CHROME_BROWSER_DOM_UI_PRINT_PREVIEW_HANDLER_H_
 #define CHROME_BROWSER_DOM_UI_PRINT_PREVIEW_HANDLER_H_
 #pragma once
 
-#include "base/scoped_ptr.h"

[James Hawkins, 2011/01/31 18:39:38]

This is right to remove scoped_ptr.h, but now we need to include ref_counted.h
to get scoped_refptr.

[Lei Zhang, 2011/01/31 22:15:51]

weak_ptr.h includes ref_counted.h, but I added it since we do use scoped_refptr.

+#include <utility>
+
 #include "base/weak_ptr.h"
 #include "chrome/browser/dom_ui/dom_ui.h"
 
+namespace base {
+class SharedMemory;
+}
+
 namespace printing {
 class PrintBackend;
-}  // namespace printing

[James Hawkins, 2011/01/31 18:39:38]

I realize there was disagreement on the list recently about this, but the style
guide still says namespaces should be commented.

[Lei Zhang, 2011/01/31 22:15:51]

I've asked for the style guide to be updated to reflect the decision that having
the comment doesn't make sense for short forward declarations.

+}
 
 // The handler for Javascript messages related to the "print preview" dialog.
 class PrintPreviewHandler : public DOMMessageHandler,
                             public base::SupportsWeakPtr<PrintPreviewHandler> {
  public:
+  typedef std::pair<base::SharedMemory*, uint32> PrintPreviewData;

[James Hawkins, 2011/01/31 18:39:38]

Document this typedef.

[Lei Zhang, 2011/01/31 22:15:51]

Done.

+
   PrintPreviewHandler();
   virtual ~PrintPreviewHandler();
 
+  // Get the print preview |data|. The data is valid as long as the

[James Hawkins, 2011/01/31 18:39:38]

s/Get/Gets/

[Lei Zhang, 2011/01/31 22:15:51]

Done.

+  // PrintPreviewHandler is valid and SetPrintPreviewData() does not get called.
+  void GetPrintPreviewData(PrintPreviewData* data);
+
+  // Save the print preview |data|. PrintPreviewHandler owns the data and is

[James Hawkins, 2011/01/31 18:39:38]

s/Save/Saves/ and now that I look at the method name, this should probably be
"Sets".

[Lei Zhang, 2011/01/31 22:15:51]

There's no function named "SetsFoo". Everything in chrome/ is SetFoo. I.e.
WebPluginProxy::SetCookie(), RenderWidget::SetBackground(),
TabContents::SetIsCrashed(), etc etc.

[James Hawkins, 2011/02/01 01:02:38]

By "this", I was referring to the comment.

+  // responsible for freeing it when either:
+  // a) there is new data.
+  // b) when PrintPreviewHandler goes away.

[James Hawkins, 2011/01/31 18:39:38]

nit: You should probably be more descriptive than 'goes away': is destroyed, is
destructed, etc.

[Lei Zhang, 2011/01/31 22:15:51]

Done.

+  void SetPrintPreviewData(const PrintPreviewData& data);
+
   // DOMMessageHandler implementation.
   virtual void RegisterMessages();
 
  private:
   // Get the PDF preview and refresh the PDF plugin. |args| is unused.
   void HandleGetPreview(const ListValue* args);
 
   // Get the list of printers and send it to the DOM UI. |args| is unused.
   void HandleGetPrinters(const ListValue* args);
 
   // Pointer to current print system.
   scoped_refptr<printing::PrintBackend> print_backend_;
 
+  // Current print preview data, owned by PrintPreviewHandler.
+  PrintPreviewData data_;
+
   DISALLOW_COPY_AND_ASSIGN(PrintPreviewHandler);
 };
 
 #endif  // CHROME_BROWSER_DOM_UI_PRINT_PREVIEW_HANDLER_H_