arc: Removing the last base::Callback/base::Closure in components/arc

This change completely deprecates base::Callback/base::Closure in
components/arc. This also adds a PRESUBMIT.py check to avoid introducing
new base::{Callback,Closure,Bind} in components/arc as well as
chrome/browser/chromeos/arc.

Even though there are still instances of base::Bind() in */arc/*, the
PRESUBMIT.py checker only warns about touched lines, which should allow
us to gradually migrate.

Bug: 714018
Test: git cl try
Change-Id: I7d36f83869513d1a1e0a71bfa898c25ee78b5fa6
Reviewed-on: https://chromium-review.googlesource.com/794307
Commit-Queue: Luis Hector Chavez <lhchavez@chromium.org>
Reviewed-by: Yusuke Sato <yusukes@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520180}

chrome/browser/chromeos/arc/intent_helper/arc_external_protocol_dialog.cc

@@ -417,8 +417,8 @@ void OnUrlHandlerList(int render_process_host_id,
     activities.emplace_back(handler->package_name, handler->activity_name);
   }
   intent_helper_bridge->GetActivityIcons(
-      activities, base::Bind(OnAppIconsReceived, render_process_host_id,
-                             routing_id, url, base::Passed(&handlers)));
+      activities, base::BindOnce(OnAppIconsReceived, render_process_host_id,
+                                 routing_id, url, std::move(handlers)));
 }
 
 // Returns true if the |url| is safe to be forwarded to ARC without showing the

chrome/browser/chromeos/arc/intent_helper/arc_navigation_throttle.cc

@@ -281,9 +281,10 @@ bool ArcNavigationThrottle::FoundPreferredOrVerifiedArcApp(
 
     intent_helper_bridge->GetActivityIcons(
         activities,
-        base::Bind(&ArcNavigationThrottle::AsyncOnAppIconsReceived,
-                   chrome::FindBrowserWithWebContents(handle->GetWebContents()),
-                   base::Passed(&handlers), url));
+        base::BindOnce(
+            &ArcNavigationThrottle::AsyncOnAppIconsReceived,
+            chrome::FindBrowserWithWebContents(handle->GetWebContents()),
+            std::move(handlers), url));
   }
 
   return cancel_navigation;
@@ -479,8 +480,9 @@ void ArcNavigationThrottle::AsyncOnAppCandidatesReceived(
     activities.emplace_back(handler->package_name, handler->activity_name);
 
   intent_helper_bridge->GetActivityIcons(
-      activities, base::Bind(&ArcNavigationThrottle::AsyncOnAppIconsReceived,
-                             browser, base::Passed(&handlers), url));
+      activities,
+      base::BindOnce(&ArcNavigationThrottle::AsyncOnAppIconsReceived, browser,
+                     std::move(handlers), url));
 }
 
 }  // namespace arc

chrome/browser/chromeos/file_manager/arc_file_tasks.cc

@@ -116,8 +116,8 @@ void OnArcHandlerList(
     activity_names.emplace_back(handler->package_name, handler->activity_name);
 
   intent_helper_bridge->GetActivityIcons(
-      activity_names, base::Bind(&OnArcIconLoaded, base::Passed(&result_list),
-                                 callback, base::Passed(&handlers_filtered)));
+      activity_names, base::BindOnce(&OnArcIconLoaded, std::move(result_list),
+                                     callback, std::move(handlers_filtered)));
 }
 
 // Called after icon data for ARC apps are loaded. Proceeds to OnArcIconEncoded.

chrome/browser/task_manager/providers/arc/arc_process_task.cc

@@ -96,8 +96,8 @@ void ArcProcessTask::StartIconLoading() {
     std::vector<arc::ArcIntentHelperBridge::ActivityName> activities = {
         {package_name_, kEmptyActivityName}};
     result = intent_helper_bridge->GetActivityIcons(
-        activities, base::Bind(&ArcProcessTask::OnIconLoaded,
-                               weak_ptr_factory_.GetWeakPtr()));
+        activities, base::BindOnce(&ArcProcessTask::OnIconLoaded,
+                                   weak_ptr_factory_.GetWeakPtr()));
   }
 
   if (result == arc::ArcIntentHelperBridge::GetResult::FAILED_ARC_NOT_READY) {

chrome/browser/ui/webui/settings/chromeos/device_storage_handler.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/ui/webui/settings/chromeos/device_storage_handler.h"
 
 #include <algorithm>
+#include <limits>
 #include <numeric>
 #include <string>
 
@@ -347,7 +348,7 @@ void StorageHandler::UpdateAndroidSize() {
   auto* arc_storage_manager =
       arc::ArcStorageManager::GetForBrowserContext(profile);
   if (arc_storage_manager) {
-    success = arc_storage_manager->GetApplicationsSize(base::Bind(
+    success = arc_storage_manager->GetApplicationsSize(base::BindOnce(
         &StorageHandler::OnGetAndroidSize, weak_ptr_factory_.GetWeakPtr()));
   }
   if (!success)

components/arc/PRESUBMIT.py

@@ -5,6 +5,39 @@
 _CPP_WHITE_LIST = (r'.+\.cc$', r'.+\.h$')
 
 
+def CheckCallback(input_api, output_api):
+  """Discourages the use of deprecated base::{Bind,Callback,Closure}."""
+  # This contains a list of regex and a human-readable error message.
+  patterns = [
+      (r'\bbase::Bind\(',
+       'uses base::Bind. Please consider using base::Bind{Once,Repeating} '
+       'instead.'),
+      (r'\bbase::Callback<',
+       'uses base::Callback. Please consider using '
+       'base::{Once,Repeating}Callback instead'),
+      (r'\bbase::Closure\b',
+       'uses base::Closure. Please consider using '
+       'base::{Once,Repeating}Closure instead'),
+  ]
+  problems = []
+  def SourceFilter(affected_file):
+    return input_api.FilterSourceFile(affected_file, _CPP_WHITE_LIST,
+                                      input_api.DEFAULT_BLACK_LIST)
+  for f in input_api.AffectedSourceFiles(SourceFilter):
+    for line_number, line in f.ChangedContents():
+      for pattern in patterns:
+        if input_api.re.search(pattern[0], line):
+          problems.append(
+              '%s:%d %s' % (f.LocalPath(), line_number, pattern[1]))
+  if problems:
+    return [output_api.PresubmitPromptWarning(
+        'base::Bind, base::Closure, and base::Callback are in the process ' +
+        'of being deprecated. See crbug.com/714018 for more information ' +
+        'about the migration\n',
+        items=problems)]
+  return []
+
+
 def CheckMakeUnique(input_api, output_api):
   """Encourage to use std::make_unique, instead of base::MakeUnique."""
   errors = []
@@ -35,6 +68,7 @@ def CheckChangeOnUpload(input_api, output_api):
   results = []
   results += input_api.canned_checks.CheckChangeLintsClean(
       input_api, output_api)
+  results += CheckCallback(input_api, output_api)
   results += CheckUniquePtr(input_api, output_api)
   results += CheckMakeUnique(input_api, output_api)
   return results

components/arc/arc_bridge_host_impl.cc

@@ -40,8 +40,8 @@ class MojoChannelImpl : public ArcBridgeHostImpl::MojoChannel {
 
   ~MojoChannelImpl() override { holder_->SetInstance(nullptr, 0); }
 
-  void set_connection_error_handler(const base::Closure& error_handler) {
-    ptr_.set_connection_error_handler(error_handler);
+  void set_connection_error_handler(base::OnceClosure error_handler) {
+    ptr_.set_connection_error_handler(std::move(error_handler));
   }
 
   void QueryVersion() {
@@ -75,7 +75,7 @@ ArcBridgeHostImpl::ArcBridgeHostImpl(ArcBridgeService* arc_bridge_service,
   DCHECK(arc_bridge_service_);
   DCHECK(instance_.is_bound());
   instance_.set_connection_error_handler(
-      base::Bind(&ArcBridgeHostImpl::OnClosed, base::Unretained(this)));
+      base::BindOnce(&ArcBridgeHostImpl::OnClosed, base::Unretained(this)));
   mojom::ArcBridgeHostPtr host_proxy;
   binding_.Bind(mojo::MakeRequest(&host_proxy));
   instance_->Init(std::move(host_proxy));
@@ -305,7 +305,7 @@ void ArcBridgeHostImpl::OnInstanceReady(
   // Since |channel| is managed by |mojo_channels_|, its lifetime is shorter
   // than |this|. Thus, the connection error handler will be invoked only
   // when |this| is alive and base::Unretained is safe here.
-  channel->set_connection_error_handler(base::Bind(
+  channel->set_connection_error_handler(base::BindOnce(
       &ArcBridgeHostImpl::OnChannelClosed, base::Unretained(this), channel));
 
   // Call QueryVersion so that the version info is properly stored in the

components/arc/arc_session_runner.h

@@ -66,7 +66,8 @@ class ArcSessionRunner : public ArcSession::Observer,
 
   // This is the factory interface to inject ArcSession instance
   // for testing purpose.
-  using ArcSessionFactory = base::Callback<std::unique_ptr<ArcSession>()>;
+  using ArcSessionFactory =
+      base::RepeatingCallback<std::unique_ptr<ArcSession>()>;
 
   explicit ArcSessionRunner(const ArcSessionFactory& factory);
   ~ArcSessionRunner() override;

components/arc/intent_helper/activity_icon_loader.cc

@@ -181,7 +181,7 @@ void ActivityIconLoader::InvalidateIcons(const std::string& package_name) {
 
 ActivityIconLoader::GetResult ActivityIconLoader::GetActivityIcons(
     const std::vector<ActivityName>& activities,
-    const OnIconsReadyCallback& cb) {
+    OnIconsReadyCallback cb) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   std::unique_ptr<ActivityToIconsMap> result(new ActivityToIconsMap);
   std::vector<mojom::ActivityNamePtr> activities_to_fetch;
@@ -200,7 +200,7 @@ ActivityIconLoader::GetResult ActivityIconLoader::GetActivityIcons(
 
   if (activities_to_fetch.empty()) {
     // If there's nothing to fetch, run the callback now.
-    cb.Run(std::move(result));
+    std::move(cb).Run(std::move(result));
     return GetResult::SUCCEEDED_SYNC;
   }
 
@@ -209,22 +209,24 @@ ActivityIconLoader::GetResult ActivityIconLoader::GetActivityIcons(
   if (!instance) {
     // The mojo channel is not yet ready (or not supported at all). Run the
     // callback with |result| that could be empty.
-    cb.Run(std::move(result));
+    std::move(cb).Run(std::move(result));
     return error_code;
   }
 
   // Fetch icons from ARC.
   instance->RequestActivityIcons(
       std::move(activities_to_fetch), mojom::ScaleFactor(scale_factor_),
-      base::Bind(&ActivityIconLoader::OnIconsReady,
-                 weak_ptr_factory_.GetWeakPtr(), base::Passed(&result), cb));
+      base::BindOnce(&ActivityIconLoader::OnIconsReady,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(result),
+                     std::move(cb)));
   return GetResult::SUCCEEDED_ASYNC;
 }
 
 void ActivityIconLoader::OnIconsResizedForTesting(
-    const OnIconsReadyCallback& cb,
+    OnIconsReadyCallback cb,
     std::unique_ptr<ActivityToIconsMap> result) {
-  OnIconsResized(std::make_unique<ActivityToIconsMap>(), cb, std::move(result));
+  OnIconsResized(std::make_unique<ActivityToIconsMap>(), std::move(cb),
+                 std::move(result));
 }
 
 void ActivityIconLoader::AddCacheEntryForTesting(const ActivityName& activity) {
@@ -247,20 +249,20 @@ bool ActivityIconLoader::HasIconsReadyCallbackRun(GetResult result) {
 
 void ActivityIconLoader::OnIconsReady(
     std::unique_ptr<ActivityToIconsMap> cached_result,
-    const OnIconsReadyCallback& cb,
+    OnIconsReadyCallback cb,
     std::vector<mojom::ActivityIconPtr> icons) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   base::PostTaskAndReplyWithResult(
       FROM_HERE,
-      base::Bind(&ResizeAndEncodeIcons, base::Passed(&icons), scale_factor_),
-      base::Bind(&ActivityIconLoader::OnIconsResized,
-                 weak_ptr_factory_.GetWeakPtr(), base::Passed(&cached_result),
-                 cb));
+      base::BindOnce(&ResizeAndEncodeIcons, std::move(icons), scale_factor_),
+      base::BindOnce(&ActivityIconLoader::OnIconsResized,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(cached_result),
+                     std::move(cb)));
 }
 
 void ActivityIconLoader::OnIconsResized(
     std::unique_ptr<ActivityToIconsMap> cached_result,
-    const OnIconsReadyCallback& cb,
+    OnIconsReadyCallback cb,
     std::unique_ptr<ActivityToIconsMap> result) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   // Update |cached_icons_|.
@@ -271,7 +273,7 @@ void ActivityIconLoader::OnIconsResized(
 
   // Merge the results that were obtained from cache before doing IPC.
   result->insert(cached_result->begin(), cached_result->end());
-  cb.Run(std::move(result));
+  std::move(cb).Run(std::move(result));
 }
 
 }  // namespace internal

components/arc/intent_helper/activity_icon_loader.h

@@ -66,7 +66,7 @@ class ActivityIconLoader {
 
   using ActivityToIconsMap = std::map<ActivityName, Icons>;
   using OnIconsReadyCallback =
-      base::Callback<void(std::unique_ptr<ActivityToIconsMap>)>;
+      base::OnceCallback<void(std::unique_ptr<ActivityToIconsMap>)>;
 
   ActivityIconLoader();
   ~ActivityIconLoader();
@@ -80,9 +80,9 @@ class ActivityIconLoader {
   // locally or ARC is not ready/supported). Otherwise, the callback is run
   // later asynchronously with icons fetched from ARC side.
   GetResult GetActivityIcons(const std::vector<ActivityName>& activities,
-                             const OnIconsReadyCallback& cb);
+                             OnIconsReadyCallback cb);
 
-  void OnIconsResizedForTesting(const OnIconsReadyCallback& cb,
+  void OnIconsResizedForTesting(OnIconsReadyCallback cb,
                                 std::unique_ptr<ActivityToIconsMap> result);
   void AddCacheEntryForTesting(const ActivityName& activity);
 
@@ -95,13 +95,13 @@ class ActivityIconLoader {
  private:
   // A function called when the mojo IPC returns.
   void OnIconsReady(std::unique_ptr<ActivityToIconsMap> cached_result,
-                    const OnIconsReadyCallback& cb,
+                    OnIconsReadyCallback cb,
                     std::vector<mojom::ActivityIconPtr> icons);
 
   // A function called when ResizeIcons finishes. Append items in |result| to
   // |cached_icons_|.
   void OnIconsResized(std::unique_ptr<ActivityToIconsMap> cached_result,
-                      const OnIconsReadyCallback& cb,
+                      OnIconsReadyCallback cb,
                       std::unique_ptr<ActivityToIconsMap> result);
 
   // The maximum scale factor the current platform supports.

components/arc/intent_helper/activity_icon_loader_unittest.cc

@@ -118,7 +118,7 @@ TEST(ActivityIconLoaderTest, TestOnIconsResized) {
   ActivityIconLoader loader;
 
   // Call OnIconsResized() and check that the cache is properly updated.
-  loader.OnIconsResizedForTesting(base::Bind(&OnIconsReady0),
+  loader.OnIconsResizedForTesting(base::BindOnce(&OnIconsReady0),
                                   std::move(activity_to_icons));
   EXPECT_EQ(3U, loader.cached_icons_for_testing().size());
   EXPECT_EQ(1U, loader.cached_icons_for_testing().count(
@@ -139,7 +139,7 @@ TEST(ActivityIconLoaderTest, TestOnIconsResized) {
   activity_to_icons->insert(std::make_pair(
       ActivityIconLoader::ActivityName("p2", "a2"),
       ActivityIconLoader::Icons(gfx::Image(), gfx::Image(), nullptr)));
-  loader.OnIconsResizedForTesting(base::Bind(&OnIconsReady3),
+  loader.OnIconsResizedForTesting(base::BindOnce(&OnIconsReady3),
                                   std::move(activity_to_icons));
   EXPECT_EQ(4U, loader.cached_icons_for_testing().size());
   EXPECT_EQ(1U, loader.cached_icons_for_testing().count(

components/arc/intent_helper/arc_intent_helper_bridge.cc

@@ -227,9 +227,9 @@ void ArcIntentHelperBridge::OpenVolumeControl() {
 
 ArcIntentHelperBridge::GetResult ArcIntentHelperBridge::GetActivityIcons(
     const std::vector<ActivityName>& activities,
-    const OnIconsReadyCallback& callback) {
+    OnIconsReadyCallback callback) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  return icon_loader_.GetActivityIcons(activities, callback);
+  return icon_loader_.GetActivityIcons(activities, std::move(callback));
 }
 
 bool ArcIntentHelperBridge::ShouldChromeHandleUrl(const GURL& url) {

components/arc/intent_helper/arc_intent_helper_bridge.h

@@ -83,7 +83,7 @@ class ArcIntentHelperBridge
       internal::ActivityIconLoader::OnIconsReadyCallback;
   using GetResult = internal::ActivityIconLoader::GetResult;
   GetResult GetActivityIcons(const std::vector<ActivityName>& activities,
-                             const OnIconsReadyCallback& callback);
+                             OnIconsReadyCallback callback);
 
   // Returns true when |url| can only be handled by Chrome. Otherwise, which is
   // when there might be one or more ARC apps that can handle |url|, returns

components/arc/intent_helper/link_handler_model.cc

@@ -118,8 +118,8 @@ void LinkHandlerModel::OnUrlHandlerList(
     }
     const ArcIntentHelperBridge::GetResult result =
         intent_helper_bridge->GetActivityIcons(
-            activities, base::Bind(&LinkHandlerModel::NotifyObserver,
-                                   weak_ptr_factory_.GetWeakPtr()));
+            activities, base::BindOnce(&LinkHandlerModel::NotifyObserver,
+                                       weak_ptr_factory_.GetWeakPtr()));
     icon_info_notified =
         internal::ActivityIconLoader::HasIconsReadyCallbackRun(result);
   }

components/arc/storage_manager/arc_storage_manager.cc

@@ -4,6 +4,7 @@
 
 #include "components/arc/storage_manager/arc_storage_manager.h"
 
+#include <utility>
 #include <vector>
 
 #include "base/bind.h"
@@ -58,12 +59,12 @@ bool ArcStorageManager::OpenPrivateVolumeSettings() {
 }
 
 bool ArcStorageManager::GetApplicationsSize(
-    const GetApplicationsSizeCallback& callback) {
+    GetApplicationsSizeCallback callback) {
   auto* storage_manager_instance = ARC_GET_INSTANCE_FOR_METHOD(
       arc_bridge_service_->storage_manager(), GetApplicationsSize);
   if (!storage_manager_instance)
     return false;
-  storage_manager_instance->GetApplicationsSize(callback);
+  storage_manager_instance->GetApplicationsSize(std::move(callback));
   return true;
 }
 

components/arc/storage_manager/arc_storage_manager.h

@@ -38,8 +38,8 @@ class ArcStorageManager : public KeyedService {
 
   // Gets storage usage of all application's APK, data, and cache size.
   using GetApplicationsSizeCallback =
-      base::Callback<void(bool succeeded, mojom::ApplicationsSizePtr)>;
-  bool GetApplicationsSize(const GetApplicationsSizeCallback& callback);
+      base::OnceCallback<void(bool succeeded, mojom::ApplicationsSizePtr)>;
+  bool GetApplicationsSize(GetApplicationsSizeCallback callback);
 
   // Deletes all applications' cache files.
   bool DeleteApplicationsCache(const base::Callback<void()>& callback);