Fix flaky DownloadTest.FeedbackServiceKeepDownload

chrome/browser/download/download_browsertest.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_browsertest.h"
 
 #include <stdint.h>
 #include <sstream>
 #include <utility>
 
@@ skipping 3419 matching lines @@
       *(downloads[0])));
 
   // Begin feedback and check that the file is "stolen".
   download_protection_service->feedback_service()->BeginFeedbackForDownload(
       downloads[0], DownloadCommands::DISCARD);
   std::vector<DownloadItem*> updated_downloads;
   GetDownloads(browser(), &updated_downloads);
   ASSERT_TRUE(updated_downloads.empty());
 }
 
-// Test is flaky Linux. crbug.com/705224
-#if defined(OS_LINUX)
-#define MAYBE_FeedbackServiceKeepDownload DISABLED_FeedbackServiceKeepDownload
-#else
-#define MAYBE_FeedbackServiceKeepDownload FeedbackServiceKeepDownload
-#endif
-IN_PROC_BROWSER_TEST_F(DownloadTest, MAYBE_FeedbackServiceKeepDownload) {
+IN_PROC_BROWSER_TEST_F(DownloadTest, FeedbackServiceKeepDownload) {
   PrefService* prefs = browser()->profile()->GetPrefs();
   prefs->SetBoolean(prefs::kSafeBrowsingEnabled, true);
   safe_browsing::SetExtendedReportingPref(prefs, true);
 
   // Make a dangerous file.
   GURL download_url(net::URLRequestMockHTTPJob::GetMockUrl(
       "downloads/dangerous/dangerous.swf"));
   std::unique_ptr<content::DownloadTestObserverInterrupted>
       interruption_observer(new content::DownloadTestObserverInterrupted(
           DownloadManagerForBrowser(browser()), 1,
@@ skipping 26 matching lines @@
       sb_service->download_protection_service();
   download_protection_service->feedback_service()->MaybeStorePingsForDownload(
       safe_browsing::DownloadProtectionService::UNCOMMON,
       true /* upload_requested */, downloads[0], ping_request, ping_response);
   ASSERT_TRUE(safe_browsing::DownloadFeedbackService::IsEnabledForDownload(
       *(downloads[0])));
 
   // Begin feedback and check that file is still there.
   download_protection_service->feedback_service()->BeginFeedbackForDownload(
       downloads[0], DownloadCommands::KEEP);
+  std::unique_ptr<DownloadTestObserverNotInProgress> completion_observer(

[asanka, 2017/03/31 14:37:24]

Instead, could you use a DownloadTestObserverTerminal ? That can begin observing
before calling BeginFeedbackForDownload.

The Dangerous -> SAFE -> Complete transition should then deterministically
completed after the download file has been copied and renamed.

Also, you can avoid the IsFinished()->StartObserving()->WaitForFinished()
sequence.

[Jialiu Lin, 2017/03/31 21:34:33]

Done.

[Jialiu Lin, 2017/03/31 21:34:33]

Done.

+      new DownloadTestObserverNotInProgress(
+          DownloadManagerForBrowser(browser()), 1));
+
+  // Make sure download is finished before starting evaluation.
+  if (!completion_observer->IsFinished()) {
+    completion_observer->StartObserving();
+    completion_observer->WaitForFinished();
+  }
 
   std::vector<DownloadItem*> updated_downloads;
   GetDownloads(browser(), &updated_downloads);
   ASSERT_EQ(std::size_t(1), updated_downloads.size());
   ASSERT_FALSE(updated_downloads[0]->IsDangerous());
-  ASSERT_TRUE(PathExists(updated_downloads[0]->GetFullPath()));
+  ASSERT_TRUE(PathExists(updated_downloads[0]->GetTargetFilePath()));
   updated_downloads[0]->Cancel(true);
 }
 
 IN_PROC_BROWSER_TEST_F(DownloadTestWithFakeSafeBrowsing,
                        SendUncommonDownloadReportIfUserProceed) {
   browser()->profile()->GetPrefs()->SetBoolean(prefs::kSafeBrowsingEnabled,
                                                true);
   // Make a dangerous file.
   GURL download_url(
       net::URLRequestMockHTTPJob::GetMockUrl(kDangerousMockFilePath));
@@ skipping 295 matching lines @@
           browser(), 1,
           content::DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_DENY));
   ui_test_utils::NavigateToURL(browser(), extension_url);
 
   observer->WaitForFinished();
 
   // Download shelf should close.
   EXPECT_FALSE(browser()->window()->IsDownloadShelfVisible());
 }
 #endif  // defined(OS_CHROMEOS)