[Downloads] Move client guid for AV scanning of downloaded files to chrome/

content/browser/download/download_manager_impl_unittest.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 <set>
 #include <string>
 
 #include "base/bind.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/memory/scoped_ptr.h"
@@ skipping 91 matching lines @@
   MOCK_METHOD1(StealDangerousDownload, void(const AcquireFileCallback&));
   MOCK_METHOD3(UpdateProgress, void(int64, int64, const std::string&));
   MOCK_METHOD1(Cancel, void(bool));
   MOCK_METHOD0(MarkAsComplete, void());
   MOCK_METHOD1(OnAllDataSaved, void(const std::string&));
   MOCK_METHOD0(OnDownloadedFileRemoved, void());
   virtual void Start(
       scoped_ptr<DownloadFile> download_file,
       scoped_ptr<DownloadRequestHandleInterface> req_handle) OVERRIDE {
     MockStart(download_file.get(), req_handle.get());
+    last_file_.swap(download_file);
   }
 
   MOCK_METHOD2(MockStart, void(DownloadFile*, DownloadRequestHandleInterface*));
 
   MOCK_METHOD0(Remove, void());
   MOCK_CONST_METHOD1(TimeRemaining, bool(base::TimeDelta*));
   MOCK_CONST_METHOD0(CurrentSpeed, int64());
   MOCK_CONST_METHOD0(PercentComplete, int());
   MOCK_CONST_METHOD0(AllDataSaved, bool());
   MOCK_CONST_METHOD1(MatchesQuery, bool(const string16& query));
@@ skipping 41 matching lines @@
   MOCK_CONST_METHOD0(GetLastReason, DownloadInterruptReason());
   MOCK_CONST_METHOD0(GetBrowserContext, BrowserContext*());
   MOCK_CONST_METHOD0(GetWebContents, WebContents*());
   MOCK_CONST_METHOD0(GetFileNameToReportUser, base::FilePath());
   MOCK_METHOD1(SetDisplayName, void(const base::FilePath&));
   MOCK_METHOD0(NotifyRemoved, void());
   // May be called when vlog is on.
   virtual std::string DebugString(bool verbose) const OVERRIDE {
     return std::string();
   }
