content::RTCCertificateGenerator and WebRTC-EnableWebRtcEcdsa update.

content/renderer/media/rtc_certificate_generator.cc

 // Copyright (c) 2015 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 "content/renderer/media/rtc_certificate_generator.h"
 
 #include <string>
 #include <utility>
 
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
+#include "base/synchronization/lock.h"
+#include "base/synchronization/waitable_event.h"
 #include "content/renderer/media/peer_connection_identity_store.h"
 #include "content/renderer/media/rtc_certificate.h"
 #include "content/renderer/media/webrtc/peer_connection_dependency_factory.h"
 #include "content/renderer/render_thread_impl.h"
-#include "third_party/webrtc/base/rtccertificate.h"
-#include "third_party/webrtc/base/scoped_ref_ptr.h"
 #include "url/gurl.h"
 
 namespace content {
 namespace {
 
 rtc::KeyParams WebRTCKeyParamsToKeyParams(
     const blink::WebRTCKeyParams& key_params) {
   switch (key_params.keyType()) {
     case blink::WebRTCKeyTypeRSA:
       return rtc::KeyParams::RSA(key_params.rsaParams().modLength,
                                  key_params.rsaParams().pubExp);
     case blink::WebRTCKeyTypeECDSA:
       return rtc::KeyParams::ECDSA(
           static_cast<rtc::ECCurve>(key_params.ecCurve()));
     default:
       NOTREACHED();
       return rtc::KeyParams();
   }
 }
 
 // Observer used by RTCCertificateGenerator::generateCertificate.
 class RTCCertificateIdentityObserver
     : public webrtc::DtlsIdentityRequestObserver {
  public:
   RTCCertificateIdentityObserver(
       const scoped_refptr<base::SingleThreadTaskRunner>& main_thread,
       const scoped_refptr<base::SingleThreadTaskRunner>& signaling_thread)
       : main_thread_(main_thread),
         signaling_thread_(signaling_thread),
-        observer_(nullptr) {
+        observer_(nullptr),
+        request_event_(false, false) {
     DCHECK(main_thread_);
     DCHECK(signaling_thread_);
   }
   ~RTCCertificateIdentityObserver() override {}
 
-  // Perform |store|->RequestIdentity with this identity observer and ensure
-  // that this identity observer is not deleted until the request has completed
-  // by holding on to a reference to itself for the duration of the request.
+  base::WaitableEvent& request_event() {

[tommi (sloooow) - chröme, 2016/05/12 12:42:55]

non-const on purpose?  If you only need it to wait, I'd prefer having a Wait()
method here and call request_event_.Wait() from there.  As is, this getter
exposes the whole object.

[hbos_chromium, 2016/05/12 14:27:59]

Done. Made request_event_ mutable.

+    return request_event_;
+  }
+
+  rtc::scoped_refptr<rtc::RTCCertificate> Result() const {

[tommi (sloooow) - chröme, 2016/05/12 12:42:55]

avoid using rtc:: types in Chrome whenever you can.

[hbos_chromium, 2016/05/12 14:27:59]

Done.

+    result_lock_.Acquire();

[tommi (sloooow) - chröme, 2016/05/12 12:42:55]

use a scoped locker object.

Why do you need the lock though?

How many times will Result() be called and why does result_ need to be valid
after the first call (including in the dtor)?

[hbos_chromium, 2016/05/12 14:27:59]

Removed the lock and added comment to make sure caller has done WaitForRequest
before calling.

+    rtc::scoped_refptr<rtc::RTCCertificate> ret(result_);
+    result_lock_.Release();
+    return ret;

[tommi (sloooow) - chröme, 2016/05/12 12:42:55]

std::move, to avoid extra refcount?

[hbos_chromium, 2016/05/12 14:28:00]

Now returning const& instead.

+  }
+
+  // Perform |store|->RequestIdentity with |this| identity observer and ensure
+  // that |this| identity observer is not deleted until the request has
+  // completed by holding on to a reference to itself for the duration of the
+  // request. The |observer| is optional, see |request_event| and |Result|.
   void RequestIdentity(
-      const blink::WebRTCKeyParams& key_params,
+      const rtc::KeyParams& key_params,
       const GURL& url,
       const GURL& first_party_for_cookies,
       const rtc::Optional<uint64_t>& expires_ms,
       std::unique_ptr<blink::WebRTCCertificateCallback> observer) {
     DCHECK(main_thread_->BelongsToCurrentThread());
-    DCHECK(!observer_) << "Already have a RequestIdentity in progress.";
     key_params_ = key_params;
     observer_ = std::move(observer);
-    DCHECK(observer_);
+    request_event_.Reset();

[tommi (sloooow) - chröme, 2016/05/12 12:42:55]

needed?

[hbos_chromium, 2016/05/12 14:28:00]

Removed.

     // Identity request must be performed on the WebRTC signaling thread.
     signaling_thread_->PostTask(FROM_HERE, base::Bind(
         &RTCCertificateIdentityObserver::RequestIdentityOnWebRtcSignalingThread,
         this, url, first_party_for_cookies, expires_ms));
   }
 
  private:
   void RequestIdentityOnWebRtcSignalingThread(
       GURL url,

[tommi (sloooow) - chröme, 2016/05/12 12:42:55]

const &

[hbos_chromium, 2016/05/12 14:27:59]

This copy is on purpose because it is being called cross-threads, a reference
would no longer be valid. (Unless base::Bind automatically copies? Don't think
so?)

       GURL first_party_for_cookies,
       rtc::Optional<uint64_t> expires_ms) {
     DCHECK(signaling_thread_->BelongsToCurrentThread());
     std::unique_ptr<PeerConnectionIdentityStore> store(
         new PeerConnectionIdentityStore(main_thread_, signaling_thread_, url,
                                         first_party_for_cookies));
-    // Request identity with |this| as the observer. OnSuccess/OnFailure will be
-    // called asynchronously.
-    store->RequestIdentity(WebRTCKeyParamsToKeyParams(key_params_),
-                           expires_ms, this);
+    // Request identity with |this| as the observer. |OnSuccess|/|OnFailure|
+    // will be called asynchronously.
+    store->RequestIdentity(key_params_, expires_ms, this);
   }
 
   // webrtc::DtlsIdentityRequestObserver implementation.
   void OnFailure(int error) override {
     DCHECK(signaling_thread_->BelongsToCurrentThread());
-    DCHECK(observer_);
     main_thread_->PostTask(FROM_HERE, base::Bind(
         &RTCCertificateIdentityObserver::DoCallbackOnMainThread,
         this, nullptr));
   }
   void OnSuccess(const std::string& der_cert,
                  const std::string& der_private_key) override {
     std::string pem_cert = rtc::SSLIdentity::DerToPem(
         rtc::kPemTypeCertificate,
         reinterpret_cast<const unsigned char*>(der_cert.data()),
         der_cert.length());
     std::string pem_key = rtc::SSLIdentity::DerToPem(
         rtc::kPemTypeRsaPrivateKey,
         reinterpret_cast<const unsigned char*>(der_private_key.data()),
         der_private_key.length());
     OnSuccess(std::unique_ptr<rtc::SSLIdentity>(
         rtc::SSLIdentity::FromPEMStrings(pem_key, pem_cert)));
   }
   void OnSuccess(std::unique_ptr<rtc::SSLIdentity> identity) override {
     DCHECK(signaling_thread_->BelongsToCurrentThread());
-    DCHECK(observer_);
     rtc::scoped_refptr<rtc::RTCCertificate> certificate =
         rtc::RTCCertificate::Create(std::move(identity));
-    main_thread_->PostTask(
-        FROM_HERE,
-        base::Bind(&RTCCertificateIdentityObserver::DoCallbackOnMainThread,
-                   this, base::Passed(base::WrapUnique(
-                             new RTCCertificate(certificate)))));
+    result_lock_.Acquire();
+    result_ = certificate;
+    result_lock_.Release();
+    request_event_.Signal();

[tommi (sloooow) - chröme, 2016/05/12 12:42:55]

will this signal always be waited upon on the main thread?  If not (and never),
then what about always posting the DoCallbackOnMainThread task, only touch
result_ there and only check the observer_ there?
You might not even need request_event_ actually.

[hbos_chromium, 2016/05/12 14:28:00]

Can't do Wait on the same thread as does the Signal; it does not process posted
messages while Wait:ing so it would deadlock. Setting result and signaling here
instead of in the posted task is on purpose.

+    if (observer_) {

[tommi (sloooow) - chröme, 2016/05/12 12:42:55]

is it safe to check observer_ on this thread without a lock?

[hbos_chromium, 2016/05/12 14:27:59]

There is an assumption about there only being one RequestIdentity pending. I
changed it so that RequestIdentity can only be called once so that we can treat
these as constants from that point on and not lock. (Slightly simpler change
than making sure only one is pending.)

+      main_thread_->PostTask(
+          FROM_HERE,
+          base::Bind(&RTCCertificateIdentityObserver::DoCallbackOnMainThread,
+                     this, certificate));
+    }
   }
 
   void DoCallbackOnMainThread(
-      std::unique_ptr<blink::WebRTCCertificate> certificate) {
+      rtc::scoped_refptr<rtc::RTCCertificate> certificate) {
     DCHECK(main_thread_->BelongsToCurrentThread());
     DCHECK(observer_);
     if (certificate)

[tommi (sloooow) - chröme, 2016/05/12 12:42:55]

now you need {} else {}

[hbos_chromium, 2016/05/12 14:27:59]

Done.

-      observer_->onSuccess(std::move(certificate));
+      observer_->onSuccess(std::unique_ptr<blink::WebRTCCertificate>(
+          new RTCCertificate(certificate)));
     else
       observer_->onError();
     observer_.reset();
   }
 
   // The main thread is the renderer thread.
   const scoped_refptr<base::SingleThreadTaskRunner> main_thread_;
   // The signaling thread is a WebRTC thread used to invoke
   // PeerConnectionIdentityStore::RequestIdentity on, as is required.
   const scoped_refptr<base::SingleThreadTaskRunner> signaling_thread_;
-  blink::WebRTCKeyParams key_params_;
+  rtc::KeyParams key_params_;

[tommi (sloooow) - chröme, 2016/05/12 12:42:55]

can these be const?

[hbos_chromium, 2016/05/12 14:27:59]

I tried to make them const by moving what RequestIdentity does into the
constructor, but I ended up with compile errors trying to instantiate the class
due to templates and having to do std::move with the observer which it did not
do. Did not change this.

DCHECKing that RequestIdentity is only called once though, in callback functions
we can treat these as const since they will not change after a request has been
made.

   std::unique_ptr<blink::WebRTCCertificateCallback> observer_;
+  base::WaitableEvent request_event_;
+  rtc::scoped_refptr<rtc::RTCCertificate> result_;
+  mutable base::Lock result_lock_;
 
   DISALLOW_COPY_AND_ASSIGN(RTCCertificateIdentityObserver);
 };
 
 }  // namespace
 
 void RTCCertificateGenerator::generateCertificate(
     const blink::WebRTCKeyParams& key_params,
     const blink::WebURL& url,
     const blink::WebURL& first_party_for_cookies,
@@ skipping 29 matching lines @@
   PeerConnectionDependencyFactory* pc_dependency_factory =
       RenderThreadImpl::current()->GetPeerConnectionDependencyFactory();
   pc_dependency_factory->EnsureInitialized();
   const scoped_refptr<base::SingleThreadTaskRunner> signaling_thread =
       pc_dependency_factory->GetWebRtcSignalingThread();
 
   rtc::scoped_refptr<RTCCertificateIdentityObserver> identity_observer(
       new rtc::RefCountedObject<RTCCertificateIdentityObserver>(
           main_thread, signaling_thread));
   // |identity_observer| lives until request has completed.
-  identity_observer->RequestIdentity(key_params, url, first_party_for_cookies,
-                                     expires_ms, std::move(observer));
+  identity_observer->RequestIdentity(
+      WebRTCKeyParamsToKeyParams(key_params), url, first_party_for_cookies,
+      expires_ms, std::move(observer));
 #else
   observer->onError();
 #endif
 }
 
+rtc::scoped_refptr<rtc::RTCCertificate>
+RTCCertificateGenerator::generateCertificateAndWait(
+      const rtc::KeyParams& key_params,
+      const rtc::Optional<uint64_t>& expires_ms) {
+#if defined(ENABLE_WEBRTC)
+  const scoped_refptr<base::SingleThreadTaskRunner> main_thread =
+      base::ThreadTaskRunnerHandle::Get();
+
+  PeerConnectionDependencyFactory* pc_dependency_factory =
+      RenderThreadImpl::current()->GetPeerConnectionDependencyFactory();
+  pc_dependency_factory->EnsureInitialized();
+  const scoped_refptr<base::SingleThreadTaskRunner> signaling_thread =
+      pc_dependency_factory->GetWebRtcSignalingThread();
+
+  rtc::scoped_refptr<RTCCertificateIdentityObserver> identity_observer(
+      new rtc::RefCountedObject<RTCCertificateIdentityObserver>(
+          main_thread, signaling_thread));
+
+  // Don't care about URL origin parameters, using empty URL.
+  identity_observer->RequestIdentity(key_params, GURL(), GURL(),
+                                     expires_ms, nullptr);
+  identity_observer->request_event().Wait();
+  return identity_observer->Result();

[tommi (sloooow) - chröme, 2016/05/12 12:42:55]

thinking about it some more, do we need the observer interface, or would holding
a callback object that is Run() on the callback thread (not the main thread),
work?  Using that, an event could be signalled or a task posted to the main
thread.

[hbos_chromium, 2016/05/12 14:28:00]

I think I need to re-read that to understand. I'll reply to this later, I gotta
run!

+#else
+  return nullptr;
+#endif
+}
+
 bool RTCCertificateGenerator::isSupportedKeyParams(
     const blink::WebRTCKeyParams& key_params) {
   return WebRTCKeyParamsToKeyParams(key_params).IsValid();
 }
 
 std::unique_ptr<blink::WebRTCCertificate> RTCCertificateGenerator::fromPEM(
     const std::string& pem_private_key,
     const std::string& pem_certificate) {
   rtc::scoped_refptr<rtc::RTCCertificate> certificate =
       rtc::RTCCertificate::FromPEM(
           rtc::RTCCertificatePEM(pem_private_key, pem_certificate));
   return std::unique_ptr<blink::WebRTCCertificate>(
       new RTCCertificate(certificate));
 }
 
 }  // namespace content