[ios] TabIdTabHelper

ios/chrome/browser/tabs/tab_helper_util.mm

 // Copyright 2017 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.
 
 #import "ios/chrome/browser/tabs/tab_helper_util.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
 #endif
 
@@ skipping 17 matching lines @@
 #import "ios/chrome/browser/store_kit/store_kit_tab_helper.h"
 #import "ios/chrome/browser/sync/ios_chrome_synced_tab_delegate.h"
 #import "ios/chrome/browser/tabs/legacy_tab_helper.h"
 #import "ios/chrome/browser/tabs/tab.h"
 #import "ios/chrome/browser/tabs/tab_private.h"
 #import "ios/chrome/browser/translate/chrome_ios_translate_client.h"
 #import "ios/chrome/browser/web/blocked_popup_tab_helper.h"
 #import "ios/chrome/browser/web/network_activity_indicator_tab_helper.h"
 #import "ios/chrome/browser/web/repost_form_tab_helper.h"
 #import "ios/chrome/browser/web/sad_tab_tab_helper.h"
+#import "ios/chrome/browser/web/tab_id_tab_helper.h"
 #import "ios/public/provider/chrome/browser/chrome_browser_provider.h"
 #import "ios/web/public/web_state/web_state.h"
 
 void AttachTabHelpers(web::WebState* web_state) {
+  // TabIDTabHelper provides the TabID, which is necessary in other helpers.

[sdefresne, 2017/06/26 10:28:55]

I would say "tab ID" instead of "TabID" in this comment. And I would say that
this is necessary for the creation of the Tab instance (which happens in when
LegacyTabHelper is created). Maybe something like this:

  // TabIDHelper sets up the tab ID which is required for the creation of the
  // Tab by LegacyTabHelper.

[edchin, 2017/06/26 16:20:57]

Done.

+  TabIDTabHelper::CreateForWebState(web_state);
   LegacyTabHelper::CreateForWebState(web_state);
   Tab* tab = LegacyTabHelper::GetTabForWebState(web_state);
   DCHECK(tab);
 
   ios::ChromeBrowserState* browser_state =
       ios::ChromeBrowserState::FromBrowserState(web_state->GetBrowserState());
 
-  // IOSChromeSessionTabHelper comes first because it sets up the tab ID,
+  // IOSChromeSessionTabHelper comes first because it sets up the session ID,

[sdefresne, 2017/06/26 10:28:55]

This is now the third tab helper installed, so "comes first" part of the comment
is incorrect. Could you change this to:

  // IOSChromeSessionTabHelper sets up the session ID used by other helper and
  // and needs to be create before them.

[edchin, 2017/06/26 16:20:57]

Done.

   // and other helpers may rely on that.
   IOSChromeSessionTabHelper::CreateForWebState(web_state);
 
   NetworkActivityIndicatorTabHelper::CreateForWebState(web_state, tab.tabId);
   IOSChromeSyncedTabDelegate::CreateForWebState(web_state);
   InfoBarManagerImpl::CreateForWebState(web_state);
   IOSSecurityStateTabHelper::CreateForWebState(web_state);
   RepostFormTabHelper::CreateForWebState(web_state);
   BlockedPopupTabHelper::CreateForWebState(web_state);
   FindTabHelper::CreateForWebState(web_state, tab.findInPageControllerDelegate);
@@ skipping 24 matching lines @@
       web_state,
       ios::TopSitesFactory::GetForBrowserState(original_browser_state).get());
 
   // Allow the embedder to attach tab helpers.
   ios::GetChromeBrowserProvider()->AttachTabHelpers(web_state, tab);
 
   // Allow the Tab to attach tab helper like objects (all those objects should
   // really be tab helpers and created above).
   [tab attachTabHelpers];
 }