[ios] ARCMigrate ios/chrome/browser/tabs/tab.mm to ARC.

ios/chrome/browser/tabs/tab.mm

Index: ios/chrome/browser/tabs/tab.mm
diff --git a/ios/chrome/browser/tabs/tab.mm b/ios/chrome/browser/tabs/tab.mm
index 6bb0c4ab49b34abb3b75b8b970a06213c483dd4f..03526bae46f7162a53e296a902610a04d6d553f2 100644
--- a/ios/chrome/browser/tabs/tab.mm
+++ b/ios/chrome/browser/tabs/tab.mm
@@ -12,13 +12,11 @@
 
 #include "base/bind.h"
 #include "base/ios/block_types.h"
-#import "base/ios/weak_nsobject.h"
 #include "base/json/string_escape.h"
 #include "base/logging.h"
 #include "base/mac/bind_objc_block.h"
 #include "base/mac/foundation_util.h"
-#include "base/mac/objc_property_releaser.h"
-#include "base/mac/scoped_nsobject.h"
+#include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/user_metrics.h"
 #include "base/metrics/user_metrics_action.h"
@@ -144,9 +142,9 @@
 #include "ui/base/page_transition_types.h"
 #include "url/origin.h"
 
-using base::UserMetricsAction;
-using web::NavigationManagerImpl;
-using net::RequestTracker;
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
 
 NSString* const kTabUrlStartedLoadingNotificationForCrashReporting =
     @"kTabUrlStartedLoadingNotificationForCrashReporting";
@@ -204,17 +202,27 @@ bool IsItemRedirectItem(web::NavigationItem* item) {
   return (ui::PageTransition::PAGE_TRANSITION_IS_REDIRECT_MASK &
           item->GetTransitionType()) == 0;
 }
