Handle plugin instance crash in ContentDecryptorDelegate.

content/renderer/pepper/content_decryptor_delegate.h

 // 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.
 
 #ifndef CONTENT_RENDERER_PEPPER_CONTENT_DECRYPTOR_DELEGATE_H_
 #define CONTENT_RENDERER_PEPPER_CONTENT_DECRYPTOR_DELEGATE_H_
 
 #include <queue>
 #include <string>
 
@@ skipping 30 matching lines @@
 
   void Initialize(const std::string& key_system);
 
   void SetSessionEventCallbacks(
       const media::SessionCreatedCB& session_created_cb,
       const media::SessionMessageCB& session_message_cb,
       const media::SessionReadyCB& session_ready_cb,
       const media::SessionClosedCB& session_closed_cb,
       const media::SessionErrorCB& session_error_cb);
 
+  void InstanceCrashed();
+
   // Provides access to PPP_ContentDecryptor_Private.
   bool CreateSession(uint32 session_id,
                      const std::string& type,
                      const uint8* init_data,
                      int init_data_length);
   bool UpdateSession(uint32 session_id,
                      const uint8* response,
                      int response_length);
   bool ReleaseSession(uint32 session_id);
   bool Decrypt(media::Decryptor::StreamType stream_type,
@@ skipping 117 matching lines @@
 
   std::queue<uint32_t> free_buffers_;
 
   // Keep track of audio parameters.
   int audio_samples_per_second_;
   int audio_channel_count_;
 
   base::WeakPtr<ContentDecryptorDelegate> weak_this_;
   base::WeakPtrFactory<ContentDecryptorDelegate> weak_ptr_factory_;
 
+  bool is_instance_crashed_;

[xhwang, 2013/12/28 01:00:28]

I am debating on this. Probably when the plugin is crashed, we should just
destroy this ContentDecryptorDelegate object. WDYT?

[ddorwin, 2014/01/07 19:28:48]

You can't simply destroy this object without informing PpapiDecryptor and higher
layers to stop calling Decrypt(), etc. PpapiDecryptor assumes there is always a
delegate, so you'd have to add such checks there anyway. Higher-level code might
already have checks for decryptor_ != NULL, but it's also more likely that there
are assumptions about the decryptor/decoder not changing.

To fully clean up, we would tell PpapiDecryptor, which would clean up the WHP as
well. (Such a change would also need the delegate != NULL checks mentioned
above.) However, that's a lot of complexity for an unexpected case.

is_instance_crashed_ isn't great, but it's simpler and isolated. Maybe it should
be is_valid_ or something similar that is more generic instead.

Pushing this knowledge into PpapiDecryptor would allow us to eliminate the
(previously always true) bool return values from most of the methods, which is
nice since it removes ambiguity. I don't think I'd go further up the stack than
that, though.

[xhwang, 2014/01/07 20:48:34]

Thanks for the comments. In summary, notifying PpapiDecryptor would allow us to
do more cleanups, but that requires more complexity. Since this is only a corner
case, I am inclined to keep |is_valid_| to make this CL simple. WDYT?

BTW, could you take a look at https://codereview.chromium.org/105743010/ first,
which would make this CL simpler? Thanks!

[ddorwin, 2014/01/07 23:59:01]

Agreed and done.

+
   DISALLOW_COPY_AND_ASSIGN(ContentDecryptorDelegate);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_RENDERER_PEPPER_CONTENT_DECRYPTOR_DELEGATE_H_