Check delegate before running Pepper media-device enumeration/monitoring requests.

content/renderer/pepper/pepper_device_enumeration_host_helper.cc

 // Copyright (c) 2012 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/pepper/pepper_device_enumeration_host_helper.h"
 
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/macros.h"
@@ skipping 17 matching lines @@
 class PepperDeviceEnumerationHostHelper::ScopedEnumerationRequest
     : public base::SupportsWeakPtr<ScopedEnumerationRequest> {
  public:
   // |owner| must outlive this object.
   ScopedEnumerationRequest(PepperDeviceEnumerationHostHelper* owner,
                            const Delegate::DevicesCallback& callback)
       : callback_(callback), requested_(false), sync_call_(false) {
     if (!owner->document_url_.is_valid())
       return;
 
+    if (!owner->delegate_)

[yzshen1, 2017/05/05 17:08:11]

Sorry this has been too long ago. I don't remember this code. :)

Do we understand why delegate could be null? Is there a legitimate reason?

If we early return from, the callback will never be called, does the caller
expect that?

[Guido Urdaneta, 2017/05/05 17:18:12]

It is also expected if the document_url is not valid a few lines above, so I
thought it was OK for a somewhat similar situation of a null delegate. 
I don't know why the delegate can become null. I am familiar with the device
enumeration code, but not so much with the lifetimes of the Pepper objects. I
glanced through the source code, but did not find an obvious reason why this
could be the case.

An alternative to leaving the callback hanging could be to invoke the callback
with an empty result.

I can prepare a patch that does that if you think that is better.

[yzshen1, 2017/05/05 17:22:31]

That makes sense. :)
This is what I am a little concerned about. There may be something wrong
elsewhere leading to an inconsistent state. 
You definitely know this code better than I do now. So it is up to you. :)

+      return;
+
     requested_ = true;
 
     // Note that the callback passed into
     // PepperDeviceEnumerationHostHelper::Delegate::EnumerateDevices() may be
     // called synchronously. In that case, |callback| may destroy this
     // object. So we don't pass in |callback| directly. Instead, we use
     // EnumerateDevicesCallbackBody() to ensure that we always call |callback|
     // asynchronously.
     sync_call_ = true;
-    DCHECK(owner->delegate_);
     owner->delegate_->EnumerateDevices(
         owner->device_type_, owner->document_url_,
         base::Bind(&ScopedEnumerationRequest::EnumerateDevicesCallbackBody,
                    AsWeakPtr()));
     sync_call_ = false;
   }
 
   bool requested() const { return requested_; }
 
  private:
@@ skipping 26 matching lines @@
   ScopedMonitoringRequest(PepperDeviceEnumerationHostHelper* owner,
                           const Delegate::DevicesCallback& callback)
       : owner_(owner),
         callback_(callback),
         requested_(false),
         subscription_id_(0) {
     DCHECK(owner_);
     if (!owner_->document_url_.is_valid())
       return;
 
+    if (!owner->delegate_)
+      return;
+
     requested_ = true;
 
-    DCHECK(owner_->delegate_);
     // |callback| is never called synchronously by StartMonitoringDevices(),
     // so it is OK to pass it directly, even if |callback| destroys |this|.
     subscription_id_ = owner_->delegate_->StartMonitoringDevices(
         owner_->device_type_, owner_->document_url_, callback);
   }
 
   ~ScopedMonitoringRequest() {
     if (requested_ && owner_->delegate_) {
       owner_->delegate_->StopMonitoringDevices(owner_->device_type_,
                                                subscription_id_);
@@ skipping 100 matching lines @@
 void PepperDeviceEnumerationHostHelper::OnNotifyDeviceChange(
     uint32_t callback_id,
     const std::vector<ppapi::DeviceRefData>& devices) {
   resource_host_->host()->SendUnsolicitedReply(
       resource_host_->pp_resource(),
       PpapiPluginMsg_DeviceEnumeration_NotifyDeviceChange(callback_id,
                                                           devices));
 }
 
 }  // namespace content