+
+// TabHistoryContext is used by history to scope the lifetime of navigation
+// entry references to Tab.
+class TabHistoryContext : public history::Context {
+ public:
+  TabHistoryContext() {}
+  ~TabHistoryContext() {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TabHistoryContext);
+};
 }  // namespace
 
 @interface Tab ()<CRWWebStateObserver,
                   CRWWebControllerObserver,
                   FindInPageControllerDelegate,
                   ReaderModeControllerDelegate> {
-  TabModel* parentTabModel_;               // weak
-  ios::ChromeBrowserState* browserState_;  // weak
+  __weak TabModel* parentTabModel_;
+  ios::ChromeBrowserState* browserState_;
 
-  base::scoped_nsobject<OpenInController> openInController_;
-  base::WeakNSProtocol<id<PassKitDialogProvider>> passKitDialogProvider_;
+  OpenInController* openInController_;
 
   // Whether or not this tab is currently being displayed.
   BOOL visible_;
@@ -237,69 +245,41 @@ bool IsItemRedirectItem(web::NavigationItem* item) {
   // Last visited timestamp.
   double lastVisitedTimestamp_;
 
-  base::mac::ObjCPropertyReleaser propertyReleaser_Tab_;
-
-  id<TabDelegate> delegate_;  // weak
-  base::WeakNSProtocol<id<TabDialogDelegate>> dialogDelegate_;
-  base::WeakNSProtocol<id<SnapshotOverlayProvider>> snapshotOverlayProvider_;
-
-  // Delegate used for snapshotting geometry.
-  id<TabSnapshottingDelegate> tabSnapshottingDelegate_;  // weak
-
   // The Full Screen Controller responsible for hiding/showing the toolbar.
-  base::scoped_nsobject<FullScreenController> fullScreenController_;
-
-  // The delegate responsible for headers over the tab.
-  id<TabHeadersDelegate> tabHeadersDelegate_;  // weak
-
-  base::WeakNSProtocol<id<FullScreenControllerDelegate>>
-      fullScreenControllerDelegate_;
+  FullScreenController* fullScreenController_;
 
   // The Overscroll controller responsible for displaying the
   // overscrollActionsView above the toolbar.
-  base::scoped_nsobject<OverscrollActionsController>
-      overscrollActionsController_;
-  base::WeakNSProtocol<id<OverscrollActionsControllerDelegate>>
-      overscrollActionsControllerDelegate_;
-
-  base::scoped_nsobject<NSString> tabId_;
+  OverscrollActionsController* overscrollActionsController_;
 
   // Lightweight object dealing with various different UI behaviours when
   // opening a URL in an external application.
-  base::scoped_nsobject<ExternalAppLauncher> externalAppLauncher_;
+  ExternalAppLauncher* externalAppLauncher_;
 
   // Handles suggestions for form entry.
-  base::scoped_nsobject<FormSuggestionController> suggestionController_;
+  FormSuggestionController* suggestionController_;
 
   // Manages the input accessory view during form input.
-  base::scoped_nsobject<FormInputAccessoryViewController>
-      inputAccessoryViewController_;
-
-  // TODO(crbug.com/661665): move the WebContentsObservers into their own
-  // container.
-  // Handles saving and autofill of passwords.
-  base::scoped_nsobject<PasswordController> passwordController_;
+  FormInputAccessoryViewController* inputAccessoryViewController_;
 
   // Handles autofill.
-  base::scoped_nsobject<AutofillController> autofillController_;
+  AutofillController* autofillController_;
 
   // Handles GAL infobar on web pages.
-  base::scoped_nsobject<NativeAppNavigationController>
-      nativeAppNavigationController_;
+  NativeAppNavigationController* nativeAppNavigationController_;
 
   // Handles caching and retrieving of snapshots.
-  base::scoped_nsobject<SnapshotManager> snapshotManager_;
+  SnapshotManager* snapshotManager_;
 
   // Handles retrieving, generating and updating snapshots of CRWWebController's
   // web page.
-  base::scoped_nsobject<WebControllerSnapshotHelper>
-      webControllerSnapshotHelper_;
+  WebControllerSnapshotHelper* webControllerSnapshotHelper_;
 
   // Handles support for window.print JavaScript calls.
   std::unique_ptr<PrintObserver> printObserver_;
 
   // AutoReloadBridge for this tab.
-  base::scoped_nsobject<AutoReloadBridge> autoReloadBridge_;
+  AutoReloadBridge* autoReloadBridge_;
 
   // WebStateImpl for this tab.
   web::WebStateImpl* webStateImpl_;
@@ -309,16 +289,13 @@ bool IsItemRedirectItem(web::NavigationItem* item) {
 
   // Context used by history to scope the lifetime of navigation entry
   // references to Tab.
-  std::unique_ptr<TabHistoryContext> tabHistoryContext_;
-
-  // The controller for everything related to reader mode.
-  base::scoped_nsobject<ReaderModeController> readerModeController_;
+  TabHistoryContext tabHistoryContext_;
 
   // C++ bridge that receives notifications from the FaviconDriver.
   std::unique_ptr<FaviconDriverObserverBridge> faviconDriverObserverBridge_;
 
   // U2F call controller object.
-  base::scoped_nsobject<U2FController> U2FController_;
+  U2FController* U2FController_;
 
   // C++ observer used to trigger snapshots after the removal of InfoBars.
   std::unique_ptr<TabInfoBarObserver> tabInfoBarObserver_;
@@ -367,17 +344,6 @@ bool IsItemRedirectItem(web::NavigationItem* item) {
 @end
 
 namespace {
-// TabHistoryContext is used by history to scope the lifetime of navigation
-// entry references to Tab.
-class TabHistoryContext : public history::Context {
- public:
-  TabHistoryContext() {}
-  ~TabHistoryContext() {}
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(TabHistoryContext);
-};
-
 class FaviconDriverObserverBridge : public favicon::FaviconDriverObserver {
  public:
   FaviconDriverObserverBridge(Tab* owner,
@@ -392,7 +358,7 @@ class FaviconDriverObserverBridge : public favicon::FaviconDriverObserver {
                         const gfx::Image& image) override;
 
  private:
-  Tab* owner_;  // Owns this instance.
+  __weak Tab* owner_;
   ScopedObserver<favicon::FaviconDriver, favicon::FaviconDriverObserver>
       scoped_observer_;
   DISALLOW_COPY_AND_ASSIGN(FaviconDriverObserverBridge);
@@ -428,7 +394,7 @@ class TabInfoBarObserver : public infobars::InfoBarManager::Observer {
                          infobars::InfoBar* new_infobar) override;
 
  private:
-  Tab* owner_;  // Owns this instance;
+  __weak Tab* owner_;
   ScopedObserver<infobars::InfoBarManager, TabInfoBarObserver> scoped_observer_;
   DISALLOW_COPY_AND_ASSIGN(TabInfoBarObserver);
 };
@@ -473,27 +439,33 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
 @implementation Tab
 
 @synthesize browserState = browserState_;
+@synthesize tabId = tabId_;
 @synthesize useGreyImageCache = useGreyImageCache_;
 @synthesize isPrerenderTab = isPrerenderTab_;
 @synthesize isLinkLoadingPrerenderTab = isLinkLoadingPrerenderTab_;
 @synthesize isVoiceSearchResultsTab = isVoiceSearchResultsTab_;
+@synthesize passwordController = passwordController_;
+@synthesize overscrollActionsController = overscrollActionsController_;
+@synthesize readerModeController = readerModeController_;
+@synthesize overscrollActionsControllerDelegate =
+    overscrollActionsControllerDelegate_;
+@synthesize passKitDialogProvider = passKitDialogProvider_;
 @synthesize delegate = delegate_;
+@synthesize dialogDelegate = dialogDelegate_;
+@synthesize snapshotOverlayProvider = snapshotOverlayProvider_;
 @synthesize tabSnapshottingDelegate = tabSnapshottingDelegate_;
 @synthesize tabHeadersDelegate = tabHeadersDelegate_;
+@synthesize fullScreenControllerDelegate = fullScreenControllerDelegate_;
 
 - (instancetype)initWithWebState:(web::WebState*)webState {
   DCHECK(webState);
   self = [super init];
   if (self) {
-    propertyReleaser_Tab_.Init(self, [Tab class]);
-
     // TODO(crbug.com/620465): Tab should only use public API of WebState.
     // Remove this cast once this is the case.
     webStateImpl_ = static_cast<web::WebStateImpl*>(webState);
     browserState_ =
         ios::ChromeBrowserState::FromBrowserState(webState->GetBrowserState());
-
-    tabHistoryContext_ = base::MakeUnique<TabHistoryContext>();
     webStateObserver_ =
         base::MakeUnique<web::WebStateObserverBridge>(webState, self);
 
@@ -501,10 +473,10 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
     [[self webController] addObserver:self];
     [[self webController] setDelegate:self];
 
-    snapshotManager_.reset([[SnapshotManager alloc] init]);
-    webControllerSnapshotHelper_.reset([[WebControllerSnapshotHelper alloc]
+    snapshotManager_ = [[SnapshotManager alloc] init];
+    webControllerSnapshotHelper_ = [[WebControllerSnapshotHelper alloc]
         initWithSnapshotManager:snapshotManager_
-                            tab:self]);
+                            tab:self];
 
     [self initNativeAppNavigationController];
 
@@ -518,40 +490,39 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
 }
 
 - (void)attachTabHelpers {
-  tabInfoBarObserver_.reset(new TabInfoBarObserver(self));
+  tabInfoBarObserver_ = base::MakeUnique<TabInfoBarObserver>(self);
   tabInfoBarObserver_->SetShouldObserveInfoBarManager(true);
 
-  if (experimental_flags::IsAutoReloadEnabled()) {
-    autoReloadBridge_.reset([[AutoReloadBridge alloc] initWithTab:self]);
-  }
+  if (experimental_flags::IsAutoReloadEnabled())
+    autoReloadBridge_ = [[AutoReloadBridge alloc] initWithTab:self];
   printObserver_ = base::MakeUnique<PrintObserver>(self.webState);
 
   base::scoped_nsprotocol<id<PasswordsUiDelegate>> passwordsUiDelegate(

[rohitrao (ping after 24h), 2017/05/02 11:42:54]

Intentionally still using scoped_nsprotocol?

[sdefresne, 2017/05/02 15:08:51]

No, I missed that one. Fixed. Thank you for the catch.

       [[PasswordsUiDelegateImpl alloc] init]);
-  passwordController_.reset([[PasswordController alloc]
-         initWithWebState:self.webState
-      passwordsUiDelegate:passwordsUiDelegate]);
+  passwordController_ =
+      [[PasswordController alloc] initWithWebState:self.webState
+                               passwordsUiDelegate:passwordsUiDelegate];
   password_manager::PasswordGenerationManager* passwordGenerationManager =
       [passwordController_ passwordGenerationManager];
-  autofillController_.reset([[AutofillController alloc]
-           initWithBrowserState:browserState_
-      passwordGenerationManager:passwordGenerationManager
-                       webState:self.webState]);
-  suggestionController_.reset([[FormSuggestionController alloc]
+  autofillController_ =
+      [[AutofillController alloc] initWithBrowserState:browserState_
+                             passwordGenerationManager:passwordGenerationManager
+                                              webState:self.webState];
+  suggestionController_ = [[FormSuggestionController alloc]
       initWithWebState:self.webState
-             providers:[self suggestionProviders]]);
-  inputAccessoryViewController_.reset([[FormInputAccessoryViewController alloc]
+             providers:[self suggestionProviders]];
+  inputAccessoryViewController_ = [[FormInputAccessoryViewController alloc]
       initWithWebState:self.webState
-             providers:[self accessoryViewProviders]]);
+             providers:[self accessoryViewProviders]];
 
   [self setShouldObserveFaviconChanges:YES];
 
   // Create the ReaderModeController immediately so it can register for
   // WebState changes.
   if (experimental_flags::IsReaderModeEnabled()) {
-    readerModeController_.reset([[ReaderModeController alloc]
-        initWithWebState:self.webState
-                delegate:self]);
+    readerModeController_ =
+        [[ReaderModeController alloc] initWithWebState:self.webState
+                                              delegate:self];
   }
 }
 
@@ -580,7 +551,7 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
   DCHECK(!model || !parentTabModel_);
   parentTabModel_ = model;
 
-  if (parentTabModel_.syncedWindowDelegate) {
+  if ([parentTabModel_ syncedWindowDelegate]) {

[rohitrao (ping after 24h), 2017/05/02 11:42:54]

This no longer works as a property?

[sdefresne, 2017/05/02 15:08:51]

I had an intermediate steps where I changed parentTabModel_ to
base::WeakNSObject<> which required changing this to method call syntax. This
still works as a property, reverted.

     IOSChromeSessionTabHelper::FromWebState(self.webState)
         ->SetWindowID(model.sessionID);
   }
@@ -603,10 +574,6 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
   return [self.webController loadPhase] == web::PAGE_LOADED;
 }
 
-- (void)setDialogDelegate:(id<TabDialogDelegate>)dialogDelegate {
-  dialogDelegate_.reset(dialogDelegate);
-}
-
 - (void)setIsVoiceSearchResultsTab:(BOOL)isVoiceSearchResultsTab {
   // There is intentionally no equality check in this setter, as we want the
   // notificaiton to be sent regardless of whether the value has changed.
@@ -614,10 +581,6 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
   [parentTabModel_ notifyTabChanged:self];
 }
 
-- (PasswordController*)passwordController {
-  return passwordController_.get();
-}
-
 - (void)retrieveSnapshot:(void (^)(UIImage*))callback {
   [webControllerSnapshotHelper_
       retrieveSnapshotForWebController:self.webController
@@ -670,7 +633,7 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
 
 - (NSString*)tabId {
   if (tabId_)
-    return tabId_.get();
+    return tabId_;
 
   DCHECK(self.webState);
   web::SerializableUserDataManager* userDataManager =
@@ -683,8 +646,8 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
     userDataManager->AddSerializableData(tabId, kTabIDKey);
   }
 
-  tabId_.reset([tabId copy]);
-  return tabId_.get();
+  tabId_ = [tabId copy];
+  return tabId_;
 }
 
 - (web::WebState*)webState {
@@ -698,9 +661,8 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
 
   favicon::FaviconDriver* faviconDriver =
       favicon::WebFaviconDriver::FromWebState(self.webState);
-  if (faviconDriver) {
+  if (faviconDriver)
     faviconDriver->FetchFavicon(url);
-  }
 }
 
 - (void)setFavicon:(const gfx::Image*)image {
@@ -727,9 +689,8 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
 
 - (UIView*)view {
   // Record reload of previously-evicted tab.
-  if (![self.webController isViewAlive] && [parentTabModel_ tabUsageRecorder]) {
+  if (![self.webController isViewAlive] && [parentTabModel_ tabUsageRecorder])
     [parentTabModel_ tabUsageRecorder]->RecordPageLoadStart(self);
-  }
   return self.webState ? self.webState->GetView() : nil;
 }
 
@@ -769,10 +730,10 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
   if (fullScreenController_) {
     [fullScreenController_ invalidate];
     [self.webController removeObserver:fullScreenController_];
-    fullScreenController_.reset([[FullScreenController alloc]
+    fullScreenController_ = [[FullScreenController alloc]
          initWithDelegate:fullScreenControllerDelegate_
         navigationManager:self.navigationManager
-                sessionID:self.tabId]);
+                sessionID:self.tabId];
     [self.webController addObserver:fullScreenController_];
     // If the content of the page was loaded without knowledge of the
     // toolbar position it will be misplaced under the toolbar instead of
@@ -816,23 +777,18 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
   [self countMainFrameLoad];
 }
 
-- (id<FullScreenControllerDelegate>)fullScreenControllerDelegate {
-  return fullScreenControllerDelegate_.get();
-}
-
 - (void)setFullScreenControllerDelegate:
     (id<FullScreenControllerDelegate>)fullScreenControllerDelegate {
-  if (fullScreenControllerDelegate == fullScreenControllerDelegate_) {
+  if (fullScreenControllerDelegate == fullScreenControllerDelegate_)
     return;
-  }
   // Lazily create a FullScreenController.
   // The check for fullScreenControllerDelegate is necessary to avoid recreating
   // a FullScreenController during teardown.
   if (!fullScreenController_ && fullScreenControllerDelegate) {
-    fullScreenController_.reset([[FullScreenController alloc]
+    fullScreenController_ = [[FullScreenController alloc]
          initWithDelegate:fullScreenControllerDelegate
         navigationManager:self.navigationManager
-                sessionID:self.tabId]);
+                sessionID:self.tabId];
     [self.webController addObserver:fullScreenController_];
     // If the content of the page was loaded without knowledge of the
     // toolbar position it will be misplaced under the toolbar instead of
@@ -840,41 +796,31 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
     // sure the content is moved to the right place.
     [fullScreenController_ moveContentBelowHeader];
   }
-  fullScreenControllerDelegate_.reset(fullScreenControllerDelegate);
-}
-
-- (OverscrollActionsController*)overscrollActionsController {
-  return overscrollActionsController_.get();
-}
-
-- (id<OverscrollActionsControllerDelegate>)overscrollActionsControllerDelegate {
-  return overscrollActionsControllerDelegate_.get();
+  fullScreenControllerDelegate_ = fullScreenControllerDelegate;
 }
 
 - (void)setOverscrollActionsControllerDelegate:
     (id<OverscrollActionsControllerDelegate>)
         overscrollActionsControllerDelegate {
   if (overscrollActionsControllerDelegate_ ==
-      overscrollActionsControllerDelegate)
+      overscrollActionsControllerDelegate) {
     return;
+  }
 
   // Lazily create a OverscrollActionsController.
   // The check for overscrollActionsControllerDelegate is necessary to avoid
   // recreating a OverscrollActionsController during teardown.
   if (!overscrollActionsController_) {
-    overscrollActionsController_.reset(
-        [[OverscrollActionsController alloc] init]);
+    overscrollActionsController_ = [[OverscrollActionsController alloc] init];
     [self.webController addObserver:overscrollActionsController_];
   }
   OverscrollStyle style = OverscrollStyle::REGULAR_PAGE_NON_INCOGNITO;
-  if (browserState_->IsOffTheRecord()) {
+  if (browserState_->IsOffTheRecord())
     style = OverscrollStyle::REGULAR_PAGE_INCOGNITO;
-  }
   [overscrollActionsController_ setStyle:style];
   [overscrollActionsController_
       setDelegate:overscrollActionsControllerDelegate];
-  overscrollActionsControllerDelegate_.reset(
-      overscrollActionsControllerDelegate);
+  overscrollActionsControllerDelegate_ = overscrollActionsControllerDelegate;
 }
 
 - (void)saveTitleToHistoryDB {
@@ -884,8 +830,9 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
   // Don't update the history if current entry has no title.
   NSString* title = [self title];
   if (![title length] ||
-      [title isEqualToString:l10n_util::GetNSString(IDS_DEFAULT_TAB_TITLE)])
+      [title isEqualToString:l10n_util::GetNSString(IDS_DEFAULT_TAB_TITLE)]) {
     return;
+  }
 
   history::HistoryService* historyService =
       ios::HistoryServiceFactory::GetForBrowserState(
@@ -951,12 +898,12 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
         referrer.url != GURL(kChromeContentSuggestionsReferrer);
 
     history::HistoryAddPageArgs args(
-        url, item->GetTimestamp(), tabHistoryContext_.get(),
-        item->GetUniqueID(), referrer.url, redirects, item->GetTransitionType(),
+        url, item->GetTimestamp(), &tabHistoryContext_, item->GetUniqueID(),
+        referrer.url, redirects, item->GetTransitionType(),
         history::SOURCE_BROWSED, false, consider_for_ntp_most_visited);
     addPageVector_.push_back(args);
   } else {
-    historyService->AddPage(url, item->GetTimestamp(), tabHistoryContext_.get(),
+    historyService->AddPage(url, item->GetTimestamp(), &tabHistoryContext_,
                             item->GetUniqueID(), referrer.url, redirects,
                             item->GetTransitionType(), history::SOURCE_BROWSED,
                             false);
@@ -1004,7 +951,7 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
   if (!initialNavigation && !isPrerenderTab_ &&
       !PageTransitionCoreTypeIs(transition, ui::PAGE_TRANSITION_RELOAD) &&
       (transition & ui::PAGE_TRANSITION_IS_REDIRECT_MASK) == 0) {
-    base::RecordAction(UserMetricsAction("MobileTabClobbered"));
+    base::RecordAction(base::UserMetricsAction("MobileTabClobbered"));
   }
   if ([parentTabModel_ tabUsageRecorder])
     [parentTabModel_ tabUsageRecorder]->RecordPageLoadStart(self);
@@ -1033,9 +980,8 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
 }
 
 - (void)webWillReload {
-  if ([parentTabModel_ tabUsageRecorder]) {
+  if ([parentTabModel_ tabUsageRecorder])
     [parentTabModel_ tabUsageRecorder]->RecordReload(self);
-  }
 }
 
 // Halt the tab, which amounts to halting its webController.
@@ -1053,24 +999,24 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
   [[NSNotificationCenter defaultCenter] removeObserver:self];
 
   [passwordController_ detach];
-  passwordController_.reset();
+  passwordController_ = nil;
   tabInfoBarObserver_.reset();
 
   faviconDriverObserverBridge_.reset();
   [openInController_ detachFromWebController];
-  openInController_.reset();
+  openInController_ = nil;
   [autofillController_ detachFromWebState];
   [suggestionController_ detachFromWebState];
   if (fullScreenController_)
     [self.webController removeObserver:fullScreenController_];
   [fullScreenController_ invalidate];
-  fullScreenController_.reset();
+  fullScreenController_ = nil;
   if (overscrollActionsController_)
     [self.webController removeObserver:overscrollActionsController_];
   [overscrollActionsController_ invalidate];
-  overscrollActionsController_.reset();
+  overscrollActionsController_ = nil;
   [readerModeController_ detachFromWebState];
-  readerModeController_.reset();
+  readerModeController_ = nil;
 
   // Invalidate any snapshot stored for this session.
   DCHECK(self.tabId);
@@ -1099,8 +1045,8 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
         favicon::WebFaviconDriver::FromWebState(self.webState);
     // Some MockWebContents used in tests do not support the FaviconDriver.
     if (faviconDriver) {
-      faviconDriverObserverBridge_.reset(
-          new FaviconDriverObserverBridge(self, faviconDriver));
+      faviconDriverObserverBridge_ =
+          base::MakeUnique<FaviconDriverObserverBridge>(self, faviconDriver);
     }
   } else {
     faviconDriverObserverBridge_.reset();
@@ -1110,7 +1056,7 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
 - (void)goBack {
   if (self.navigationManager) {
     DCHECK(self.navigationManager->CanGoBack());
-    base::RecordAction(UserMetricsAction("Back"));
+    base::RecordAction(base::UserMetricsAction("Back"));
     self.navigationManager->GoBack();
   }
 }
@@ -1118,7 +1064,7 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
 - (void)goForward {
   if (self.navigationManager) {
     DCHECK(self.navigationManager->CanGoForward());
-    base::RecordAction(UserMetricsAction("Forward"));
+    base::RecordAction(base::UserMetricsAction("Forward"));
     self.navigationManager->GoForward();
   }
 }
@@ -1146,28 +1092,26 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
 - (BOOL)openExternalURL:(const GURL&)url
               sourceURL:(const GURL&)sourceURL
             linkClicked:(BOOL)linkClicked {
-  if (!externalAppLauncher_.get())
-    externalAppLauncher_.reset([[ExternalAppLauncher alloc] init]);
+  if (!externalAppLauncher_)
+    externalAppLauncher_ = [[ExternalAppLauncher alloc] init];
 
   // This method may release CRWWebController which may cause a crash
   // (crbug.com/393949).
-  [[self.webController retain] autorelease];
+  // [[self.webController retain] autorelease];

[rohitrao (ping after 24h), 2017/05/02 11:42:54]

What is the plan for replacing this call?  Should we hold a strong reference to
the webController instead?

[sdefresne, 2017/05/02 15:08:51]

This call can be safely removed as the lifetime of the objects is now sane. I
just forgot to remove the code after commenting it. I've tested the reproduction
steps for the linked bug and the app does not crash.

 
   // Make a local url copy for possible modification.
   GURL finalURL = url;
 
   // Check if it's a direct FIDO U2F x-callback call. If so, do not open it, to
   // prevent pages from spoofing requests with different origins.
-  if (finalURL.SchemeIs("u2f-x-callback")) {
+  if (finalURL.SchemeIs("u2f-x-callback"))
     return NO;
-  }
 
   // Check if it's a FIDO U2F call.
   if (finalURL.SchemeIs("u2f")) {
     // Create U2FController object lazily.
-    if (!U2FController_) {
-      U2FController_.reset([[U2FController alloc] init]);
-    }
+    if (!U2FController_)
+      U2FController_ = [[U2FController alloc] init];
 
     DCHECK([self navigationManager]);
     GURL origin =
@@ -1179,9 +1123,8 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
                                                 tabURL:self.url
                                                  tabID:self.tabId];
 
-    if (!finalURL.is_valid()) {
+    if (!finalURL.is_valid())
       return NO;
-    }
   }
 
   if ([externalAppLauncher_ openURL:finalURL linkClicked:linkClicked]) {
@@ -1196,9 +1139,8 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
     if (sourceURL.is_valid()) {
       ReadingListModel* model =
           ReadingListModelFactory::GetForBrowserState(browserState_);
-      if (model && model->loaded()) {
+      if (model && model->loaded())
         model->SetReadStatus(sourceURL, true);
-      }
     }
 
     return YES;
@@ -1209,11 +1151,10 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
 - (void)webState:(web::WebState*)webState
     didFinishNavigation:(web::NavigationContext*)navigation {
   if (navigation->IsSameDocument()) {
+    // Fetch the favicon for the new URL.
     auto* faviconDriver = favicon::WebFaviconDriver::FromWebState(webState);
-    if (faviconDriver) {
-      // Fetch the favicon for the new URL.
+    if (faviconDriver)
       faviconDriver->FetchFavicon(navigation->GetUrl());
-    }
   }
 
   if (!navigation->IsErrorPage()) {
@@ -1244,18 +1185,18 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
 
 - (OpenInController*)openInController {
   if (!openInController_) {
-    openInController_.reset([[OpenInController alloc]
+    openInController_ = [[OpenInController alloc]
         initWithRequestContext:browserState_->GetRequestContext()
-                 webController:self.webController]);
+                 webController:self.webController];
   }
-  return openInController_.get();
+  return openInController_;
 }
 
 - (id<CRWNativeContent>)controllerForUnhandledContentAtURL:(const GURL&)url {
   // Shows download manager UI for unhandled content.
   DownloadManagerController* downloadController =
-      [[[DownloadManagerController alloc] initWithWebState:self.webState
-                                               downloadURL:url] autorelease];
+      [[DownloadManagerController alloc] initWithWebState:self.webState
+                                              downloadURL:url];
   [downloadController start];
   return downloadController;
 }
@@ -1288,22 +1229,21 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
 }
 
 - (void)countMainFrameLoad {
-  if ([self isPrerenderTab] || [self url].SchemeIs(kChromeUIScheme)) {
+  if ([self isPrerenderTab] || [self url].SchemeIs(kChromeUIScheme))
     return;
-  }
-  base::RecordAction(UserMetricsAction("MobilePageLoaded"));
+  base::RecordAction(base::UserMetricsAction("MobilePageLoaded"));
 }
 
 - (void)applicationDidBecomeActive {
-  if (requireReloadAfterBecomingActive_) {
-    if (visible_) {
-      self.navigationManager->Reload(web::ReloadType::NORMAL,
-                                     false /* check_for_repost */);
-    } else {
-      [self.webController requirePageReload];
-    }
-    requireReloadAfterBecomingActive_ = NO;
+  if (!requireReloadAfterBecomingActive_)
+    return;
+  if (visible_) {
+    self.navigationManager->Reload(web::ReloadType::NORMAL,
+                                   false /* check_for_repost */);
+  } else {
+    [self.webController requirePageReload];
   }
+  requireReloadAfterBecomingActive_ = NO;
 }
 
 #pragma mark -
@@ -1329,10 +1269,6 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
   return self.view;
 }
 
-- (ReaderModeController*)readerModeController {
-  return readerModeController_.get();
-}
-
 - (BOOL)canSwitchToReaderMode {
   // Only if the page is loaded and the page passes suitability checks.
   ReaderModeController* controller = self.readerModeController;
@@ -1410,15 +1346,6 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
   navigationManager->LoadURLWithParams(params);
 }
 
-- (id<SnapshotOverlayProvider>)snapshotOverlayProvider {
-  return snapshotOverlayProvider_.get();
-}
-
-- (void)setSnapshotOverlayProvider:
-    (id<SnapshotOverlayProvider>)snapshotOverlayProvider {
-  snapshotOverlayProvider_.reset(snapshotOverlayProvider);
-}
-
 - (void)evaluateU2FResultFromURL:(const GURL&)URL {
   DCHECK(U2FController_);
   [U2FController_ evaluateU2FResultFromU2FURL:URL webState:self.webState];
@@ -1459,7 +1386,7 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
   // checked in several places.
   if (isUserNavigationEvent && !isPrerenderTab_ &&
       ![self navigationManager]->GetPendingItem() && url != self.url) {
-    base::RecordAction(UserMetricsAction("MobileTabClobbered"));
+    base::RecordAction(base::UserMetricsAction("MobileTabClobbered"));
     if ([parentTabModel_ tabUsageRecorder])
       [parentTabModel_ tabUsageRecorder]->RecordPageLoadStart(self);
   }
@@ -1498,9 +1425,8 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
   }
   favicon::FaviconDriver* faviconDriver =
       favicon::WebFaviconDriver::FromWebState(webState);
-  if (faviconDriver) {
+  if (faviconDriver)
     faviconDriver->FetchFavicon(lastCommittedURL);
-  }
   [parentTabModel_ notifyTabChanged:self];
   if (parentTabModel_) {
     [[NSNotificationCenter defaultCenter]
@@ -1553,9 +1479,8 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
   else
     [autoReloadBridge_ loadFailedForURL:lastCommittedURL wasPost:wasPost];
   [webControllerSnapshotHelper_ setSnapshotCoalescingEnabled:YES];
-  if (!loadSuccess) {
+  if (!loadSuccess)
     [fullScreenController_ disableFullScreen];
-  }
   [self recordInterfaceOrientation];
   navigation_metrics::RecordMainFrameNavigation(
       lastCommittedURL, true, self.browserState->IsOffTheRecord());
@@ -1584,9 +1509,8 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
       finishPageLoadForTab:self
                loadSuccess:loadSuccess];
 
-  if (loadSuccess) {
+  if (loadSuccess)
     [self updateSnapshotWithOverlay:YES visibleFrameOnly:YES];
-  }
   [webControllerSnapshotHelper_ setSnapshotCoalescingEnabled:NO];
 }
 
@@ -1631,8 +1555,8 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
     return NO;
 
   base::scoped_nsprotocol<id<NativeAppMetadata>> metadata(

[rohitrao (ping after 24h), 2017/05/02 11:42:54]

Are we intentionally still using scoped_nsprotocol here?  If so, is that worth
an explanatory comment?

[sdefresne, 2017/05/02 15:08:51]

No, it was a mistake (but the code has been removed in another CL and
disappeared during the rebase, so nothing to do here).

-      [[ios::GetChromeBrowserProvider()->GetNativeAppWhitelistManager()
-          nativeAppForURL:url] retain]);
+      [ios::GetChromeBrowserProvider()->GetNativeAppWhitelistManager()
+          nativeAppForURL:url]);
   if (![metadata shouldAutoOpenLinks])
     return NO;
 
@@ -1686,9 +1610,8 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
 
   // Always allow frame loads.
   BOOL isFrameLoad = (url != mainDocumentURL);
-  if (isFrameLoad) {
+  if (isFrameLoad)
     return YES;
-  }
 
   // TODO(crbug.com/546402): If this turns out to be useful, find a less hacky
   // hook point to send this from.
@@ -1817,9 +1740,8 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
   }
 
   if (visible_) {
-    if (!applicationIsNotActive) {
+    if (!applicationIsNotActive)
       [fullScreenController_ disableFullScreen];
-    }
   } else {
     [self.webController requirePageReload];
   }
@@ -1861,8 +1783,8 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
   signin_metrics::LogAccountReconcilorStateOnGaiaResponse(
       ios::AccountReconcilorFactory::GetForBrowserState(browserState_)
           ->GetState());
-  base::scoped_nsobject<GenericChromeCommand> command(
-      [[GenericChromeCommand alloc] initWithTag:IDC_SHOW_ACCOUNTS_SETTINGS]);
+  GenericChromeCommand* command =
+      [[GenericChromeCommand alloc] initWithTag:IDC_SHOW_ACCOUNTS_SETTINGS];
   [self.view chromeExecuteCommand:command];
 }
 
@@ -1877,8 +1799,8 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
   signin_metrics::LogAccountReconcilorStateOnGaiaResponse(
       ios::AccountReconcilorFactory::GetForBrowserState(browserState_)
           ->GetState());
-  base::scoped_nsobject<GenericChromeCommand> command(
-      [[GenericChromeCommand alloc] initWithTag:IDC_SHOW_ADD_ACCOUNT]);
+  GenericChromeCommand* command =
+      [[GenericChromeCommand alloc] initWithTag:IDC_SHOW_ADD_ACCOUNT];
   [self.view chromeExecuteCommand:command];
 }
 
@@ -1899,17 +1821,17 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
   [self goBack];
 
   if (url.is_valid()) {
-    base::scoped_nsobject<OpenUrlCommand> command([[OpenUrlCommand alloc]
+    OpenUrlCommand* command = [[OpenUrlCommand alloc]
          initWithURL:url
             referrer:web::Referrer()  // Strip referrer when switching modes.
          inIncognito:YES
         inBackground:NO
-            appendTo:kLastTab]);
+            appendTo:kLastTab];
     [self.view chromeExecuteCommand:command];
   } else {
-    base::scoped_nsobject<GenericChromeCommand> chromeCommand(
-        [[GenericChromeCommand alloc] initWithTag:IDC_NEW_INCOGNITO_TAB]);
-    [self.view chromeExecuteCommand:chromeCommand];
+    GenericChromeCommand* command =
+        [[GenericChromeCommand alloc] initWithTag:IDC_NEW_INCOGNITO_TAB];
+    [self.view chromeExecuteCommand:command];
   }
 }
 
@@ -1921,19 +1843,11 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
   if (browserState_->IsOffTheRecord())
     return;
   DCHECK(!nativeAppNavigationController_);
-  nativeAppNavigationController_.reset(
-      [[NativeAppNavigationController alloc] initWithWebState:self.webState]);
+  nativeAppNavigationController_ =
+      [[NativeAppNavigationController alloc] initWithWebState:self.webState];
   DCHECK(nativeAppNavigationController_);
 }
 
-- (id<PassKitDialogProvider>)passKitDialogProvider {
-  return passKitDialogProvider_.get();
-}
-
-- (void)setPassKitDialogProvider:(id<PassKitDialogProvider>)provider {
-  passKitDialogProvider_.reset(provider);
-}
-
 - (void)wasShown {
   visible_ = YES;
   [self updateFullscreenWithToolbarVisible:YES];
@@ -1962,7 +1876,7 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
 @implementation Tab (TestingSupport)
 
 - (void)replaceExternalAppLauncher:(id)externalAppLauncher {
-  externalAppLauncher_.reset([externalAppLauncher retain]);
+  externalAppLauncher_ = externalAppLauncher;
 }
 
 - (TabModel*)parentTabModel {
@@ -1970,7 +1884,7 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
 }
 
 - (FormInputAccessoryViewController*)inputAccessoryViewController {
-  return inputAccessoryViewController_.get();
+  return inputAccessoryViewController_;
 }
 
 @end