Avoid touching ExtendedAuthenticatorImpl's ref count before its fully constructed

ExtendedAuthenticatorImpl is a ref counted object. Its first reference
is taken by base::Bind in its constructor, and the reference is passed
to GetSystemSalt to do an asynchronous task.
However, if GetSystemSalt impl drops the passed callback soon, (e.g.
by PostTask failure), the reference to ExtendedAuthenticatorImpl is
gone and the instance is destroyed, even before the construction has
completed. That is, `new ExtendedAuthenticatorImpl` might return a
stale pointer on the previous code.

This CL adds a static constructor to ExtendedAuthenticatorImpl, and
moves the problematic part out of the constructor.

Bug: 866456
Change-Id: I377acc8a753c01a79a4cf09617bfa3b0f6aa923f
Reviewed-on: https://chromium-review.googlesource.com/1148164Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Taiju Tsuiki <tzik@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577766}

chromeos/login/auth/extended_authenticator.cc

@@ -11,13 +11,13 @@ namespace chromeos {
 // static
 scoped_refptr<ExtendedAuthenticator> ExtendedAuthenticator::Create(
     NewAuthStatusConsumer* consumer) {
-  return base::MakeRefCounted<ExtendedAuthenticatorImpl>(consumer);
+  return ExtendedAuthenticatorImpl::Create(consumer);
 }
 
 // static
 scoped_refptr<ExtendedAuthenticator> ExtendedAuthenticator::Create(
       AuthStatusConsumer* consumer) {
-  return base::MakeRefCounted<ExtendedAuthenticatorImpl>(consumer);
+  return ExtendedAuthenticatorImpl::Create(consumer);
 }
 
 ExtendedAuthenticator::ExtendedAuthenticator() = default;

chromeos/login/auth/extended_authenticator_impl.cc

@@ -44,18 +44,34 @@ void RecordEndMarker(const std::string& marker) {
 
 }  // namespace
 
+// static
+scoped_refptr<ExtendedAuthenticatorImpl> ExtendedAuthenticatorImpl::Create(
+    NewAuthStatusConsumer* consumer) {
+  auto extended_authenticator =
+      base::WrapRefCounted(new ExtendedAuthenticatorImpl(consumer));
+  SystemSaltGetter::Get()->GetSystemSalt(base::Bind(
+      &ExtendedAuthenticatorImpl::OnSaltObtained, extended_authenticator));
+  return extended_authenticator;
+}
+
+// static
+scoped_refptr<ExtendedAuthenticatorImpl> ExtendedAuthenticatorImpl::Create(
+    AuthStatusConsumer* consumer) {
+  auto extended_authenticator =
+      base::WrapRefCounted(new ExtendedAuthenticatorImpl(consumer));
+  SystemSaltGetter::Get()->GetSystemSalt(base::Bind(
+      &ExtendedAuthenticatorImpl::OnSaltObtained, extended_authenticator));
+  return extended_authenticator;
+}
+
 ExtendedAuthenticatorImpl::ExtendedAuthenticatorImpl(
     NewAuthStatusConsumer* consumer)
     : salt_obtained_(false), consumer_(consumer), old_consumer_(NULL) {
-  SystemSaltGetter::Get()->GetSystemSalt(
-      base::Bind(&ExtendedAuthenticatorImpl::OnSaltObtained, this));
 }
 
 ExtendedAuthenticatorImpl::ExtendedAuthenticatorImpl(
     AuthStatusConsumer* consumer)
     : salt_obtained_(false), consumer_(NULL), old_consumer_(consumer) {
-  SystemSaltGetter::Get()->GetSystemSalt(
-      base::Bind(&ExtendedAuthenticatorImpl::OnSaltObtained, this));
 }
 
 void ExtendedAuthenticatorImpl::SetConsumer(AuthStatusConsumer* consumer) {

chromeos/login/auth/extended_authenticator_impl.h

@@ -25,8 +25,10 @@ class UserContext;
 // Implements ExtendedAuthenticator.
 class CHROMEOS_EXPORT ExtendedAuthenticatorImpl : public ExtendedAuthenticator {
  public:
-  explicit ExtendedAuthenticatorImpl(NewAuthStatusConsumer* consumer);
-  explicit ExtendedAuthenticatorImpl(AuthStatusConsumer* consumer);
+  static scoped_refptr<ExtendedAuthenticatorImpl> Create(
+      NewAuthStatusConsumer* consumer);
+  static scoped_refptr<ExtendedAuthenticatorImpl> Create(
+      AuthStatusConsumer* consumer);
 
   // ExtendedAuthenticator:
   void SetConsumer(AuthStatusConsumer* consumer) override;
@@ -49,6 +51,8 @@ class CHROMEOS_EXPORT ExtendedAuthenticatorImpl : public ExtendedAuthenticator {
                             const ContextCallback& callback) override;
 
  private:
+  explicit ExtendedAuthenticatorImpl(NewAuthStatusConsumer* consumer);
+  explicit ExtendedAuthenticatorImpl(AuthStatusConsumer* consumer);
   ~ExtendedAuthenticatorImpl() override;
 
   // Callback for system salt getter.