Refactor dangerous download warning text generation into DownloadItemModel.

chrome/browser/download/download_item_model.h

 // 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_DOWNLOAD_DOWNLOAD_ITEM_MODEL_H_
 #define CHROME_BROWSER_DOWNLOAD_DOWNLOAD_ITEM_MODEL_H_
 #pragma once
 
 #include <string>
 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "base/string16.h"
 
 class SavePackage;
 
 namespace content {
 class DownloadItem;
 }
 
-// This class provides an interface for functions which have different behaviors
-// depending on the type of download.
+namespace gfx {
+class Font;
+}
+
+// This class is an abstraction for common UI tasks associated with a download.
 class BaseDownloadItemModel {
  public:
   explicit BaseDownloadItemModel(content::DownloadItem* download)
       : download_(download) { }
   virtual ~BaseDownloadItemModel() { }
 
   // Cancel the task corresponding to the item.
   virtual void CancelTask() = 0;
 
   // Get the status text to display.
   virtual string16 GetStatusText() = 0;
 
+  // Get the warning text to display for a dangerous download. The |base_width|
+  // is the maximum width of an embedded filename (if there is one). The metrics
+  // for the filename will be based on |font|.
+  virtual string16 GetWarningText(const gfx::Font& font, int base_width) = 0;

[Randy Smith (Not in Mondays), 2012/03/01 21:34:45]

When is this valid to call, in terms of IsDangerous() and IsMalicious() values? 
If it's valid when they are not true, what does it return?

[asanka, 2012/03/01 21:55:33]

It's only valid to call when IsDangerous() is true. I added a comment to this
effect.

+
+  // Is this considered a malicious download?
+  virtual bool IsMalicious() = 0;
+
+  // Is this considered a dangerous download?
+  virtual bool IsDangerous() = 0;
+
   content::DownloadItem* download() { return download_; }
 
  protected:
   content::DownloadItem* download_;
 };
 
 // This class is a model class for DownloadItemView. It provides functionality
 // for canceling the downloading, and also the text for displaying downloading
 // status.
 class DownloadItemModel : public BaseDownloadItemModel {

[Randy Smith (Not in Mondays), 2012/03/01 21:34:45]

Not really part of this CL, but remind me of the inheritance model here?  This
looks as if the impl of BaseDownloadItemModel for views is being declared in the
non-GUI type specific class, which seems wrong.

[asanka, 2012/03/01 21:55:33]

BaseDownloadItemModel and DownloadItemModel are both independent of UI toolkit.
Historically, we used to have a SavePageModel that also implemented
BaseDownloadItemModel. BaseDownloadItemModel was used so that the UI didn't need
to know whether it was talking to a DownloadItem or a SavePackage.

SavePageModel is now no more. Perhaps we should get rid of BaseDownloadItemModel
interface and just use DownloadItemModel.

[Randy Smith (Not in Mondays), 2012/03/01 22:02:32]

That would make sense to me; at a minimum we should clarify the above comment. 
No need for this CL, though.

[asanka, 2012/03/02 17:45:16]

Done.

  public:
   explicit DownloadItemModel(content::DownloadItem* download);
   virtual ~DownloadItemModel() { }
 
-  // Cancel the downloading.
+  // BaseDownloadItemModel
   virtual void CancelTask() OVERRIDE;
-
-  // Get downloading status text.
   virtual string16 GetStatusText() OVERRIDE;
+  virtual string16 GetWarningText(const gfx::Font& font,
+                                  int base_width) OVERRIDE;
+  virtual bool IsMalicious() OVERRIDE;
+  virtual bool IsDangerous() OVERRIDE;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(DownloadItemModel);
 };
 
 #endif  // CHROME_BROWSER_DOWNLOAD_DOWNLOAD_ITEM_MODEL_H_