Fix the blimp settings crash.

blimp/client/core/context/blimp_client_context_impl.cc

Index: blimp/client/core/context/blimp_client_context_impl.cc
diff --git a/blimp/client/core/context/blimp_client_context_impl.cc b/blimp/client/core/context/blimp_client_context_impl.cc
index 81769e53483afa6bf70bf0211f742a2be050e0bc..7ac95eebc6e7fb13991990a010cba7dc420fdf63 100644
--- a/blimp/client/core/context/blimp_client_context_impl.cc
+++ b/blimp/client/core/context/blimp_client_context_impl.cc
@@ -149,6 +149,17 @@ BlimpClientContextImpl::~BlimpClientContextImpl() {
 
 void BlimpClientContextImpl::SetDelegate(BlimpClientContextDelegate* delegate) {

[Khushal, 2016/11/03 00:13:35]

This is being called with a null delegate from its dtor. See
https://cs.chromium.org/chromium/src/chrome/browser/android/blimp/chrome_blim...

So what you want to is:
DCHECK(!delegate_ || !delegate), basically one of them should be null. Then,

if (delegate_) {
  assignment_fetcher_ = nullptr;
}
delegate_ = delegate;
if (delegate_) {
  assignment_fetcher_ = // Create the fetcher.
}

[nyquist, 2016/11/03 00:21:40]

This makes sense to me.

[xingliu, 2016/11/03 00:33:31]

Sorry I didn't notice in dtor it sets to nullptr.
But assignment_fetcher_can probably out live the delegate? 

[Khushal, 2016/11/03 00:50:48]

I just looked at that class, and yes, it does look like the fetcher doesn't have
anything to do with the delegate. But in that case, I'm super confused why we
have a delegate callback for creating the IdentitySource. Why don't we pass it
in the context's ctor, like the other things tied to the Profile when creating
it.
For instance, the PrefService is passed at that time here:
https://cs.chromium.org/chromium/src/chrome/browser/android/blimp/blimp_clien...

So why not create the IdentityProvider there as well,
base::WrapUnique<IdentityProvider>(new ProfileIdentityProvider(
      SigninManagerFactory::GetForProfile(profile_),
      ProfileOAuth2TokenServiceFactory::GetForProfile(profile_),
      base::Closure()))
You can get profile the same way, Profile::FromBrowserContext(context).

Am I missing something here?

[xingliu, 2016/11/03 01:45:37]

Yeah, that's probably the right way to do that :)

   delegate_ = delegate;
+
+  // |assignment_fetcher_| needs services from BlimpClientContextDelegate, so
+  // it can't be created in the constructor.
+  DCHECK(delegate_);
+  assignment_fetcher_ = base::MakeUnique<AssignmentFetcher>(
+      io_thread_task_runner_, file_thread_task_runner_,
+      delegate_->CreateIdentityProvider(), GetAssignerURL(),
+      base::Bind(&BlimpClientContextImpl::OnAssignmentReceived,
+                 weak_factory_.GetWeakPtr()),
+      base::Bind(&BlimpClientContextDelegate::OnAuthenticationError,
+                 base::Unretained(delegate_)));
 }
 
 std::unique_ptr<BlimpContents> BlimpClientContextImpl::CreateBlimpContents(
@@ -161,15 +172,7 @@ std::unique_ptr<BlimpContents> BlimpClientContextImpl::CreateBlimpContents(
 }
 
 void BlimpClientContextImpl::Connect() {
-  if (!assignment_fetcher_) {
-    assignment_fetcher_ = base::MakeUnique<AssignmentFetcher>(
-        io_thread_task_runner_, file_thread_task_runner_,
-        delegate_->CreateIdentityProvider(), GetAssignerURL(),
-        base::Bind(&BlimpClientContextImpl::OnAssignmentReceived,
-                   weak_factory_.GetWeakPtr()),
-        base::Bind(&BlimpClientContextDelegate::OnAuthenticationError,
-                   base::Unretained(delegate_)));
-  }
+  DCHECK(assignment_fetcher_);
   assignment_fetcher_->Fetch();
 }
 
@@ -187,6 +190,7 @@ BlimpClientContextImpl::CreateFeedbackData() {
 }
 
 IdentitySource* BlimpClientContextImpl::GetIdentitySource() {
+  DCHECK(assignment_fetcher_);
   return assignment_fetcher_->GetIdentitySource();
 }