Media Remoting: Add remoting control logic for encrypted contents.

media/remoting/remoting_cdm_factory.cc

+// 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 "media/remoting/remoting_cdm_factory.h"
+
+#include "base/bind.h"
+#include "base/single_thread_task_runner.h"
+#include "media/base/cdm_config.h"
+#include "media/remoting/remoting_cdm.h"
+
+namespace media {
+
+namespace {
+
+void CreateCdm(const std::string& key_system,
+               const GURL& security_origin,
+               const CdmConfig& cdm_config,
+               const SessionMessageCB& session_message_cb,
+               const SessionClosedCB& session_closed_cb,
+               const SessionKeysChangeCB& session_keys_change_cb,
+               const SessionExpirationUpdateCB& session_expiration_update_cb,
+               const CdmCreatedCB& cdm_created_cb,
+               std::unique_ptr<RemotingCdmController> remoting_controller,
+               CdmFactory* const default_cdm_factory,
+               bool is_remoting) {
+  if (is_remoting) {
+    VLOG(1) << "Create remoting CDM.";
+    // TODO(xjz): Merge this with Eric's implementation to create remoting CDM.
+    NOTIMPLEMENTED();

[xhwang, 2016/11/01 08:21:29]

If we land this CL now, when we are "remoting" (not sure about the exact
meaning), protected content playback will fail. Is this correct? Could this
actually happen in prod?

Can we land a CL to add a command line flag first before we enable remoting by
default? This will help avoid unnecessary noise/stress to you, other devs, and
users.

[xjz, 2016/11/01 21:55:53]

No, we will never enable "remoting" if we land this CL now.
And I will add the command line flag in a separate CL soon. :)

[miu, 2016/11/03 00:03:08]

I talked about this issue here: https://codereview.chromium.org/2470003002/#msg5

tl;dr: Nothing's enabled yet. Let's do the feature flag in browser code (a
single point at a higher level where all code paths can be (de)activated).

[xhwang, 2016/11/03 06:41:01]

Makes sense. As long as we always pick the default Renderer/CDM for now we are
good. When it's possible to pick remoting Renderer/CDM we should have a
"feature" for it.

+  } else {

[xhwang, 2016/11/01 08:21:29]

nit: we like to return early:

if (foo) {
  Bar();
  return;
}

Qux();

[xjz, 2016/11/01 21:55:53]

This seems fine to me, as we will merge with the RemotingCdm implementation very
soon. 

+    VLOG(1) << "Create local CDM.";
+    default_cdm_factory->Create(key_system, security_origin, cdm_config,
+                                session_message_cb, session_closed_cb,
+                                session_keys_change_cb,
+                                session_expiration_update_cb, cdm_created_cb);
+  }
+}
+
+}  // namespace
+
+RemotingCdmFactory::RemotingCdmFactory(
+    std::unique_ptr<CdmFactory> default_cdm_factory,
+    mojom::RemoterFactory* remoter_factory)
+    : default_cdm_factory_(std::move(default_cdm_factory)),
+      remoter_factory_(remoter_factory) {
+  DCHECK(default_cdm_factory_);
+  DCHECK(remoter_factory_);
+}
+
+RemotingCdmFactory::~RemotingCdmFactory() {}
+
+std::unique_ptr<RemotingCdmController>
+RemotingCdmFactory::CreateRemotingController() {
+  mojom::RemotingSourcePtr remoting_source;
+  mojom::RemotingSourceRequest remoting_source_request =
+      mojo::GetProxy(&remoting_source);
+  mojom::RemoterPtr remoter;
+  remoter_factory_->Create(std::move(remoting_source),
+                           mojo::GetProxy(&remoter));
+  return base::MakeUnique<RemotingCdmController>(
+      make_scoped_refptr(new RemotingSourceImpl(
+          std::move(remoting_source_request), std::move(remoter))));
+}
+
+// TODO: Replace the callbacks with an interface. http://crbug.com/657940.

[xhwang, 2016/11/01 08:21:29]

nit: Usually you have a name/ladp for the TODO :)

[xjz, 2016/11/01 21:55:53]

Done.

+void RemotingCdmFactory::Create(
+    const std::string& key_system,
+    const GURL& security_origin,
+    const CdmConfig& cdm_config,
+    const SessionMessageCB& session_message_cb,
+    const SessionClosedCB& session_closed_cb,
+    const SessionKeysChangeCB& session_keys_change_cb,
+    const SessionExpirationUpdateCB& session_expiration_update_cb,
+    const CdmCreatedCB& cdm_created_cb) {
+  std::unique_ptr<RemotingCdmController> remoting_controller =
+      CreateRemotingController();
+  RemotingCdmController* remoting_controller_ptr = remoting_controller.get();

[xhwang, 2016/11/01 08:21:29]

nit: It's worth a comment why you are doing this.

[xjz, 2016/11/01 21:55:53]

Done.

+  remoting_controller_ptr->ShouldCreateRemotingCdm(base::Bind(
+      &CreateCdm, key_system, security_origin, cdm_config, session_message_cb,
+      session_closed_cb, session_keys_change_cb, session_expiration_update_cb,
+      cdm_created_cb, base::Passed(&remoting_controller),
+      default_cdm_factory_.get()));

[xhwang, 2016/11/01 08:21:29]

Is it possible that the callback is fired after |this| is destroyed? If so,
default_cdm_factory will be destroyed as well when the callback fires. It's
probably easier if CreateCdm() is a member function and we use a weakptr to make
sure the callback won't fire after |this| is destroyed.

If you do that, you can also store a pending_remoting_controller_ instead of
passing of ownership of the |remoting_controller| to the callback, which IMHO is
cleaner, but it's totally up to you.

[xjz, 2016/11/01 21:55:53]

Changed CreateCdm() as a member function of RemotingCdmFactory. But the
|remoting_controller| and other parameters other than |default_cdm_factor_|
still need to be passed, as the RemotingCdmFactory is shared by the whole
RenderFrameImpl.

+}
+
+}  // namespace media