download: Remove DownloadManager::MockDownloadOpenForTesting

chrome/browser/download/download_test_file_chooser_observer.cc

 // 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.
 
 #include "chrome/browser/download/download_test_file_chooser_observer.h"
 
 #include "base/bind.h"
 #include "base/message_loop.h"
 #include "chrome/browser/download/chrome_download_manager_delegate.h"
 #include "chrome/browser/download/download_service.h"
@@ skipping 32 matching lines @@
                                   const FilePath& suggested_path,
                                   const FileSelectedCallback&
                                       callback) OVERRIDE {
     file_chooser_displayed_ = true;
     MessageLoop::current()->PostTask(
         FROM_HERE,
         base::Bind(callback,
                    (file_chooser_enabled_ ? suggested_path : FilePath())));
   }
 
+  virtual void OpenDownload(content::DownloadItem* item) OVERRIDE {}

[Randy Smith (Not in Mondays), 2013/01/23 21:23:34]

Is this class being used in DownloadTest.AutoOpen?  I don't see how it would be.
 

Mocking out opening is also not really part of this class' concept.  I think the
right thing is to create a new class in
chrome/browser/download/download_browsertest.cc derived from
ChromeDownloadManagerDelegate that just mocks out Open(), and use it in that
test only.

[hashimoto, 2013/01/24 05:52:31]

Both DownloadTest.AutoOpen and DownloadExtensionTest.DownloadExtensionTest_Open
are using this class by instantiating DownloadTestFileChooserObserver in their
test fixture classes.
Since this class is the only user of
DownloadService::SetDownloadManagerDelegateForTesting, and is responsible to
'mock' user-facing download related behavior, I thought this class is the place
where this empty OpenDownload() should be. (Perhaps, it should be better renamed
to MockDownloadManagerDelegate or something.)
Since DownloadTestFileChooserObserver is already setup in
DownloadTest:SetUpOnMainThread for all tests under DownloadTest, I think adding
one empty method here is quite simpler than setting up a new
DownloadManagerDelegate.
The same goes for DownloadExtensionTest.

Thoughts?

+
  private:
   virtual ~MockFileChooserDownloadManagerDelegate() {}
 
   bool file_chooser_enabled_;
   bool file_chooser_displayed_;
 };
 
 }  // namespace internal
 
 DownloadTestFileChooserObserver::DownloadTestFileChooserObserver(
     Profile* profile) {
   test_delegate_ =
       new internal::MockFileChooserDownloadManagerDelegate(profile);
   DownloadServiceFactory::GetForProfile(profile)->
       SetDownloadManagerDelegateForTesting(test_delegate_.get());
 }
 
 DownloadTestFileChooserObserver::~DownloadTestFileChooserObserver() {
 }
 
 void DownloadTestFileChooserObserver::EnableFileChooser(bool enable) {
   test_delegate_->EnableFileChooser(enable);
 }
 
 bool DownloadTestFileChooserObserver::TestAndResetDidShowFileChooser() {
   return test_delegate_->TestAndResetDidShowFileChooser();
 }
-