Clean up Assignment Create in BlimpClientContextImpl.

blimp/client/core/context/assignment_fetcher.cc

Index: blimp/client/core/context/assignment_fetcher.cc
diff --git a/blimp/client/core/context/assignment_fetcher.cc b/blimp/client/core/context/assignment_fetcher.cc
new file mode 100644
index 0000000000000000000000000000000000000000..a1b6aabf9055aa547fedafe86f3916e81970efe7
--- /dev/null
+++ b/blimp/client/core/context/assignment_fetcher.cc
@@ -0,0 +1,71 @@
+// Copyright 2016 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.
+
+#include "blimp/client/core/context/assignment_fetcher.h"
+
+#include <string>
+
+#include "base/bind.h"
+#include "base/memory/ptr_util.h"
+#include "blimp/client/core/session/assignment_source.h"
+
+namespace blimp {
+namespace client {
+
+namespace {
+
+const char kDefaultAssignerUrl[] =
+    "https://blimp-pa.googleapis.com/v1/assignment";
+
+}  // namespace
+
+AssignmentFetcher::AssignmentFetcher(
+    base::Callback<void(AssignmentRequestResult, const Assignment&)>
+        assignment_received_callback,
+    scoped_refptr<base::SingleThreadTaskRunner> io_thread_task_runner,
+    scoped_refptr<base::SingleThreadTaskRunner> file_thread_task_runner)
+    : assignment_received_callback_(assignment_received_callback),
+      io_thread_task_runner_(io_thread_task_runner),
+      file_thread_task_runner_(file_thread_task_runner) {}
+
+AssignmentFetcher::~AssignmentFetcher() {}

[David Trainor- moved to gerrit, 2016/10/13 03:52:29]

= default

[CJ, 2016/10/13 21:19:16]

Done.

+
+void AssignmentFetcher::Fetch(BlimpClientContextDelegate* delegate) {

[David Trainor- moved to gerrit, 2016/10/13 03:52:29]

Should this just take the AssignmentRequestResult callback?  What do you think
about joining the Assigner and IdentitySource errors?

[CJ, 2016/10/13 21:19:16]

Not sure what u mean by the errors, but otherwise done. 

[David Trainor- moved to gerrit, 2016/10/14 16:38:33]

So one of the main reasons we have to pass in a delegate with fetch is that we
rely on the delegate for a few things:

- Accessing an IdentityProvider (IIRC)
- Notifying the caller of IdentitySource errors.

If we could decouple those requirements from this class we wouldn't need to pass
in a delegate here :).

[CJ, 2016/10/18 00:18:32]

Done.

+  delegate_ = delegate;
+  // Start Blimp authentication flow. The OAuth2 token will be used in
+  // assignment source.
+  GetIdentitySource()->Connect();
+}
+
+IdentitySource* AssignmentFetcher::GetIdentitySource() {
+  if (!identity_source_) {
+    CreateIdentitySource();
+  }
+  return identity_source_.get();
+}
+
+void AssignmentFetcher::CreateIdentitySource() {

[David Trainor- moved to gerrit, 2016/10/13 03:52:29]

If we don't need a delegate we could probably just create this in the
constructor.  It needs an IdentityProvider, maybe we can make that an
AssignmentFetcher constructor argument and lazily build the fetcher?

[CJ, 2016/10/13 21:19:16]

I am unfamiliar with doing things lazily. Would you have some time to maybe
explain this to me later?

[David Trainor- moved to gerrit, 2016/10/14 16:38:33]

Sure!  Want to have a quick vc at 1:30 or something?

[CJ, 2016/10/18 00:18:32]

Acknowledged.

+  identity_source_ = base::MakeUnique<IdentitySource>(
+      delegate_, base::Bind(&AssignmentFetcher::OnAuthTokenReceived,

[David Trainor- moved to gerrit, 2016/10/13 03:52:29]

If we remove the delegate dependency we could also pass in an error callback or
something?

[CJ, 2016/10/13 21:19:16]

Not sure what you mean. It needs the delegate to make the IdentitySource. Not
following.

[David Trainor- moved to gerrit, 2016/10/14 16:38:33]

Yeah I was hoping we could rip that whole dependency out of that class too. 
That's what some of the earlier comments were about.  Sorry for the confusion
:(.

[CJ, 2016/10/18 00:18:32]

Done.

+                            base::Unretained(this)));
+}
+
+void AssignmentFetcher::OnAuthTokenReceived(
+    const std::string& client_auth_token) {
+  if (!assignment_source_) {
+    assignment_source_.reset(new AssignmentSource(
+        GetAssignerURL(), io_thread_task_runner_, file_thread_task_runner_));
+  }
+
+  VLOG(1) << "Trying to get assignment.";
+  assignment_source_->GetAssignment(client_auth_token,
+                                    assignment_received_callback_);
+}
+
+GURL AssignmentFetcher::GetAssignerURL() {

[David Trainor- moved to gerrit, 2016/10/13 03:52:29]

This was overridden by blimp_client_context_impl_android to pass the right
assigner URL.  Should we just pass this as an argument?  We could also
potentially pass in the Settings object mlliu@ is working on right now to pull
the assigner URL from in the future.

[CJ, 2016/10/13 21:19:16]

Done.

+  return GURL(kDefaultAssignerUrl);
+}
+
+}  // namespace client
+}  // namespace blimp