[ios] Enable -Wimplicit-retain-self warning

Objective-C block can capture implicitly any variables, including access to ivar. This leads to some block incorrectly retaining `self` through access to ivar while the block is also using weak pointer to self (e.g. `__weak ++typeof(self) weakSelf = self;`). The warning -Wimplicit-retain-self flags all access to ivar that causes the compiler to retain `self` forcing users to either use an explicit access (e.g. `self->_foo` instead of `_foo`) or to go through the weak pointer (e.g. `weakSelf->_foo`). As there are many targets that do retain self implicitly, disable the warning for those targets (they are not in the majority, thus it will prevent use of the incorrect pattern to spread to other files). They will be fixed in followup CLs. Bug: 1198164 Change-Id: I18ded9fa8372acf41d015f18a13ebc0a12464663 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2823835 Commit-Queue: Sylvain Defresne <sdefresne@chromium.org> Reviewed-by: Dirk Pranke <dpranke@google.com> Reviewed-by: Yuwei Huang <yuweih@chromium.org> Reviewed-by: Stepan Khapugin <stkhapugin@chromium.org> Cr-Commit-Position: refs/heads/master@{#872421}
Denger-Network · Apr 14, 2021 · 44d5cfa · 44d5cfa
1 parent 41cdb05
commit 44d5cfa
Show file tree

Hide file tree

Showing 28 changed files with 114 additions and 25 deletions.
diff --git a/build/config/compiler/BUILD.gn b/build/config/compiler/BUILD.gn
@@ -1677,6 +1677,11 @@ config("chromium_code") {
       cflags_objc = [ "-Wobjc-missing-property-synthesis" ]
       cflags_objcc = [ "-Wobjc-missing-property-synthesis" ]
     }
+
+    if (is_ios) {
+      cflags_objc = [ "-Wimplicit-retain-self" ]
+      cflags_objcc = cflags_objc
+    }
   }
 
   if (is_clang) {

diff --git a/build/config/ios/BUILD.gn b/build/config/ios/BUILD.gn
@@ -196,6 +196,11 @@ config("ios_shared_library_flags") {
   ]
 }
 
