Bug 1321907 - Remove mIsProfilerActive. r?njn draft
authorMarkus Stange <mstange@themasta.com>
Wed, 22 Mar 2017 21:45:10 -0400
changeset 503320 c9f7da493a20fbef2536dd3269cfe9b6772f5c49
parent 503319 2f5b8c8747ab81c7a46014bb6f932bdbc4ef5b59
child 503321 44a45d5b18e1c919a0b063f1ddaa009b15944487
push id50537
push userbmo:mstange@themasta.com
push dateThu, 23 Mar 2017 02:37:11 +0000
reviewersnjn
bugs1321907
milestone55.0a1
Bug 1321907 - Remove mIsProfilerActive. r?njn Replace it with profiler_is_active() in one place, and simply remove it in the other places. These other places are: - Around the call to profiler_OOP_exit_profile: profiler_OOP_exit_profile itself already checks whether the profiler is running and does nothing if it's not. - When handling the 'profiler-subprocess-gather' notification. This notification is sent by the profiler because it's interested in the profile, so there's little reason to reject it. - In RecvProfile: If the child process sent us a profile, it did so in response to a GatherProfile request, so chances are that we're still interested in that response. These changes may get us a little closer to a state where you can call getProfileDataAsync, stop the profiler before the content process profiles have all come in, and then still receive a response with all the profiles. At the moment, stopping the profiler will abort the profile gathering process, but that seems more like an accident and less like the behavior you'd want. MozReview-Commit-ID: 2tRXC70BztJ
tools/profiler/gecko/CrossProcessProfilerController.cpp
tools/profiler/public/CrossProcessProfilerController.h
--- a/tools/profiler/gecko/CrossProcessProfilerController.cpp
+++ b/tools/profiler/gecko/CrossProcessProfilerController.cpp
@@ -56,43 +56,40 @@ private:
 };
 
 NS_IMPL_ISUPPORTS(ProfilerObserver, nsIObserver)
 
 CrossProcessProfilerController::CrossProcessProfilerController(
   ProfilerControllingProcess* aProcess)
   : mProcess(aProcess)
   , mObserver(new ProfilerObserver(*this))
-  , mIsProfilerActive(false)
 {
-  nsCOMPtr<nsIProfiler> profiler(do_GetService("@mozilla.org/tools/profiler;1"));
-  bool profilerActive = false;
-  DebugOnly<nsresult> rv = profiler->IsActive(&profilerActive);
-  MOZ_ASSERT(NS_SUCCEEDED(rv));
-
-  if (profilerActive) {
+  if (profiler_is_active()) {
+    // If the profiler is already running in this process, start it in the
+    // child process immediately.
     nsCOMPtr<nsIProfilerStartParams> currentProfilerParams;
-    rv = profiler->GetStartParams(getter_AddRefs(currentProfilerParams));
+    nsCOMPtr<nsIProfiler> profiler(do_GetService("@mozilla.org/tools/profiler;1"));
+    DebugOnly<nsresult> rv = profiler->GetStartParams(getter_AddRefs(currentProfilerParams));
     MOZ_ASSERT(NS_SUCCEEDED(rv));
 
     StartProfiler(currentProfilerParams);
   }
 
   nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
   if (obs) {
     size_t length = ArrayLength(sObserverTopics);
     for (size_t i = 0; i < length; ++i) {
       obs->AddObserver(mObserver, sObserverTopics[i], false);
     }
   }
 }
 
 CrossProcessProfilerController::~CrossProcessProfilerController()
 {
-  if (mIsProfilerActive && !mProfile.IsEmpty()) {
+  if (!mProfile.IsEmpty()) {
     profiler_OOP_exit_profile(mProfile);
   }
 
   nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
   if (obs) {
     size_t length = ArrayLength(sObserverTopics);
     for (size_t i = 0; i < length; ++i) {
       obs->RemoveObserver(mObserver, sObserverTopics[i]);
@@ -111,33 +108,29 @@ CrossProcessProfilerController::StartPro
 
   ipcParams.enabled() = true;
   aParams->GetEntries(&ipcParams.entries());
   aParams->GetInterval(&ipcParams.interval());
   ipcParams.features() = aParams->GetFeatures();
   ipcParams.threadFilters() = aParams->GetThreadFilterNames();
 
   mProcess->SendStartProfiler(ipcParams);
-
-  mIsProfilerActive = true;
 }
 
 void
 CrossProcessProfilerController::Observe(nsISupports* aSubject,
                                         const char* aTopic)
 {
   if (!strcmp(aTopic, "profiler-subprocess-gather")) {
     // profiler-subprocess-gather is the request to capture the profile. We
     // need to tell the other process that we're interested in its profile,
     // and we tell the gatherer that we've forwarded the request, so that it
     // can keep track of the number of pending profiles.
-    if (mIsProfilerActive) {
-      profiler_will_gather_OOP_profile();
-      mProcess->SendGatherProfile();
-    }
+    profiler_will_gather_OOP_profile();
+    mProcess->SendGatherProfile();
   }
   else if (!strcmp(aTopic, "profiler-subprocess")) {
     // profiler-subprocess is sent once the gatherer knows that all other
     // processes have replied with their profiles. It's sent during the final
     // assembly of the parent process profile, and this is where we pass the
     // subprocess profile along.
     nsCOMPtr<nsIProfileSaveEvent> pse = do_QueryInterface(aSubject);
     if (pse) {
@@ -150,35 +143,31 @@ CrossProcessProfilerController::Observe(
   // These four notifications are sent by the profiler when its corresponding
   // methods are called inside this process. These state changes just need to
   // be forwarded to the other process.
   else if (!strcmp(aTopic, "profiler-started")) {
     nsCOMPtr<nsIProfilerStartParams> params(do_QueryInterface(aSubject));
     StartProfiler(params);
   }
   else if (!strcmp(aTopic, "profiler-stopped")) {
-    mIsProfilerActive = false;
     mProcess->SendStopProfiler();
   }
   else if (!strcmp(aTopic, "profiler-paused")) {
     mProcess->SendPauseProfiler(true);
   }
   else if (!strcmp(aTopic, "profiler-resumed")) {
     mProcess->SendPauseProfiler(false);
   }
 }
 
 // This is called in response to a SendGatherProfile request, or when the
 // other process exits while the profiler is running.
 void
 CrossProcessProfilerController::RecvProfile(const nsCString& aProfile)
 {
-  if (NS_WARN_IF(!mIsProfilerActive)) {
-    return;
-  }
   // Store the profile on this object.
   mProfile = aProfile;
   // Tell the gatherer that we've received the profile from this process, but
   // don't actually give it the profile. It will request the profile once all
   // processes have replied, through the "profiler-subprocess" observer
   // notification.
   profiler_gathered_OOP_profile();
 }
--- a/tools/profiler/public/CrossProcessProfilerController.h
+++ b/tools/profiler/public/CrossProcessProfilerController.h
@@ -31,14 +31,13 @@ private:
   void StartProfiler(nsIProfilerStartParams* aParams);
   void Observe(nsISupports* aSubject, const char* aTopic);
 
   friend class ProfilerObserver;
 
   ProfilerControllingProcess* mProcess;
   RefPtr<ProfilerObserver> mObserver;
   nsCString mProfile;
-  bool mIsProfilerActive;
 };
 
 } // namespace mozilla
 
 #endif