Bug 1111787 - Part 2: Kill plugins with valid node IDs when clearing storage data for those IDs will be invalid and can't be used anymore. r=cpearce
authorJW Wang <jwwang@mozilla.com>
Sun, 28 Dec 2014 22:18:00 -0500
changeset 238995 20298762b610fe8726459f0d17a7cefafc071c5c
parent 238994 7ff735f32aac001514341b16d2275c72b16f320b
child 238996 f3c7e9a00af88d87187eb2e8062fb3edaf684fcf
push id7472
push userraliiev@mozilla.com
push dateMon, 12 Jan 2015 20:36:27 +0000
treeherdermozilla-aurora@300ca104f8fb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscpearce
bugs1111787
milestone37.0a1
Bug 1111787 - Part 2: Kill plugins with valid node IDs when clearing storage data for those IDs will be invalid and can't be used anymore. r=cpearce
dom/media/gmp/GMPParent.cpp
dom/media/gmp/GMPParent.h
dom/media/gmp/GMPService.cpp
--- a/dom/media/gmp/GMPParent.cpp
+++ b/dom/media/gmp/GMPParent.cpp
@@ -57,17 +57,16 @@ namespace gmp {
 
 GMPParent::GMPParent()
   : mState(GMPStateNotLoaded)
   , mProcess(nullptr)
   , mDeleteProcessOnlyOnUnload(false)
   , mAbnormalShutdownInProgress(false)
   , mAsyncShutdownRequired(false)
   , mAsyncShutdownInProgress(false)
-  , mHasAccessedStorage(false)
 {
 }
 
 GMPParent::~GMPParent()
 {
   // Can't Close or Destroy the process here, since destruction is MainThread only
   MOZ_ASSERT(NS_IsMainThread());
 }
@@ -692,22 +691,16 @@ GMPParent::ActorDestroy(ActorDestroyReas
     // infinitely if we do.
     MOZ_ASSERT(mState == GMPStateClosing);
     DeleteProcess();
     // Note: final destruction will be Dispatched to ourself
     mService->ReAddOnGMPThread(self);
   }
 }
 
-bool
-GMPParent::HasAccessedStorage() const
-{
-  return mHasAccessedStorage;
-}
-
 mozilla::dom::PCrashReporterParent*
 GMPParent::AllocPCrashReporterParent(const NativeThreadId& aThread)
 {
 #ifndef MOZ_CRASHREPORTER
   MOZ_ASSERT(false, "Should only be sent if crash reporting is enabled.");
 #endif
   CrashReporterParent* cr = new CrashReporterParent();
   cr->SetChildData(aThread, GeckoProcessType_GMPlugin);
@@ -804,17 +797,16 @@ GMPParent::DeallocPGMPStorageParent(PGMP
 
 bool
 GMPParent::RecvPGMPStorageConstructor(PGMPStorageParent* aActor)
 {
   GMPStorageParent* p  = (GMPStorageParent*)aActor;
   if (NS_WARN_IF(NS_FAILED(p->Init()))) {
     return false;
   }
-  mHasAccessedStorage = true;
   return true;
 }
 
 bool
 GMPParent::RecvPGMPTimerConstructor(PGMPTimerParent* actor)
 {
   return true;
 }
--- a/dom/media/gmp/GMPParent.h
+++ b/dom/media/gmp/GMPParent.h
@@ -108,16 +108,17 @@ public:
   // Plugins are associated with an NodeIds by calling SetNodeId() before
   // loading.
   //
   // If a plugin has no NodeId specified and it is loaded, it is assumed to
   // be shared across NodeIds.
 
   // Specifies that a GMP can only work with the specified NodeIds.
   void SetNodeId(const nsACString& aNodeId);
+  const nsACString& GetNodeId() const { return mNodeId; }
 
   // Returns true if a plugin can be or is being used across multiple NodeIds.
   bool CanBeSharedCrossNodeIds() const;
 
   // A GMP can be used from a NodeId if it's already been set to work with
   // that NodeId, or if it's not been set to work with any NodeId and has
   // not yet been loaded (i.e. it's not shared across NodeIds).
   bool CanBeUsedFrom(const nsACString& aNodeId) const;
@@ -126,18 +127,16 @@ public:
     return nsCOMPtr<nsIFile>(mDirectory).forget();
   }
 
   // GMPSharedMem
   virtual void CheckThread() MOZ_OVERRIDE;
 
   void AbortAsyncShutdown();
 
-  bool HasAccessedStorage() const;
-
 private:
   ~GMPParent();
   nsRefPtr<GeckoMediaPluginService> mService;
   bool EnsureProcessLoaded();
   nsresult ReadGMPMetaData();
 #ifdef MOZ_CRASHREPORTER
   void WriteExtraDataForMinidump(CrashReporter::AnnotationTable& notes);
   void GetCrashID(nsString& aResult);
@@ -192,15 +191,14 @@ private:
   nsCOMPtr<nsIThread> mGMPThread;
   nsCOMPtr<nsITimer> mAsyncShutdownTimeout; // GMP Thread only.
   // NodeId the plugin is assigned to, or empty if the the plugin is not
   // assigned to a NodeId.
   nsAutoCString mNodeId;
 
   bool mAsyncShutdownRequired;
   bool mAsyncShutdownInProgress;
-  bool mHasAccessedStorage;
 };
 
 } // namespace gmp
 } // namespace mozilla
 
 #endif // GMPParent_h_
--- a/dom/media/gmp/GMPService.cpp
+++ b/dom/media/gmp/GMPService.cpp
@@ -1251,29 +1251,27 @@ GeckoMediaPluginService::ClearStorage()
   MOZ_ASSERT(NS_GetCurrentThread() == mGMPThread);
   LOGD(("%s::%s", __CLASS__, __FUNCTION__));
 
 #ifdef MOZ_WIDGET_GONK
   NS_WARNING("GeckoMediaPluginService::ClearStorage not implemented on B2G");
   return;
 #endif
 
-  // Shutdown plugins that have touched storage, as they could have
-  // state that depends on storage. We don't want them to write data
-  // after we've cleared storage, as they could end up in an inconsistent
-  // state, so we must ensure they're shutdown before we actually clear
-  // storage. Note: we can't shut them down while holding the lock,
+  // Shutdown all plugins that have valid node IDs as those IDs will become
+  // invalid and shouldn't be used anymore after we clear storage data.
+  // Note: we can't shut them down while holding the lock,
   // as the lock is not re-entrant and shutdown requires taking the lock.
   // The plugin list is only edited on the GMP thread, so this should be OK.
   nsTArray<nsRefPtr<GMPParent>> pluginsToKill;
   {
     MutexAutoLock lock(mMutex);
     for (size_t i = 0; i < mPlugins.Length(); i++) {
       nsRefPtr<GMPParent> parent(mPlugins[i]);
-      if (parent->HasAccessedStorage()) {
+      if (!parent->GetNodeId().IsEmpty()) {
         pluginsToKill.AppendElement(parent);
       }
     }
   }
 
   for (size_t i = 0; i < pluginsToKill.Length(); i++) {
     pluginsToKill[i]->CloseActive(false);
     // Abort async shutdown because we're going to wipe the plugin's storage,