[ios clean] Toolbar displays total number of tabs.

ios/clean/chrome/browser/ui/toolbar/toolbar_mediator.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/clean/chrome/browser/ui/toolbar/toolbar_mediator.h"
 
 #include "base/memory/ptr_util.h"
+#include "base/scoped_observer.h"
 #include "base/strings/sys_string_conversions.h"
+#import "ios/chrome/browser/web_state_list/web_state_list.h"
+#import "ios/chrome/browser/web_state_list/web_state_list_observer_bridge.h"
 #import "ios/clean/chrome/browser/ui/toolbar/toolbar_consumer.h"
 #import "ios/web/public/navigation_manager.h"
 #include "ios/web/public/web_state/web_state.h"
 #import "ios/web/public/web_state/web_state_observer_bridge.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
 #endif
 
-@interface ToolbarMediator ()<CRWWebStateObserver>
+@interface ToolbarMediator ()<CRWWebStateObserver, WebStateListObserving>
 @end
 
 @implementation ToolbarMediator {
   std::unique_ptr<web::WebStateObserverBridge> _webStateObserver;
+  std::unique_ptr<WebStateListObserverBridge> _webStateListObserver;
+  std::unique_ptr<ScopedObserver<WebStateList, WebStateListObserverBridge>>
+      _scopedWebStateListObserver;
 }
 
 @synthesize consumer = _consumer;
 @synthesize webState = _webState;
+@synthesize webStateList = _webStateList;
 
 - (void)dealloc {
+  [self disconnect];
+}
+
+#pragma mark - Public
+
+- (void)disconnect {
+  self.webStateList = nullptr;
   _webStateObserver.reset();
-  _webState = nullptr;
 }
 
 #pragma mark - CRWWebStateObserver
 
 - (void)webState:(web::WebState*)webState didLoadPageWithSuccess:(BOOL)success {
   [self.consumer
       setCanGoBack:self.webState->GetNavigationManager()->CanGoBack()];
   [self.consumer
       setCanGoForward:self.webState->GetNavigationManager()->CanGoForward()];
 }
 
 - (void)webStateDidStartLoading:(web::WebState*)webState {
   [self.consumer setIsLoading:self.webState->IsLoading()];
 }
 
 - (void)webStateDidStopLoading:(web::WebState*)webState {
   [self.consumer setIsLoading:self.webState->IsLoading()];
 }
 
 - (void)webState:(web::WebState*)webState
     didChangeLoadingProgress:(double)progress {
   [self.consumer setLoadingProgress:progress];
 }
 
+#pragma mark - WebStateListObserver
+
+- (void)webStateList:(WebStateList*)webStateList
+    didInsertWebState:(web::WebState*)webState
+              atIndex:(int)index {
+  [self.consumer setNumberOfTabs:_webStateList->count()];
+}
+
+- (void)webStateList:(WebStateList*)webStateList
+    didDetachWebState:(web::WebState*)webState
+              atIndex:(int)index {
+  [self.consumer setNumberOfTabs:_webStateList->count()];
+}
+
 #pragma mark - Setters
 
 - (void)setWebState:(web::WebState*)webState {
   _webState = webState;
   _webStateObserver =
       base::MakeUnique<web::WebStateObserverBridge>(_webState, self);
   if (self.consumer) {
     [self updateConsumer];
   }
 }
 
 - (void)setConsumer:(id<ToolbarConsumer>)consumer {
   _consumer = consumer;
   if (self.webState) {
     [self updateConsumer];
   }
+  if (self.webStateList) {
+    [self.consumer setNumberOfTabs:_webStateList->count()];
+  }
+}
+
+- (void)setWebStateList:(WebStateList*)webStateList {
+  _webStateList = webStateList;
+  _webStateListObserver = base::MakeUnique<WebStateListObserverBridge>(self);
+  _scopedWebStateListObserver = base::MakeUnique<
+      ScopedObserver<WebStateList, WebStateListObserverBridge>>(
+      _webStateListObserver.get());
+  if (_webStateList) {
+    _scopedWebStateListObserver->Add(_webStateList);
+    if (self.consumer) {
+      [self.consumer setNumberOfTabs:_webStateList->count()];

[edchin, 2017/05/27 16:25:06]

If the webStateList is set to nil, wouldn't you want to set the tab count to
zero, instead of leaving it at some existing number? Consider
DCHECK(webStateList). I don't think we have this pattern yet, but I feel it is
better than convoluted logic that checks for nullability. 

[marq (ping after 24h), 2017/05/29 11:04:36]

Yes, I think it's correct to declare that setting a nullptr WSL here is a
programming error, and enforce that with a DCHECK at the start of the method. 

(Consider: if the WSL is empty, what toolbar is this mediator talking to?)

[sczs, 2017/05/30 00:18:21]

The main problem is that we are explicitly calling self.webStateList = nullptr
on disconnect. Without this we get the following observer_list failed check
during testing:

FATAL:observer_list.h(338)] Check failed: ObserverListBase<ObserverType>::size()
== 0U (1 vs. 0)
(If I try to set the backing variable to nullptr I still get this same check
fail)

I think Ed and Rohit got to the bottom of this, and that's why TabGridMediator
also checks for a nil webStateList

The second issue is that TabCoordinator unit tests are currently not
initializing a WebStateList, so they would also trigger a DCHECK(webStateList).
We also need to update those. 

For these reasons I would recommend leaving this as is and we can revisit this
issue for both this mediator and TabGridMediator on a separate CL. I will add a
TODO for now.

If you don't agree with this approach please let me know, I'm open to any ideas
:) 

+    }
+  }
 }
 
 #pragma mark - Helper methods
 
 // Updates the consumer to match the current WebState.
 - (void)updateConsumer {
   DCHECK(self.webState);
   DCHECK(self.consumer);
   [self.consumer
       setCanGoForward:self.webState->GetNavigationManager()->CanGoForward()];
   [self.consumer
       setCanGoBack:self.webState->GetNavigationManager()->CanGoBack()];
   [self.consumer setIsLoading:self.webState->IsLoading()];
 }
 
 @end