[Downloads] Add real observers to MockDownloadItem.

[Downloads] Add real observers to MockDownloadItem.

Managing observer lists is needed by all the tests that end up calling
DownloadItem::AddObserver. This CL adds real observers to
MockDownloadItem so that tests don't need to mock this behavior.

BUG=367311
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=269585

[Randy Smith (Not in Mondays), 2014-04-23 19:28:35]

LGTM with below comments.

https://codereview.chromium.org/248713004/diff/1/chrome/browser/download/down...
File chrome/browser/download/download_path_reservation_tracker_unittest.cc
(right):

https://codereview.chromium.org/248713004/diff/1/chrome/browser/download/down...
chrome/browser/download/download_path_reservation_tracker_unittest.cc:333:
item2->UpdateObservers();
You do this often enough that maybe a helper function is appropriate? 
Static/anonymous namespace seems fine to me; no need for any fancy inheritance
or anything.

https://codereview.chromium.org/248713004/diff/1/content/public/test/mock_dow...
File content/public/test/mock_download_item.cc (right):

https://codereview.chromium.org/248713004/diff/1/content/public/test/mock_dow...
content/public/test/mock_download_item.cc:34: }
nit: The file originally had vertical whitespace before the namespace closing
curly.  I'm ok with your making that change if it was intentionally, but I
personally prefer the whitespace.

https://codereview.chromium.org/248713004/diff/1/content/public/test/mock_dow...
File content/public/test/mock_download_item.h (right):

https://codereview.chromium.org/248713004/diff/1/content/public/test/mock_dow...
content/public/test/mock_download_item.h:29: virtual void UpdateObservers()
OVERRIDE;
I'd cluster this with the below and name it somewhat differently.  We're not
actually updating the observers; we're notifying the observers that the download
has been updated.  I'd suggest
"NotifyObserversDownload{Updated,Opened,Removed}".

[Randy Smith (Not in Mondays), 2014-04-23 19:30:14]

I find myself wondering if the MockDownloadManager could use a similar change. 
I haven't checked, but I do think that this cleans up a noticeable amount of
test code.

[asanka, 2014-04-25 21:31:27]

I'll do another CL for the MockDownloadManager.

https://codereview.chromium.org/248713004/diff/1/chrome/browser/download/down...
File chrome/browser/download/download_path_reservation_tracker_unittest.cc
(right):

https://codereview.chromium.org/248713004/diff/1/chrome/browser/download/down...
chrome/browser/download/download_path_reservation_tracker_unittest.cc:333:
item2->UpdateObservers();
On 2014/04/23 19:28:35, rdsmith wrote:
> You do this often enough that maybe a helper function is appropriate? 
> Static/anonymous namespace seems fine to me; no need for any fancy inheritance
> or anything.

Done. Added a SetDownloadItemState() function.

https://codereview.chromium.org/248713004/diff/1/content/public/test/mock_dow...
File content/public/test/mock_download_item.cc (right):

https://codereview.chromium.org/248713004/diff/1/content/public/test/mock_dow...
content/public/test/mock_download_item.cc:34: }
On 2014/04/23 19:28:35, rdsmith wrote:
> nit: The file originally had vertical whitespace before the namespace closing
> curly.  I'm ok with your making that change if it was intentionally, but I
> personally prefer the whitespace.

Added it back. It was 'git cl format' that removed the whitespace.

https://codereview.chromium.org/248713004/diff/1/content/public/test/mock_dow...
File content/public/test/mock_download_item.h (right):

https://codereview.chromium.org/248713004/diff/1/content/public/test/mock_dow...
content/public/test/mock_download_item.h:29: virtual void UpdateObservers()
OVERRIDE;
On 2014/04/23 19:28:35, rdsmith wrote:
> I'd cluster this with the below and name it somewhat differently.  We're not
> actually updating the observers; we're notifying the observers that the
download
> has been updated.  I'd suggest
> "NotifyObserversDownload{Updated,Opened,Removed}".

Done.

[asanka, 2014-04-25 22:19:16]

Adding OWNERS:

mattm:
  chrome/browser/safe_browsing/download_protection_service_unittest.cc

phajdan.jr:
  content/public/test/mock_download_item.cc
  content/public/test/mock_download_item.h

[mattm, 2014-04-25 23:30:33]

safe_browsing lgtm

[Paweł Hajdan Jr., 2014-04-28 10:58:59]

LGTM

[asanka, 2014-05-07 23:07:53]

bauerb: Could you take a look at plugin_installer_unittest?

[Bernhard Bauer, 2014-05-09 14:25:04]

chrome/browser/plugins LGTM, thanks!

[asanka, 2014-05-09 15:00:05]

The CQ bit was checked by asanka@chromium.org

[commit-bot: I haz the power, 2014-05-09 15:07:55]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/asanka@chromium.org/248713004/40001

[commit-bot: I haz the power, 2014-05-10 18:04:53]

Change committed as 269585