+config("disable_implicit_retain_self_warning") {
+  cflags_objc = [ "-Wno-implicit-retain-self" ]
+  cflags_objcc = cflags_objc
+}
+
 config("xctest_config") {
   framework_dirs = [ "$ios_sdk_platform_path/Developer/Library/Frameworks" ]
 

diff --git a/components/autofill/ios/browser/BUILD.gn b/components/autofill/ios/browser/BUILD.gn
@@ -5,7 +5,10 @@
 import("//ios/web/js_compile.gni")
 
 source_set("browser") {
-  configs += [ "//build/config/compiler:enable_arc" ]
+  configs += [
+    "//build/config/compiler:enable_arc",
+    "//build/config/ios:disable_implicit_retain_self_warning",
+  ]
   sources = [
     "autofill_agent.h",
     "autofill_agent.mm",
@@ -64,7 +67,10 @@ js_compile_bundle("autofill_js") {
 
 source_set("test_support") {
   testonly = true
-  configs += [ "//build/config/compiler:enable_arc" ]
+  configs += [
+    "//build/config/compiler:enable_arc",
+    "//build/config/ios:disable_implicit_retain_self_warning",
+  ]
   sources = [
     "credit_card_save_manager_test_observer_bridge.h",
     "credit_card_save_manager_test_observer_bridge.mm",

diff --git a/components/password_manager/ios/BUILD.gn b/components/password_manager/ios/BUILD.gn
@@ -5,7 +5,10 @@
 import("//ios/web/js_compile.gni")
 
 component("ios") {
-  configs += [ "//build/config/compiler:enable_arc" ]
+  configs += [
+    "//build/config/compiler:enable_arc",
+    "//build/config/ios:disable_implicit_retain_self_warning",
+  ]
 
   deps = [
     "//base",

diff --git a/ios/chrome/app/BUILD.gn b/ios/chrome/app/BUILD.gn
@@ -184,7 +184,10 @@ source_set("safe_mode_app_state_agent") {
 }
 
 source_set("app_internal") {
-  configs += [ "//build/config/compiler:enable_arc" ]
+  configs += [
+    "//build/config/compiler:enable_arc",
+    "//build/config/ios:disable_implicit_retain_self_warning",
+  ]
   sources = [
     "chrome_overlay_window.h",
     "chrome_overlay_window.mm",

diff --git a/ios/chrome/app/application_delegate/BUILD.gn b/ios/chrome/app/application_delegate/BUILD.gn
@@ -150,7 +150,10 @@ source_set("app_state_header") {
 }
 
 source_set("application_delegate_internal") {
-  configs += [ "//build/config/compiler:enable_arc" ]
+  configs += [
+    "//build/config/compiler:enable_arc",
+    "//build/config/ios:disable_implicit_retain_self_warning",
+  ]
   public_deps = [ ":app_state_header" ]
   sources = [
     "app_state.mm",

diff --git a/ios/chrome/app/spotlight/BUILD.gn b/ios/chrome/app/spotlight/BUILD.gn
@@ -3,7 +3,10 @@
 # found in the LICENSE file.
 
 source_set("spotlight") {
-  configs += [ "//build/config/compiler:enable_arc" ]
+  configs += [
+    "//build/config/compiler:enable_arc",
+    "//build/config/ios:disable_implicit_retain_self_warning",
+  ]
   sources = [
     "actions_spotlight_manager.h",
     "actions_spotlight_manager.mm",

diff --git a/ios/chrome/browser/crash_report/BUILD.gn b/ios/chrome/browser/crash_report/BUILD.gn
@@ -26,7 +26,10 @@ source_set("crash_report") {
     "synthetic_crash_report_util.mm",
   ]
 
-  configs += [ "//build/config/compiler:enable_arc" ]
+  configs += [
+    "//build/config/compiler:enable_arc",
+    "//build/config/ios:disable_implicit_retain_self_warning",
+  ]
 
   deps = [
     "//base",

diff --git a/ios/chrome/browser/passwords/BUILD.gn b/ios/chrome/browser/passwords/BUILD.gn
@@ -186,7 +186,10 @@ source_set("passwords_generation_utils") {
 }
 
 source_set("unit_tests") {
-  configs += [ "//build/config/compiler:enable_arc" ]
+  configs += [
+    "//build/config/compiler:enable_arc",
+    "//build/config/ios:disable_implicit_retain_self_warning",
+  ]
   testonly = true
   sources = [
     "ios_chrome_password_check_manager_unittest.mm",

diff --git a/ios/chrome/browser/prerender/BUILD.gn b/ios/chrome/browser/prerender/BUILD.gn
@@ -98,6 +98,7 @@ source_set("eg2_tests") {
   configs += [
     "//build/config/compiler:enable_arc",
     "//build/config/ios:xctest_config",
+    "//build/config/ios:disable_implicit_retain_self_warning",
   ]
   testonly = true
   sources = [ "prerender_egtest.mm" ]

diff --git a/ios/chrome/browser/ui/authentication/BUILD.gn b/ios/chrome/browser/ui/authentication/BUILD.gn
@@ -5,7 +5,10 @@
 import("//build/config/chrome_build.gni")
 
 source_set("authentication") {
-  configs += [ "//build/config/compiler:enable_arc" ]
+  configs += [
+    "//build/config/compiler:enable_arc",
+    "//build/config/ios:disable_implicit_retain_self_warning",
+  ]
   sources = [
     "authentication_flow.h",
     "authentication_flow.mm",

diff --git a/ios/chrome/browser/ui/browser_view/BUILD.gn b/ios/chrome/browser/ui/browser_view/BUILD.gn
@@ -3,7 +3,10 @@
 # found in the LICENSE file.
 
 source_set("browser_view") {
-  configs += [ "//build/config/compiler:enable_arc" ]
+  configs += [
+    "//build/config/compiler:enable_arc",
+    "//build/config/ios:disable_implicit_retain_self_warning",
+  ]
   sources = [
     "browser_coordinator.h",
     "browser_coordinator.mm",

diff --git a/ios/chrome/browser/ui/history/BUILD.gn b/ios/chrome/browser/ui/history/BUILD.gn
@@ -3,7 +3,10 @@
 # found in the LICENSE file.
 
 source_set("history") {
-  configs += [ "//build/config/compiler:enable_arc" ]
+  configs += [
+    "//build/config/compiler:enable_arc",
+    "//build/config/ios:disable_implicit_retain_self_warning",
+  ]
   sources = [
     "history_coordinator.h",
     "history_coordinator.mm",
@@ -52,7 +55,10 @@ source_set("constants") {
 }
 
 source_set("history_ui") {
-  configs += [ "//build/config/compiler:enable_arc" ]
+  configs += [
+    "//build/config/compiler:enable_arc",
+    "//build/config/ios:disable_implicit_retain_self_warning",
+  ]
   sources = [
     "history_consumer.h",
     "history_entries_status_item.h",

diff --git a/ios/chrome/browser/ui/image_util/BUILD.gn b/ios/chrome/browser/ui/image_util/BUILD.gn
@@ -32,5 +32,8 @@ source_set("web") {
     "//ui/base",
   ]
   frameworks = [ "Photos.framework" ]
-  configs += [ "//build/config/compiler:enable_arc" ]
+  configs += [
+    "//build/config/compiler:enable_arc",
+    "//build/config/ios:disable_implicit_retain_self_warning",
+  ]
 }
diff --git a/ios/chrome/browser/ui/open_in/BUILD.gn b/ios/chrome/browser/ui/open_in/BUILD.gn
@@ -3,7 +3,10 @@
 # found in the LICENSE file.
 
 source_set("open_in_ui") {
-  configs += [ "//build/config/compiler:enable_arc" ]
+  configs += [
+    "//build/config/compiler:enable_arc",
+    "//build/config/ios:disable_implicit_retain_self_warning",
+  ]
   sources = [
     "open_in_controller.h",
     "open_in_controller.mm",

diff --git a/ios/chrome/browser/ui/overscroll_actions/BUILD.gn b/ios/chrome/browser/ui/overscroll_actions/BUILD.gn
@@ -3,7 +3,10 @@
 # found in the LICENSE file.
 
 source_set("overscroll_actions") {
-  configs += [ "//build/config/compiler:enable_arc" ]
+  configs += [
+    "//build/config/compiler:enable_arc",
+    "//build/config/ios:disable_implicit_retain_self_warning",
+  ]
   sources = [
     "overscroll_actions_controller.h",
     "overscroll_actions_controller.mm",

diff --git a/ios/chrome/browser/ui/recent_tabs/BUILD.gn b/ios/chrome/browser/ui/recent_tabs/BUILD.gn
@@ -55,7 +55,10 @@ source_set("recent_tabs_ui_constants") {
 }
 
 source_set("recent_tabs_ui") {
-  configs += [ "//build/config/compiler:enable_arc" ]
+  configs += [
+    "//build/config/compiler:enable_arc",
+    "//build/config/ios:disable_implicit_retain_self_warning",
+  ]
   sources = [
     "recent_tabs_consumer.h",
     "recent_tabs_menu_provider.h",

diff --git a/ios/chrome/browser/ui/scanner/BUILD.gn b/ios/chrome/browser/ui/scanner/BUILD.gn
@@ -3,7 +3,10 @@
 # found in the LICENSE file.
 
 source_set("scanner") {
-  configs += [ "//build/config/compiler:enable_arc" ]
+  configs += [
+    "//build/config/compiler:enable_arc",
+    "//build/config/ios:disable_implicit_retain_self_warning",
+  ]
   sources = [
     "camera_controller.h",
     "camera_controller.mm",

diff --git a/ios/chrome/browser/ui/settings/BUILD.gn b/ios/chrome/browser/ui/settings/BUILD.gn
@@ -398,6 +398,7 @@ source_set("eg2_tests") {
   configs += [
     "//build/config/compiler:enable_arc",
     "//build/config/ios:xctest_config",
+    "//build/config/ios:disable_implicit_retain_self_warning",
   ]
   testonly = true
   sources = [

diff --git a/ios/chrome/browser/ui/side_swipe/BUILD.gn b/ios/chrome/browser/ui/side_swipe/BUILD.gn
@@ -3,7 +3,10 @@
 # found in the LICENSE file.
 
 source_set("side_swipe") {
-  configs += [ "//build/config/compiler:enable_arc" ]
+  configs += [
+    "//build/config/compiler:enable_arc",
+    "//build/config/ios:disable_implicit_retain_self_warning",
+  ]
   sources = [
     "card_side_swipe_view.h",
     "card_side_swipe_view.mm",

diff --git a/ios/chrome/browser/ui/tab_switcher/tab_grid/BUILD.gn b/ios/chrome/browser/ui/tab_switcher/tab_grid/BUILD.gn
@@ -104,7 +104,10 @@ source_set("tab_grid_ui") {
     "thumb_strip_plus_sign_button.mm",
   ]
 
-  configs += [ "//build/config/compiler:enable_arc" ]
+  configs += [
+    "//build/config/compiler:enable_arc",
+    "//build/config/ios:disable_implicit_retain_self_warning",
+  ]
 
   deps = [
     ":features",

diff --git a/ios/chrome/browser/ui/tabs/BUILD.gn b/ios/chrome/browser/ui/tabs/BUILD.gn
@@ -3,7 +3,10 @@
 # found in the LICENSE file.
 
 source_set("tabs") {
-  configs += [ "//build/config/compiler:enable_arc" ]
+  configs += [
+    "//build/config/compiler:enable_arc",
+    "//build/config/ios:disable_implicit_retain_self_warning",
+  ]
   sources = [
     "background_tab_animation_view.h",
     "background_tab_animation_view.mm",

diff --git a/ios/chrome/browser/web/BUILD.gn b/ios/chrome/browser/web/BUILD.gn
@@ -5,7 +5,10 @@
 import("//ios/web/js_compile.gni")
 
 source_set("web") {
-  configs += [ "//build/config/compiler:enable_arc" ]
+  configs += [
+    "//build/config/compiler:enable_arc",
+    "//build/config/ios:disable_implicit_retain_self_warning",
+  ]
   sources = [
     "dom_altering_lock.h",
     "dom_altering_lock.mm",
@@ -399,6 +402,7 @@ source_set("eg2_tests") {
   configs += [
     "//build/config/compiler:enable_arc",
     "//build/config/ios:xctest_config",
+    "//build/config/ios:disable_implicit_retain_self_warning",
   ]
   testonly = true
 

diff --git a/ios/chrome/share_extension/BUILD.gn b/ios/chrome/share_extension/BUILD.gn
@@ -52,7 +52,10 @@ ios_appex_bundle("share_extension") {
     "ui_util.mm",
   ]
 
-  configs += [ "//build/config/compiler:enable_arc" ]
+  configs += [
+    "//build/config/compiler:enable_arc",
+    "//build/config/ios:disable_implicit_retain_self_warning",
+  ]
 
   deps = [
     ":system_strings",

diff --git a/ios/web/security/BUILD.gn b/ios/web/security/BUILD.gn
@@ -5,7 +5,10 @@
 import("//ios/build/config.gni")
 
 source_set("security") {
-  configs += [ "//build/config/compiler:enable_arc" ]
+  configs += [
+    "//build/config/compiler:enable_arc",
+    "//build/config/ios:disable_implicit_retain_self_warning",
+  ]
 
   deps = [
     "//base",

diff --git a/ios/web/web_state/ui/BUILD.gn b/ios/web/web_state/ui/BUILD.gn
@@ -72,7 +72,10 @@ source_set("ui") {
     "crw_wk_ui_handler_delegate.h",
   ]
 
-  configs += [ "//build/config/compiler:enable_arc" ]
+  configs += [
+    "//build/config/compiler:enable_arc",
+    "//build/config/ios:disable_implicit_retain_self_warning",
+  ]
 }
 
 source_set("web_controller_header") {

diff --git a/ios/web_view/BUILD.gn b/ios/web_view/BUILD.gn
@@ -328,6 +328,7 @@ source_set("web_view_sources") {
   configs += [
     ":config",
     "//build/config/compiler:enable_arc",
+    "//build/config/ios:disable_implicit_retain_self_warning",
   ]
 }
 

diff --git a/remoting/ios/app/BUILD.gn b/remoting/ios/app/BUILD.gn
@@ -117,7 +117,10 @@ source_set("common_source_set") {
     "//remoting/ios/facade",
   ]
 
-  configs += [ "//build/config/compiler:enable_arc" ]
+  configs += [
+    "//build/config/compiler:enable_arc",
+    "//build/config/ios:disable_implicit_retain_self_warning",
+  ]
 }
 
 source_set("app_source_set") {