Add browsertest for the feedback service

chrome/browser/download/download_test_file_activity_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_activity_observer.h"
 
 #include "base/bind.h"
 #include "base/message_loop/message_loop.h"
 #include "chrome/browser/download/chrome_download_manager_delegate.h"
 #include "chrome/browser/download/download_service.h"
 #include "chrome/browser/download/download_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 
 namespace content {
 class DownloadItem;
 }
 
 // Test ChromeDownloadManagerDelegate that controls whether how file chooser
 // dialogs are handled, and how files are opend.
 // By default, file chooser dialogs are disabled.
 class DownloadTestFileActivityObserver::MockDownloadManagerDelegate
     : public ChromeDownloadManagerDelegate {
  public:
   explicit MockDownloadManagerDelegate(Profile* profile)
       : ChromeDownloadManagerDelegate(profile),
         file_chooser_enabled_(false),
         file_chooser_displayed_(false),
+        downloads_dangerous_(false),
         weak_ptr_factory_(this) {
     if (!profile->IsOffTheRecord())
       GetDownloadIdReceiverCallback().Run(
           content::DownloadItem::kInvalidId + 1);
   }
 
   virtual ~MockDownloadManagerDelegate() {}
 
   void EnableFileChooser(bool enable) {
     file_chooser_enabled_ = enable;
   }
 
   bool TestAndResetDidShowFileChooser() {
     bool did_show = file_chooser_displayed_;
     file_chooser_displayed_ = false;
     return did_show;
   }
 
   base::WeakPtr<MockDownloadManagerDelegate> GetWeakPtr() {
     return weak_ptr_factory_.GetWeakPtr();
   }
 
+  void ChangeDangerTypeForFeedbackTest() {

[asanka, 2014/03/26 18:01:30]

Suggestion: Use a more generic name since this is a shared API. Perhaps
something like SetDangerTypeForNewDownloads(content::DownloadDangerType type) ?
Make sure MockDownloadManagerDelegate preserves its previous behavior if that
method is never called.

[felt, 2014/06/03 00:35:22]

My problem here is that I don't know how to get the global danger type into the
static SetDangerous callback, which is a DownloadTargetCallback. I don't want to
modify DownloadTargetCallback, nor do I want to make a switch statement and 5
versions of SetDangerous. There must be a cleaner way that I am not thinking of,
any ideas?

[asanka, 2014/06/05 19:20:12]

static void SetDangerous(const DownloadTargetCallback& callback,
      DownloadDangerType forced_danger_type,
      ...);

And when binding the callback:

base::Bind(&SetDangerous, callback, global_danger_type_)

+    downloads_dangerous_ = true;

[asanka, 2014/03/26 18:01:30]

Use a more descriptive name? mark_downloads_as_dangerous_ ?

[felt, 2014/06/03 00:35:22]

Done.

+  }
+
+  virtual bool DetermineDownloadTarget(
+      content::DownloadItem* item,
+      const content::DownloadTargetCallback& callback) OVERRIDE {
+    if (downloads_dangerous_ &&
+        item->GetDangerType() == content::DOWNLOAD_DANGER_TYPE_DANGEROUS_FILE) {

[asanka, 2014/03/26 18:01:30]

If you are forcing a danger type, I'd propose just doing it rather than
conditionalize it the item's current danger type. Also, the danger type isn't
set until ChromeDownloadManagerDelegate::DetermineDownloadTarget() returns.

[felt, 2014/06/03 00:35:22]

I believe this should be fixed now.

+      content::DownloadTargetCallback dangerous_callback =
+          base::Bind(&MockDownloadManagerDelegate::SetDangerous,
+                     callback);
+      return ChromeDownloadManagerDelegate::DetermineDownloadTarget(
+          item, dangerous_callback);
+    } else {
+      return ChromeDownloadManagerDelegate::DetermineDownloadTarget(
+          item, callback);
+    }
+  }
+
+  static void SetDangerous(
+      const content::DownloadTargetCallback& callback,
+      const base::FilePath& target_path,
+      content::DownloadItem::TargetDisposition disp,
+      content::DownloadDangerType danger_type,
+      const base::FilePath& intermediate_path) {
+    callback.Run(target_path,
+                 disp,
+                 content::DOWNLOAD_DANGER_TYPE_UNCOMMON_CONTENT,
+                 intermediate_path);
+  }
+
  protected:
 
   virtual void PromptUserForDownloadPath(content::DownloadItem* item,
                                          const base::FilePath& suggested_path,
                                          const FileSelectedCallback&
                                              callback) OVERRIDE {
     file_chooser_displayed_ = true;
     base::MessageLoop::current()->PostTask(
         FROM_HERE, base::Bind(callback, (file_chooser_enabled_ ? suggested_path
                                          : base::FilePath())));
   }
 
   virtual void OpenDownload(content::DownloadItem* item) OVERRIDE {}
 
  private:
   bool file_chooser_enabled_;
   bool file_chooser_displayed_;
+  bool downloads_dangerous_;
   base::WeakPtrFactory<MockDownloadManagerDelegate> weak_ptr_factory_;
 };
 
 DownloadTestFileActivityObserver::DownloadTestFileActivityObserver(
     Profile* profile) {
   scoped_ptr<MockDownloadManagerDelegate> mock_delegate(
       new MockDownloadManagerDelegate(profile));
   test_delegate_ = mock_delegate->GetWeakPtr();
   DownloadServiceFactory::GetForBrowserContext(profile)->
       SetDownloadManagerDelegateForTesting(
           mock_delegate.PassAs<ChromeDownloadManagerDelegate>());
 }
 
 DownloadTestFileActivityObserver::~DownloadTestFileActivityObserver() {
 }
 
 void DownloadTestFileActivityObserver::EnableFileChooser(bool enable) {
   if (test_delegate_.get())
     test_delegate_->EnableFileChooser(enable);
 }
 
 bool DownloadTestFileActivityObserver::TestAndResetDidShowFileChooser() {
   return test_delegate_.get() &&
       test_delegate_->TestAndResetDidShowFileChooser();
 }
+
+void DownloadTestFileActivityObserver::ChangeDangerTypeForFeedbackTest() {
+  if (test_delegate_.get())
+    test_delegate_->ChangeDangerTypeForFeedbackTest();
+}