PasswordController should give up on WebState destruction

ios/chrome/browser/passwords/password_controller.mm

 // Copyright 2012 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/passwords/password_controller.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <memory>
@@ skipping 216 matching lines @@
 }  // namespace
 
 @implementation PasswordController {
   std::unique_ptr<PasswordManager> passwordManager_;
   std::unique_ptr<PasswordGenerationManager> passwordGenerationManager_;
   std::unique_ptr<PasswordManagerClient> passwordManagerClient_;
   std::unique_ptr<PasswordManagerDriver> passwordManagerDriver_;
   base::scoped_nsobject<PasswordGenerationAgent> passwordGenerationAgent_;
 
   JsPasswordManager* passwordJsManager_;  // weak
+  web::WebState* webState_;               // weak
 
   // The pending form data.
   std::unique_ptr<autofill::PasswordFormFillData> formData_;
 
   // Bridge to observe WebState from Objective-C.
   std::unique_ptr<web::WebStateObserverBridge> webStateObserverBridge_;
 }
 
+@synthesize isBeingDestroyed = isBeingDestroyed_;

[Eugene But (OOO till 7-30), 2016/05/03 17:49:49]

Sorry the name I suggested was not very good. isWebStateDestroyed sounds more
correct.

[vabr (Chromium), 2016/05/04 15:03:46]

Done.

+
 - (instancetype)initWithWebState:(web::WebState*)webState
              passwordsUiDelegate:(id<PasswordsUiDelegate>)UIDelegate {
   self = [self initWithWebState:webState
             passwordsUiDelegate:UIDelegate
                          client:nullptr];
   return self;
 }
 
 - (instancetype)initWithWebState:(web::WebState*)webState
              passwordsUiDelegate:(id<PasswordsUiDelegate>)UIDelegate
                           client:(std::unique_ptr<PasswordManagerClient>)
                                      passwordManagerClient {
   DCHECK(webState);
   self = [super init];
   if (self) {
-    webStateObserverBridge_.reset(
-        new web::WebStateObserverBridge(webState, self));
+    isBeingDestroyed_ = NO;

[Eugene But (OOO till 7-30), 2016/05/03 17:49:49]

From Objective-C Style Guide:

Don't initialize instance variables to 0 or nil in the init method; doing so is
redundant.

https://engdoc.corp.google.com/eng/doc/objcguide.xml?showone=Initialization&c...

[vabr (Chromium), 2016/05/04 15:03:46]

Done.

+    webState_ = webState;

[Eugene But (OOO till 7-30), 2016/05/03 17:49:49]

This change is not part of the bugfix and because of that I would limit this CL
to have only the fix.
This CL may be cherrypicked to release branch so we should limit the amount of
changes on branches.

[vabr (Chromium), 2016/05/04 15:03:46]

Done.

     if (passwordManagerClient)
       passwordManagerClient_ = std::move(passwordManagerClient);
     else
       passwordManagerClient_.reset(new IOSChromePasswordManagerClient(self));
     passwordManager_.reset(new PasswordManager(passwordManagerClient_.get()));
     passwordManagerDriver_.reset(new IOSChromePasswordManagerDriver(self));
     if (experimental_flags::IsPasswordGenerationEnabled() &&
         !passwordManagerClient_->IsOffTheRecord()) {
       passwordGenerationManager_.reset(new PasswordGenerationManager(
           passwordManagerClient_.get(), passwordManagerDriver_.get()));
       passwordGenerationAgent_.reset([[PasswordGenerationAgent alloc]
                initWithWebState:webState
                 passwordManager:passwordManager_.get()
           passwordManagerDriver:passwordManagerDriver_.get()
             passwordsUiDelegate:UIDelegate]);
     }
 
     passwordJsManager_ = base::mac::ObjCCastStrict<JsPasswordManager>(
         [webState->GetJSInjectionReceiver()
             instanceOfClass:[JsPasswordManager class]]);
+    webStateObserverBridge_.reset(

[Eugene But (OOO till 7-30), 2016/05/03 17:49:49]

This also not a part of the fix, so please exclude this from CL.

[vabr (Chromium), 2016/05/04 15:03:46]

Done.

+        new web::WebStateObserverBridge(webState, self));
   }
   return self;
 }
 
 - (instancetype)init {
   NOTREACHED();
   return nil;
 }
 
 - (void)dealloc {
   [self detach];
   [super dealloc];
 }
 
 - (ios::ChromeBrowserState*)browserState {
-  return webStateObserverBridge_ && webStateObserverBridge_->web_state()
-             ? ios::ChromeBrowserState::FromBrowserState(
-                   webStateObserverBridge_->web_state()->GetBrowserState())
-             : nullptr;
+  return webState_ ? ios::ChromeBrowserState::FromBrowserState(
+                         webState_->GetBrowserState())
+                   : nullptr;
 }
 
 - (const GURL&)lastCommittedURL {
   if (!webStateObserverBridge_ || !webStateObserverBridge_->web_state())
     return GURL::EmptyGURL();
   return webStateObserverBridge_->web_state()->GetLastCommittedURL();
 }
 
 - (void)detach {
+  webState_ = nullptr;
   webStateObserverBridge_.reset();
   passwordGenerationAgent_.reset();
   passwordGenerationManager_.reset();
   passwordManagerDriver_.reset();
   passwordManager_.reset();
   passwordManagerClient_.reset();
 }
 
 - (void)findAndFillPasswordForms:(NSString*)username
                         password:(NSString*)password
@@ skipping 56 matching lines @@
 
 - (void)webState:(web::WebState*)webState
     didSubmitDocumentWithFormNamed:(const std::string&)formName
                      userInitiated:(BOOL)userInitiated {
   base::WeakNSObject<PasswordController> weakSelf(self);
   // This code is racing against the new page loading and will not get the
   // password form data if the page has changed. In most cases this code wins
   // the race.
   // TODO(crbug.com/418827): Fix this by passing in more data from the JS side.
   id completionHandler = ^(BOOL found, const autofill::PasswordForm& form) {
-    if (weakSelf) {
+    if (weakSelf && [weakSelf isBeingDestroyed] == NO) {

[Eugene But (OOO till 7-30), 2016/05/03 17:49:49]

NIT: ![weakSelf isBeingDestroyed]

[vabr (Chromium), 2016/05/04 15:03:46]

Done.

       weakSelf.get()->passwordManager_->OnPasswordFormSubmitted(
           weakSelf.get()->passwordManagerDriver_.get(), form);
     }
   };
   [self extractSubmittedPasswordForm:formName
                    completionHandler:completionHandler];
 }
 
 - (void)webStateDestroyed:(web::WebState*)webState {
+  isBeingDestroyed_ = YES;
   [self detach];
 }
 
 - (void)findPasswordFormsWithCompletionHandler:
     (void (^)(const std::vector<autofill::PasswordForm>&))completionHandler {
   DCHECK(completionHandler);
 
   if (!webStateObserverBridge_ || !webStateObserverBridge_->web_state())
     return;
 
@@ skipping 405 matching lines @@
 
 - (PasswordManager*)passwordManager {
   return passwordManager_.get();
 }
 
 - (JsPasswordManager*)passwordJsManager {
   return passwordJsManager_;
 }
 
 @end