Fix base::Bind/Callback -> Once/Repeating for regressed directories

Use of base::Bind and base::Callback is deprecated in favor of the more
explicit Once/Repeating versions. A presubmit enforces this, except on
a set of files and directories that have not yet been converted. This
list, however, has not always been updated when a directory is migrated.
Unfortunately, a few such directories have since regressed.

This cl fixes those regressions and removes the legacy exemption for the
directories to prevent any future regressions.

Bug: 1141533, 1007678, 1007729, 1007733, 1007793
Change-Id: I5efa002606ebacb4efa9db14016cc96f3ac72fb7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2500563
Commit-Queue: Alex Turner <alexmt@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821535}

PRESUBMIT.py

@@ -375,13 +375,10 @@ _NOT_CONVERTED_TO_MODERN_BIND_AND_CALLBACK = '|'.join((
   '^chromeos/components/',
   '^components/cast_channel/',
   '^components/component_updater/',
-  '^components/content_settings/',
   '^components/drive/',
   '^components/ownership/',
   '^components/policy/',
   '^components/search_engines/',
-  '^components/security_interstitials/',
-  '^components/signin/',
   '^components/sync/',
   '^components/ukm/',
   '^components/webcrypto/',
@@ -389,7 +386,6 @@ _NOT_CONVERTED_TO_MODERN_BIND_AND_CALLBACK = '|'.join((
   '^extensions/renderer/',
   '^google_apis/drive/',
   '^ios/chrome/',
-  '^ios/components/',
   '^ios/web/',
   '^ios/web_view/',
   '^ipc/',

components/content_settings/renderer/content_settings_agent_impl.cc

@@ -112,8 +112,9 @@ ContentSettingsAgentImpl::ContentSettingsAgentImpl(
   render_frame->GetWebFrame()->SetContentSettingsClient(this);
 
   render_frame->GetAssociatedInterfaceRegistry()->AddInterface(
-      base::Bind(&ContentSettingsAgentImpl::OnContentSettingsAgentRequest,
-                 base::Unretained(this)));
+      base::BindRepeating(
+          &ContentSettingsAgentImpl::OnContentSettingsAgentRequest,
+          base::Unretained(this)));
 
   content::RenderFrame* main_frame =
       render_frame->GetRenderView()->GetMainRenderFrame();

components/content_settings/renderer/content_settings_agent_impl_browsertest.cc

@@ -304,7 +304,8 @@ TEST_F(ContentSettingsAgentImplBrowserTest, JSBlockSentAfterPageLoad) {
   // has not yet been sent at the time when the navigation commits.
   CommitTimeConditionChecker checker(
       view_->GetMainRenderFrame(),
-      base::Bind(HasSentOnContentBlocked, base::Unretained(&mock_agent)),
+      base::BindRepeating(HasSentOnContentBlocked,
+                          base::Unretained(&mock_agent)),
       false);
 
   std::string url_str = "data:text/html;charset=utf-8,";

components/security_interstitials/content/captive_portal_metrics_recorder.cc

@@ -38,7 +38,7 @@ CaptivePortalMetricsRecorder::CaptivePortalMetricsRecorder(
       captive_portal_no_response_(false),
       captive_portal_detected_(false) {
   captive_portal_detection_enabled_ = captive_portal_service->enabled();
-  subscription_ = captive_portal_service->RegisterCallback(base::Bind(
+  subscription_ = captive_portal_service->RegisterCallback(base::BindRepeating(
       &CaptivePortalMetricsRecorder::Observe, base::Unretained(this)));
 }
 

components/security_interstitials/content/common_name_mismatch_handler.cc

@@ -34,9 +34,8 @@ CommonNameMismatchHandler::~CommonNameMismatchHandler() {
 CommonNameMismatchHandler::TestingState
     CommonNameMismatchHandler::testing_state_ = NOT_TESTING;
 
-void CommonNameMismatchHandler::CheckSuggestedUrl(
-    const GURL& url,
-    const CheckUrlCallback& callback) {
+void CommonNameMismatchHandler::CheckSuggestedUrl(const GURL& url,
+                                                  CheckUrlCallback callback) {
   // Should be used only in tests.
   if (testing_state_ == IGNORE_REQUESTS_FOR_TESTING)
     return;
@@ -46,7 +45,7 @@ void CommonNameMismatchHandler::CheckSuggestedUrl(
   DCHECK(check_url_callback_.is_null());
 
   check_url_ = url;
-  check_url_callback_ = callback;
+  check_url_callback_ = std::move(callback);
 
   // Create traffic annotation tag.
   net::NetworkTrafficAnnotationTag traffic_annotation =

components/security_interstitials/content/common_name_mismatch_handler.h

@@ -46,8 +46,8 @@ class CommonNameMismatchHandler {
     IGNORE_REQUESTS_FOR_TESTING
   };
 
-  typedef base::Callback<void(SuggestedUrlCheckResult result,
-                              const GURL& suggested_url)>
+  typedef base::OnceCallback<void(SuggestedUrlCheckResult result,
+                                  const GURL& suggested_url)>
       CheckUrlCallback;
 
   CommonNameMismatchHandler(
@@ -57,7 +57,7 @@ class CommonNameMismatchHandler {
 
   // Performs a network request to suggested URL. After completion, runs the
   // |callback|.
-  void CheckSuggestedUrl(const GURL& url, const CheckUrlCallback& callback);
+  void CheckSuggestedUrl(const GURL& url, CheckUrlCallback callback);
 
   // Determines if, for |request_url| serving a certificate that is valid for
   // the domain names |dns_names|, there is a name that the certificate is

components/security_interstitials/content/ssl_error_handler.cc

@@ -377,7 +377,7 @@ class SSLErrorHandlerDelegateImpl : public SSLErrorHandler::Delegate {
                        GURL* suggested_url) const override;
   void CheckSuggestedUrl(
       const GURL& suggested_url,
-      const CommonNameMismatchHandler::CheckUrlCallback& callback) override;
+      CommonNameMismatchHandler::CheckUrlCallback callback) override;
   void NavigateToSuggestedURL(const GURL& suggested_url) override;
   bool IsErrorOverridable() const override;
   void ShowCaptivePortalInterstitial(const GURL& landing_url) override;
@@ -448,14 +448,15 @@ bool SSLErrorHandlerDelegateImpl::GetSuggestedUrl(
 
 void SSLErrorHandlerDelegateImpl::CheckSuggestedUrl(
     const GURL& suggested_url,
-    const CommonNameMismatchHandler::CheckUrlCallback& callback) {
+    CommonNameMismatchHandler::CheckUrlCallback callback) {
   scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory(
       content::BrowserContext::GetDefaultStoragePartition(browser_context_)
           ->GetURLLoaderFactoryForBrowserProcess());
   common_name_mismatch_handler_.reset(
       new CommonNameMismatchHandler(request_url_, url_loader_factory));
 
-  common_name_mismatch_handler_->CheckSuggestedUrl(suggested_url, callback);
+  common_name_mismatch_handler_->CheckSuggestedUrl(suggested_url,
+                                                   std::move(callback));
 }
 
 void SSLErrorHandlerDelegateImpl::NavigateToSuggestedURL(
@@ -773,8 +774,8 @@ void SSLErrorHandler::StartHandlingError() {
       }
       delegate_->CheckSuggestedUrl(
           suggested_url,
-          base::Bind(&SSLErrorHandler::CommonNameMismatchHandlerCallback,
-                     weak_ptr_factory_.GetWeakPtr()));
+          base::BindOnce(&SSLErrorHandler::CommonNameMismatchHandlerCallback,
+                         weak_ptr_factory_.GetWeakPtr()));
       timer_.Start(FROM_HERE, g_config.Pointer()->interstitial_delay(), this,
                    &SSLErrorHandler::ShowSSLInterstitial);
 
@@ -790,7 +791,7 @@ void SSLErrorHandler::StartHandlingError() {
 
 #if BUILDFLAG(ENABLE_CAPTIVE_PORTAL_DETECTION)
   subscription_ = captive_portal_service_->RegisterCallback(
-      base::Bind(&SSLErrorHandler::Observe, base::Unretained(this)));
+      base::BindRepeating(&SSLErrorHandler::Observe, base::Unretained(this)));
 
   captive_portal::CaptivePortalTabHelper* captive_portal_tab_helper =
       captive_portal::CaptivePortalTabHelper::FromWebContents(web_contents_);
@@ -954,8 +955,8 @@ void SSLErrorHandler::DeleteSSLErrorHandler() {
 void SSLErrorHandler::HandleCertDateInvalidError() {
   const base::TimeTicks now = base::TimeTicks::Now();
   timer_.Start(FROM_HERE, g_config.Pointer()->interstitial_delay(),
-               base::Bind(&SSLErrorHandler::HandleCertDateInvalidErrorImpl,
-                          base::Unretained(this), now));
+               base::BindOnce(&SSLErrorHandler::HandleCertDateInvalidErrorImpl,
+                              base::Unretained(this), now));
   // Try kicking off a time fetch to get an up-to-date estimate of the
   // true time. This will only have an effect if network time is
   // unavailable or if there is not already a query in progress.

components/security_interstitials/content/ssl_error_handler.h

@@ -69,7 +69,8 @@ extern const base::Feature kCaptivePortalCertificateList;
 class SSLErrorHandler : public content::WebContentsUserData<SSLErrorHandler>,
                         public content::WebContentsObserver {
  public:
-  typedef base::Callback<void(content::WebContents*)> TimerStartedCallback;
+  typedef base::RepeatingCallback<void(content::WebContents*)>
+      TimerStartedCallback;
   typedef base::OnceCallback<void(
       std::unique_ptr<security_interstitials::SecurityInterstitialPage>)>
       BlockingPageReadyCallback;
@@ -116,7 +117,7 @@ class SSLErrorHandler : public content::WebContentsUserData<SSLErrorHandler>,
                                  GURL* suggested_url) const = 0;
     virtual void CheckSuggestedUrl(
         const GURL& suggested_url,
-        const CommonNameMismatchHandler::CheckUrlCallback& callback) = 0;
+        CommonNameMismatchHandler::CheckUrlCallback callback) = 0;
     virtual void NavigateToSuggestedURL(const GURL& suggested_url) = 0;
     virtual bool IsErrorOverridable() const = 0;
     virtual void ShowCaptivePortalInterstitial(const GURL& landing_url) = 0;

components/security_interstitials/content/ssl_error_handler_unittest.cc

@@ -122,9 +122,10 @@ const char kCertWithoutOrganizationOrCommonName[] =
 // Runs |quit_closure| on the UI thread once a URL request has been
 // seen. Returns a request that hangs.
 std::unique_ptr<net::test_server::HttpResponse> WaitForRequest(
-    const base::Closure& quit_closure,
+    base::OnceClosure quit_closure,
     const net::test_server::HttpRequest& request) {
-  content::GetUIThreadTaskRunner({})->PostTask(FROM_HERE, quit_closure);
+  content::GetUIThreadTaskRunner({})->PostTask(FROM_HERE,
+                                               std::move(quit_closure));
   return std::make_unique<net::test_server::HungResponse>();
 }
 
@@ -170,7 +171,7 @@ class TestSSLErrorHandlerDelegate : public SSLErrorHandler::Delegate {
   void SendSuggestedUrlCheckResult(
       const CommonNameMismatchHandler::SuggestedUrlCheckResult& result,
       const GURL& suggested_url) {
-    suggested_url_callback_.Run(result, suggested_url);
+    std::move(suggested_url_callback_).Run(result, suggested_url);
   }
 
   int captive_portal_checked() const { return captive_portal_checked_; }
@@ -259,10 +260,10 @@ class TestSSLErrorHandlerDelegate : public SSLErrorHandler::Delegate {
 
   void CheckSuggestedUrl(
       const GURL& suggested_url,
-      const CommonNameMismatchHandler::CheckUrlCallback& callback) override {
+      CommonNameMismatchHandler::CheckUrlCallback callback) override {
     DCHECK(suggested_url_callback_.is_null());
     suggested_url_checked_ = true;
-    suggested_url_callback_ = callback;
+    suggested_url_callback_ = std::move(callback);
   }
 
   void NavigateToSuggestedURL(const GURL& suggested_url) override {
@@ -1109,7 +1110,7 @@ TEST_F(SSLErrorHandlerDateInvalidTest, MAYBE_TimeQueryStarted) {
   // Enable network time queries and handle the error. A bad clock interstitial
   // should be shown.
   test_server()->RegisterRequestHandler(
-      base::Bind(&network_time::GoodTimeResponseHandler));
+      base::BindRepeating(&network_time::GoodTimeResponseHandler));
   EXPECT_TRUE(test_server()->Start());
   tracker()->SetTimeServerURLForTesting(test_server()->GetURL("/"));
   field_trial_test()->SetNetworkQueriesWithVariationsService(
@@ -1173,8 +1174,8 @@ TEST_F(SSLErrorHandlerDateInvalidTest, MAYBE_TimeQueryHangs) {
   // network time cannot be determined before the timer elapses, an SSL
   // interstitial should be shown.
   base::RunLoop wait_for_time_query_loop;
-  test_server()->RegisterRequestHandler(
-      base::Bind(&WaitForRequest, wait_for_time_query_loop.QuitClosure()));
+  test_server()->RegisterRequestHandler(base::BindRepeating(
+      &WaitForRequest, wait_for_time_query_loop.QuitClosure()));
   EXPECT_TRUE(test_server()->Start());
   tracker()->SetTimeServerURLForTesting(test_server()->GetURL("/"));
   field_trial_test()->SetNetworkQueriesWithVariationsService(

components/signin/internal/identity_manager/account_fetcher_service.cc

@@ -78,8 +78,9 @@ void AccountFetcherService::Initialize(
   repeating_timer_ = std::make_unique<signin::PersistentRepeatingTimer>(
       signin_client_->GetPrefs(), AccountFetcherService::kLastUpdatePref,
       kRefreshFromTokenServiceDelay,
-      base::Bind(&AccountFetcherService::RefreshAllAccountInfo,
-                 base::Unretained(this), /*only_fetch_if_invalid=*/false));
+      base::BindRepeating(&AccountFetcherService::RefreshAllAccountInfo,
+                          base::Unretained(this),
+                          /*only_fetch_if_invalid=*/false));
 
   // Tokens may have already been loaded and we will not receive a
   // notification-on-registration for |token_service_->AddObserver(this)| few

components/signin/ios/browser/account_consistency_service_unittest.mm

@@ -135,7 +135,7 @@ class TestWebState : public web::TestWebState {
     __block web::WebStatePolicyDecider::PolicyDecision policyDecision =
         web::WebStatePolicyDecider::PolicyDecision::Allow();
     auto callback =
-        base::Bind(^(web::WebStatePolicyDecider::PolicyDecision decision) {
+        base::BindOnce(^(web::WebStatePolicyDecider::PolicyDecision decision) {
           policyDecision = decision;
         });
     decider_->ShouldAllowResponse(response, for_main_frame,

components/signin/public/base/persistent_repeating_timer.cc

@@ -30,8 +30,8 @@ void PersistentRepeatingTimer::Start() {
     OnTimerFired();
   } else {
     timer_.Start(FROM_HERE, delay_ - time_since_update,
-                 base::Bind(&PersistentRepeatingTimer::OnTimerFired,
-                            base::Unretained(this)));
+                 base::BindRepeating(&PersistentRepeatingTimer::OnTimerFired,
+                                     base::Unretained(this)));
   }
   DCHECK(timer_.IsRunning());
 }

components/signin/public/base/persistent_repeating_timer_unittest.cc

@@ -42,8 +42,8 @@ class PersistentRepeatingTimerTest : public ::testing::Test {
 TEST_F(PersistentRepeatingTimerTest, MissingPref) {
   PersistentRepeatingTimer timer(
       &pref_service_, kLastUpdatedTimePref, kTestDelay,
-      base::Bind(&PersistentRepeatingTimerTest::RunTask,
-                 base::Unretained(this)));
+      base::BindRepeating(&PersistentRepeatingTimerTest::RunTask,
+                          base::Unretained(this)));
   CheckCallCount(0);
 
   // The task is run immediately on start.
@@ -62,8 +62,8 @@ TEST_F(PersistentRepeatingTimerTest, MissingPref) {
 TEST_F(PersistentRepeatingTimerTest, MultipleStarts) {
   PersistentRepeatingTimer timer(
       &pref_service_, kLastUpdatedTimePref, kTestDelay,
-      base::Bind(&PersistentRepeatingTimerTest::RunTask,
-                 base::Unretained(this)));
+      base::BindRepeating(&PersistentRepeatingTimerTest::RunTask,
+                          base::Unretained(this)));
   CheckCallCount(0);
 
   // The task is run immediately on start.
@@ -91,8 +91,8 @@ TEST_F(PersistentRepeatingTimerTest, RecentPref) {
 
   PersistentRepeatingTimer timer(
       &pref_service_, kLastUpdatedTimePref, kTestDelay,
-      base::Bind(&PersistentRepeatingTimerTest::RunTask,
-                 base::Unretained(this)));
+      base::BindRepeating(&PersistentRepeatingTimerTest::RunTask,
+                          base::Unretained(this)));
   CheckCallCount(0);
 
   // The task is NOT run immediately on start.
@@ -118,8 +118,8 @@ TEST_F(PersistentRepeatingTimerTest, OldPref) {
 
   PersistentRepeatingTimer timer(
       &pref_service_, kLastUpdatedTimePref, kTestDelay,
-      base::Bind(&PersistentRepeatingTimerTest::RunTask,
-                 base::Unretained(this)));
+      base::BindRepeating(&PersistentRepeatingTimerTest::RunTask,
+                          base::Unretained(this)));
   CheckCallCount(0);
 
   // The task is run immediately on start.

ios/components/io_thread/ios_io_thread.mm

@@ -72,8 +72,8 @@
 #error "This file requires ARC support."
 #endif
 
-// The IOSIOThread object must outlive any tasks posted to the IO thread
-// before the Quit task, so base::Bind() calls are not refcounted.
+// The IOSIOThread object must outlive any tasks posted to the IO thread before
+// the Quit task, so base::Bind{Once,Repeating}() calls are not refcounted.
 
 namespace io_thread {
 

ios/components/security_interstitials/lookalikes/lookalike_url_tab_helper_unittest.mm

@@ -42,7 +42,7 @@ class LookalikeUrlTabHelperTest : public PlatformTest {
     __block web::WebStatePolicyDecider::PolicyDecision policy_decision =
         web::WebStatePolicyDecider::PolicyDecision::Allow();
     auto callback =
-        base::Bind(^(web::WebStatePolicyDecider::PolicyDecision decision) {
+        base::BindOnce(^(web::WebStatePolicyDecider::PolicyDecision decision) {
           policy_decision = decision;
           callback_called = true;
         });