PasswordController should give up on WebState destruction

ios/chrome/browser/passwords/password_controller.mm

Index: ios/chrome/browser/passwords/password_controller.mm
diff --git a/ios/chrome/browser/passwords/password_controller.mm b/ios/chrome/browser/passwords/password_controller.mm
index 98369d96a1b0ebb2a5bf015d6c54d2018760e0c7..ddc95ef12717ecfcc28e145caef5bf477aa84313 100644
--- a/ios/chrome/browser/passwords/password_controller.mm
+++ b/ios/chrome/browser/passwords/password_controller.mm
@@ -234,6 +234,7 @@ bool GetPageURLAndCheckTrustLevel(web::WebState* web_state, GURL* page_url) {
   base::scoped_nsobject<PasswordGenerationAgent> passwordGenerationAgent_;
 
   JsPasswordManager* passwordJsManager_;  // weak
+  web::WebState* webState_;               // weak
 
   // The pending form data.
   std::unique_ptr<autofill::PasswordFormFillData> formData_;
@@ -242,6 +243,8 @@ bool GetPageURLAndCheckTrustLevel(web::WebState* web_state, GURL* page_url) {
   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
@@ -257,8 +260,8 @@ bool GetPageURLAndCheckTrustLevel(web::WebState* web_state, GURL* page_url) {
   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
@@ -279,6 +282,8 @@ bool GetPageURLAndCheckTrustLevel(web::WebState* web_state, GURL* page_url) {
     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;
 }
@@ -294,10 +299,9 @@ bool GetPageURLAndCheckTrustLevel(web::WebState* web_state, GURL* page_url) {
 }
 
 - (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 {
@@ -307,6 +311,7 @@ bool GetPageURLAndCheckTrustLevel(web::WebState* web_state, GURL* page_url) {
 }
 
 - (void)detach {
+  webState_ = nullptr;
   webStateObserverBridge_.reset();
   passwordGenerationAgent_.reset();
   passwordGenerationManager_.reset();
@@ -383,7 +388,7 @@ bool GetPageURLAndCheckTrustLevel(web::WebState* web_state, GURL* page_url) {
   // 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);
     }
@@ -393,6 +398,7 @@ bool GetPageURLAndCheckTrustLevel(web::WebState* web_state, GURL* page_url) {
 }
 
 - (void)webStateDestroyed:(web::WebState*)webState {
+  isBeingDestroyed_ = YES;
   [self detach];
 }