[clean ios] Fix TabCollectionMediator disconnect.

ios/clean/chrome/browser/ui/tab_collection/tab_collection_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/tab_collection/tab_collection_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"
@@ skipping 31 matching lines @@
 }
 
 - (void)dealloc {
   [self disconnect];
 }
 
 #pragma mark - Public
 
 - (void)disconnect {
   _webStateList = nullptr;
-  _webStateObserver.reset();
+  _scopedWebStateListObserver->RemoveAll();

[sdefresne, 2017/06/12 09:48:07]

I don't understand the comment that ScopedObserver takes care of clearing
_webStateObserver.

_webStateObserver is std::unique_ptr<web::WebStateObserverBridge> while
_scopedWebStateListObserver is std::unique_ptr<ScopedObserver<WebStateList,
WebStateListObserverBridge>>.

WebStateListObserver != WebStateObserver so they are not related. So you should
remove the comment from the description.

Regarding whether the .reset() call is required or not, I think it should be
present because it unregister the _webStateObserver from WebState at a
known-time instead of doing it when the autoreleasepool is clearer. I think we
should restore the .reset().

[marq (ping after 24h), 2017/06/13 08:18:53]

Yeah, brain fart on my part.

Does it make sense to be using a scoped observer for the WSL observer, but not
for the WebState observer? Shouldn't both be done the same way?

[sdefresne, 2017/06/13 09:00:27]

WebStateObserver is a weird observer that auto-unregister itself in its
destructor (this is one of the rare observer that do it this way).

See web_state_observer.mm:

  WebStateObserver::~WebStateObserver() {
    if (web_state_)
      web_state_->RemoveObserver(this);
  }

It is also one of the only observer that can only observe a single WebState (so
if you want to observe multiple WebStates at the same time you need to use
multiple WebStateObserver, where as with other observers you can observe
multiple instance with the same observer).

 }
 
 #pragma mark - Properties
 
 - (void)setWebStateList:(WebStateList*)webStateList {
   DCHECK(webStateList);
   _scopedWebStateListObserver->RemoveAll();
   _webStateList = webStateList;
   _scopedWebStateListObserver->Add(_webStateList);
   _webStateObserver = base::MakeUnique<web::WebStateObserverBridge>(
@@ skipping 104 matching lines @@
 - (void)populateConsumerItems {
   if (self.consumer && self.webStateList) {
     [self.consumer
         populateItems:[self
                           tabCollectionItemsFromWebStateList:self.webStateList]
         selectedIndex:self.webStateList->active_index()];
   }
 }
 
 @end