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..b912edc8138c8133a7d9b6a775518b9dd92e49b7 100644
--- a/blimp/client/core/context/blimp_client_context_impl.cc
+++ b/blimp/client/core/context/blimp_client_context_impl.cc
@@ -161,15 +161,9 @@ 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_)));
-  }
+  if (!assignment_fetcher_)
+    CreateAssignmentFetcher();
+
   assignment_fetcher_->Fetch();
 }
 
@@ -187,6 +181,9 @@ BlimpClientContextImpl::CreateFeedbackData() {
 }
 
 IdentitySource* BlimpClientContextImpl::GetIdentitySource() {
+  if (!assignment_fetcher_)
+    CreateAssignmentFetcher();
+
   return assignment_fetcher_->GetIdentitySource();
 }
 
@@ -253,6 +250,22 @@ void BlimpClientContextImpl::DropConnection() {
       FROM_HERE, base::Bind(&DropConnectionOnIOThread, net_components_.get()));
 }
 
+void BlimpClientContextImpl::CreateAssignmentFetcher() {
+  if (assignment_fetcher_)

[Khushal, 2016/11/02 23:45:45]

nit: Why do the if check at the call sites if you're doing it here anyway?

+    return;
+
+  // |assignment_fetcher_| needs services from BlimpClientContextDelegate, so
+  // it can't be created in the constructor.
+  DCHECK(delegate_);
+  assignment_fetcher_ = base::MakeUnique<AssignmentFetcher>(

[Khushal, 2016/11/02 21:29:56]

Can we create this in SetDelegate instead then? Rather than having the if check
everywhere?

[xingliu, 2016/11/02 23:26:52]

@nyquist, I think do it in SetDelegate makes sense, but is there any assumption
in SetDelegate that we only can called it for once, or BlimpClientContext can
run without a delegate.

Since Multi window android can potentially have two
ChromeTabbedActivity(ChromeTabbedActivity2 for the second window), so
SetDelegate can get called twice.

[Khushal, 2016/11/02 23:45:45]

Definitely can't do without a |delegate_|, you need it for the
|assignment_fetcher_| for instance. :)
Hmmm, well, there should be only one delegate for a context at any instant for
sure. Reading from here:
https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chr...

And when the delegate is dying it will reset its pointer to you. Shouldn't we
destroy everything we initialized using the delegate at this point? I would
expect that even if after this the context gets re-used by another delegate. So
you have a clear initialization sequence where you create everything tied to a
delegate when you get it, and destroy the same things when its going away.

And for the calls, like Connect, which should be made only after the context has
been provided with a delegate, a HasDelegate check would help anyway.

[xingliu, 2016/11/03 00:08:04]

Yeah, agree.
Put it into SetDelegate. Since the delegate now is always needed.

+      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_)));
+}
+
 void BlimpClientContextImpl::OnImageDecodeError() {
   // Currently we just drop the connection on image decoding error.
   DropConnection();