+
+ private:
+  scoped_ptr<DownloadFile> last_file_;
 };
 
 class MockDownloadManagerDelegate : public DownloadManagerDelegate {
  public:
   MockDownloadManagerDelegate();
   virtual ~MockDownloadManagerDelegate();
 
   MOCK_METHOD0(Shutdown, void());
   MOCK_METHOD1(GetNextId, void(const DownloadIdCallback&));
   MOCK_METHOD2(DetermineDownloadTarget,
                bool(DownloadItem* item,
                     const DownloadTargetCallback&));
   MOCK_METHOD1(ShouldOpenFileBasedOnExtension, bool(const base::FilePath&));
   MOCK_METHOD2(ShouldCompleteDownload,
                bool(DownloadItem*, const base::Closure&));
   MOCK_METHOD2(ShouldOpenDownload,
                bool(DownloadItem*, const DownloadOpenDelayedCallback&));
   MOCK_METHOD0(GenerateFileHash, bool());
   MOCK_METHOD4(GetSaveDir, void(BrowserContext*,
                                 base::FilePath*, base::FilePath*, bool*));
   MOCK_METHOD5(ChooseSavePath, void(
       WebContents*, const base::FilePath&, const base::FilePath::StringType&,
       bool, const SavePackagePathPickedCallback&));
+
+  virtual std::string ApplicationClientIdForFileScanning() const {

[asanka, 2013/08/08 21:16:01]

Use a mock and set expectations so this is consistent with the rest of the test.

[Greg Billock, 2013/08/08 23:55:46]

Done.

+    return std::string("client-id");
+  }
 };
 
 MockDownloadManagerDelegate::MockDownloadManagerDelegate() {}
 
 MockDownloadManagerDelegate::~MockDownloadManagerDelegate() {}
 
 class MockDownloadItemFactory
     : public DownloadItemFactory,
       public base::SupportsWeakPtr<MockDownloadItemFactory> {
  public:
@@ skipping 161 matching lines @@
 
   virtual DownloadFile* CreateFile(
       scoped_ptr<DownloadSaveInfo> save_info,
       const base::FilePath& default_download_directory,
       const GURL& url,
       const GURL& referrer_url,
       bool calculate_hash,
       scoped_ptr<ByteStreamReader> stream,
       const net::BoundNetLog& bound_net_log,
       base::WeakPtr<DownloadDestinationObserver> observer) {
-    return MockCreateFile(*save_info.get(), default_download_directory, url,
-                          referrer_url, calculate_hash,
-                          stream.get(), bound_net_log, observer);
+    MockCreateFile(*save_info.get(), default_download_directory, url,
+                   referrer_url, calculate_hash,
+                   stream.get(), bound_net_log, observer);
+    mock_file_ = new MockDownloadFile();
+    return mock_file_;
   }
+
+  MockDownloadFile* GetCreatedDownloadFile() {
+    return mock_file_;
+  }
+
+ private:
+  MockDownloadFile* mock_file_;
 };
 
 class MockBrowserContext : public BrowserContext {
  public:
   MockBrowserContext() {}
   ~MockBrowserContext() {}
 
   MOCK_CONST_METHOD0(GetPath, base::FilePath());
   MOCK_CONST_METHOD0(IsOffTheRecord, bool());
   MOCK_METHOD0(GetRequestContext, net::URLRequestContextGetter*());
@@ skipping 191 matching lines @@
       .WillOnce(Return());
   EXPECT_CALL(GetMockDownloadManagerDelegate(), GetNextId(_))
       .WillOnce(RunCallback<0>(local_id));
 
   // Doing nothing will set the default download directory to null.
   EXPECT_CALL(GetMockDownloadManagerDelegate(), GetSaveDir(_, _, _, _));
   EXPECT_CALL(GetMockDownloadManagerDelegate(), GenerateFileHash())
       .WillOnce(Return(true));
   EXPECT_CALL(*mock_download_file_factory_.get(),
               MockCreateFile(Ref(*info->save_info.get()), _, _, _, true,
                              stream.get(), _, _));

[asanka, 2013/08/08 21:16:01]

Create a MockDownloadFile here, set its expectations to receive the client ID,
and then return it from the MockCreateFile call.

Something like:

MockDownloadFile* mock_file = new MockDownloadFile;
EXPECT_CALL(*mock_file, SetClientGuid(std::string("client-id")));

EXPECT_CALL(..., MockCreateFile(...))
  .WillOnce(Return(mock_file));

[Greg Billock, 2013/08/08 23:55:46]

Done, but as an aside I think these tests would be improved by less use of
gmock.

 
   download_manager_->StartDownload(
       info.Pass(), stream.Pass(), DownloadUrlParameters::OnStartedCallback());
-  EXPECT_TRUE(download_manager_->GetDownload(local_id));
+  EXPECT_TRUE(NULL != download_manager_->GetDownload(local_id));

[asanka, 2013/08/08 21:16:01]

Is this change necessary?

[Greg Billock, 2013/08/08 23:55:46]

I'll put it back.

+
+  // Make sure the client app GUID was set in the DownloadItem.
+  MockDownloadFile* mock_file =
+      mock_download_file_factory_->GetCreatedDownloadFile();
+  EXPECT_EQ("client-id", mock_file->client_guid());
 }
 
 // Confirm that calling DetermineDownloadTarget behaves properly if the delegate
 // blocks starting.
 TEST_F(DownloadManagerTest, DetermineDownloadTarget_True) {
   // Put a mock we have a handle to on the download manager.
   MockDownloadItemImpl& item(AddItemToManager());
   EXPECT_CALL(item, GetState())
       .WillRepeatedly(Return(DownloadItem::IN_PROGRESS));
 
@@ skipping 55 matching lines @@
       .WillOnce(Return());
   EXPECT_CALL(GetMockDownloadItem(3), Remove())
       .Times(0);
 
   download_manager_->RemoveAllDownloads();
   // Because we're mocking the download item, the Remove call doesn't
   // result in them being removed from the DownloadManager list.
 }
 
 }  // namespace content