Fixed issue with DownloadTestObserver.

chrome/browser/download/download_test_observer.h

 // 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.
 
 #ifndef CHROME_BROWSER_DOWNLOAD_DOWNLOAD_TEST_OBSERVER_H_
 #define CHROME_BROWSER_DOWNLOAD_DOWNLOAD_TEST_OBSERVER_H_
 #pragma once
 
 #include <set>
 
 #include "base/basictypes.h"
 #include "base/memory/ref_counted.h"
 #include "content/public/browser/download_item.h"
 #include "content/public/browser/download_manager.h"
 
 // Construction of this class defines a system state, based on some number
 // of downloads being seen in a particular state + other events that
 // may occur in the download system.  That state will be recorded if it
 // occurs at any point after construction.  When that state occurs, the class
 // is considered finished.  Callers may either probe for the finished state, or
 // wait on it.

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

You are changing the behavior of this class, so you need to update the comments
that describe that behavior.  That's either here, around WaitForFinished(), or
both.

[ahendrickson, 2012/03/02 17:29:41]

Done.

 //
 // TODO(rdsmith): Detect manager going down, remove pointer to

[ahendrickson, 2012/03/02 17:29:41]

Does my change fix this issue?  I think so, because Shutdown() canceling the
downloads should make IsFinished() return true.

 // DownloadManager, transition to finished.  (For right now we
 // just use a scoped_refptr<> to keep it around, but that may cause
 // timeouts on waiting if a DownloadManager::Shutdown() occurs which
 // cancels our in-progress downloads.)
 class DownloadTestObserver : public content::DownloadManager::Observer,
                              public content::DownloadItem::Observer {
  public:
   // Action an observer should take if a dangerous download is encountered.
   enum DangerousDownloadAction {
     ON_DANGEROUS_DOWNLOAD_ACCEPT,  // Accept the download
@@ skipping 32 matching lines @@
   virtual void OnDownloadOpened(content::DownloadItem* download) OVERRIDE {}
 
   // content::DownloadManager::Observer
   virtual void ModelChanged(content::DownloadManager* manager) OVERRIDE;
 
   virtual void SelectFileDialogDisplayed(
       content::DownloadManager* manager, int32 id) OVERRIDE;
 
   size_t NumDangerousDownloadsSeen() const;
 
+  // If we finish other than because we met the condtions, call this to find
+  // out how many rogue downloads there were.
+  size_t NumOtherDownloadsSeen() const;

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

In context, it isn't clear what "rogue downloads" are, so it's not clear how to
use this new call.

[ahendrickson, 2012/03/02 17:29:41]

Changed the comment.

+
  private:
   typedef std::set<content::DownloadItem*> DownloadSet;
 
   // Called when we know that a download item is in a final state.
   // Note that this is not the same as it first transitioning in to the
   // final state; multiple notifications may occur once the item is in
   // that state.  So we keep our own track of transitions into final.
   void DownloadInFinalState(content::DownloadItem* download);
 
+  // Called when we think the download item is in a terminal state.
+  void DownloadInTerminalState(content::DownloadItem* download);
+
   void SignalIfFinished();
 
   // The observed download manager.
   scoped_refptr<content::DownloadManager> download_manager_;
 
+  // The set of |DownloadItem|s that have transitioned to a final state before
+  // we start.  Will not overlap with |finished_downloads_|.
+  DownloadSet previously_finished_downloads_;
+
+  // The set of |DownloadItem|s that have transitioned to a final state without
+  // having hit the 'finished' state.  Will not overlap with
+  // |finished_downloads_|.
+  // This is so the test will finish even if it doesn't have the expected
+  // result.
+  DownloadSet finished_other_downloads_;
+
   // The set of DownloadItem's that have transitioned to their finished state
   // since construction of this object.  When the size of this array
   // reaches wait_count_, we're done.
   DownloadSet finished_downloads_;
 
   // The set of DownloadItem's we are currently observing.  Generally there
   // won't be any overlap with the above; once we see the final state
   // on a DownloadItem, we'll stop observing it.
   DownloadSet downloads_observed_;
 
@@ skipping 75 matching lines @@
   void PingIOThread(int cycle);
 
   content::DownloadManager* download_manager_;
   DownloadSet downloads_observed_;
   bool waiting_for_zero_inprogress_;
 
   DISALLOW_COPY_AND_ASSIGN(DownloadTestFlushObserver);
 };
 
 #endif  // CHROME_BROWSER_DOWNLOAD_DOWNLOAD_TEST_OBSERVER_H_