Revert of Make profiling lock global rather than per Target (patchset #2

of https://codereview.chromium.org/475803002/) Reason for revert: 'common' should not be aware of profiler. Original issue's description: > Make profiling lock global rather than per Target > > We should pause any actions that could affect performance if any of the targets is being profiled, not only current target. > > BUG=403684 > [email protected] > > Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=180336 [email protected],[email protected],[email protected] NOTREECHECKS=true NOTRY=true BUG=403684 Review URL: https://codereview.chromium.org/471393002 git-svn-id: svn://svn.chromium.org/blink/trunk@180342 bbb929c8-8fbe-4397-9dbb-9b2b20218538
yoavweiss · Aug 15, 2014 · 508a201 · 508a201
1 parent 72fef91
commit 508a201
Show file tree

Hide file tree

Showing 16 changed files with 113 additions and 83 deletions.
diff --git a/Source/devtools/devtools.gypi b/Source/devtools/devtools.gypi
@@ -67,7 +67,6 @@
             'front_end/common/Console.js',
             'front_end/common/ContentProvider.js',
             'front_end/common/Geometry.js',
-            'front_end/common/Lock.js',
             'front_end/common/ModuleExtensionInterfaces.js',
             'front_end/common/modules.js',
             'front_end/common/NotificationService.js',

diff --git a/Source/devtools/front_end/audits/AuditLauncherView.js b/Source/devtools/front_end/audits/AuditLauncherView.js
@@ -63,7 +63,7 @@ WebInspector.AuditLauncherView = function(auditController)
     var target = this._auditController.target();
     target.networkManager.addEventListener(WebInspector.NetworkManager.EventTypes.RequestStarted, this._onRequestStarted, this);
     target.networkManager.addEventListener(WebInspector.NetworkManager.EventTypes.RequestFinished, this._onRequestFinished, this);
-    WebInspector.profilingLock.addEventListener(WebInspector.Lock.Events.StateChanged, this._updateButton, this);
+    target.profilingLock.addEventListener(WebInspector.Lock.Events.StateChanged, this._updateButton, this);
 
     var defaultSelectedAuditCategory = {};
     defaultSelectedAuditCategory[WebInspector.AuditLauncherView.AllCategoriesKey] = true;
@@ -144,11 +144,11 @@ WebInspector.AuditLauncherView.prototype = {
         this._toggleUIComponents(this._auditRunning);
         var target = this._auditController.target();
         if (this._auditRunning) {
-            WebInspector.profilingLock.acquire();
+            target.profilingLock.acquire();
             this._startAudit();
         } else {
             this._stopAudit();
-            WebInspector.profilingLock.release();
+            target.profilingLock.release();
         }
     },
 
@@ -327,7 +327,7 @@ WebInspector.AuditLauncherView.prototype = {
     _updateButton: function()
     {
         var target = this._auditController.target();
-        var enable = this._auditRunning || (this._currentCategoriesCount && !WebInspector.profilingLock.isAcquired());
+        var enable = this._auditRunning || (this._currentCategoriesCount && !target.profilingLock.isAcquired());
         this._launchButton.textContent = this._auditRunning ? WebInspector.UIString("Stop") : WebInspector.UIString("Run");
         this._launchButton.disabled = !enable;
         this._launchButton.title = enable ? "" : WebInspector.anotherProfilerActiveLabel();

diff --git a/Source/devtools/front_end/common/Lock.js b/Source/devtools/front_end/common/Lock.js
diff --git a/Source/devtools/front_end/common/Object.js b/Source/devtools/front_end/common/Object.js
@@ -144,6 +144,51 @@ WebInspector.Event.prototype = {
     }
 }
 
+/**
+ * @constructor
+ * @extends {WebInspector.Object}
+ */
+WebInspector.Lock = function()
+{
+    this._count = 0; // Reentrant.
+}
+
+/**
+ * @enum {string}
+ */
+WebInspector.Lock.Events = {
+    StateChanged: "StateChanged"
+}
+
+WebInspector.Lock.prototype = {
+    /**
+     * @return {boolean}
+     */
+    isAcquired: function()
+    {
+        return !!this._count;
+    },
+
+    acquire: function()
+    {
+        if (++this._count === 1)
+            this.dispatchEventToListeners(WebInspector.Lock.Events.StateChanged);
+    },
+
+    release: function()
+    {
+        --this._count;
+        if (this._count < 0) {
+            console.error("WebInspector.Lock acquire/release calls are unbalanced " + new Error().stack);
+            return;
+        }
+        if (!this._count)
+            this.dispatchEventToListeners(WebInspector.Lock.Events.StateChanged);
+    },
+
+    __proto__: WebInspector.Object.prototype
+}
+
 /**
  * @interface
  */

diff --git a/Source/devtools/front_end/inspector.html b/Source/devtools/front_end/inspector.html
@@ -48,7 +48,6 @@
     <script type="text/javascript" src="common/TextUtils.js"></script>
     <script type="text/javascript" src="common/Progress.js"></script>
     <script type="text/javascript" src="common/ModuleExtensionInterfaces.js"></script>
-    <script type="text/javascript" src="common/Lock.js"></script>
     <script type="text/javascript" src="ui/CompletionDictionary.js"></script>
     <script type="text/javascript" src="ui/DOMExtension.js"></script>
     <script type="text/javascript" src="ui/treeoutline.js"></script>

diff --git a/Source/devtools/front_end/profiler/CanvasProfileView.js b/Source/devtools/front_end/profiler/CanvasProfileView.js
@@ -700,22 +700,22 @@ WebInspector.CanvasProfileType.prototype = {
     _runSingleFrameCapturing: function()
     {
         var frameId = this._selectedFrameId();
-        WebInspector.profilingLock.acquire();
+        this._target.profilingLock.acquire();
         CanvasAgent.captureFrame(frameId, this._didStartCapturingFrame.bind(this, frameId));
-        WebInspector.profilingLock.release();
+        this._target.profilingLock.release();
     },
 
     _startFrameCapturing: function()
     {
         var frameId = this._selectedFrameId();
-        WebInspector.profilingLock.acquire();
+        this._target.profilingLock.acquire();
         CanvasAgent.startCapturing(frameId, this._didStartCapturingFrame.bind(this, frameId));
     },
 
     _stopFrameCapturing: function()
     {
         if (!this._lastProfileHeader) {
-            WebInspector.profilingLock.release();
+            this._target.profilingLock.release();
             return;
         }
         var profileHeader = this._lastProfileHeader;
@@ -726,7 +726,7 @@ WebInspector.CanvasProfileType.prototype = {
             profileHeader._updateCapturingStatus();
         }
         CanvasAgent.stopCapturing(traceLogId, didStopCapturing);
-        WebInspector.profilingLock.release();
+        this._target.profilingLock.release();
     },
 
     /**

diff --git a/Source/devtools/front_end/profiler/ProfilesPanel.js b/Source/devtools/front_end/profiler/ProfilesPanel.js
@@ -238,9 +238,9 @@ WebInspector.ProfileType.prototype = {
     setProfileBeingRecorded: function(profile)
     {
         if (this._profileBeingRecorded && this._profileBeingRecorded.target())
-            WebInspector.profilingLock.release();
+            this._profileBeingRecorded.target().profilingLock.release();
         if (profile && profile.target())
-            WebInspector.profilingLock.acquire();
+            profile.target().profilingLock.acquire();
         this._profileBeingRecorded = profile;
     },
 
@@ -440,6 +440,7 @@ WebInspector.ProfileHeader.prototype = {
  * @constructor
  * @implements {WebInspector.Searchable}
  * @implements {WebInspector.ProfileType.DataDisplayDelegate}
+ * @implements {WebInspector.TargetManager.Observer}
  * @extends {WebInspector.PanelWithSidebarTree}
  */
 WebInspector.ProfilesPanel = function()
@@ -500,10 +501,26 @@ WebInspector.ProfilesPanel = function()
     this.element.addEventListener("contextmenu", this._handleContextMenuEvent.bind(this), true);
     this._registerShortcuts();
 
-    WebInspector.profilingLock.addEventListener(WebInspector.Lock.Events.StateChanged, this._onProfilingStateChanged, this);
+    WebInspector.targetManager.observeTargets(this);
 }
 
 WebInspector.ProfilesPanel.prototype = {
+    /**
+     * @param {!WebInspector.Target} target
+     */
+    targetAdded: function(target)
+    {
+        target.profilingLock.addEventListener(WebInspector.Lock.Events.StateChanged, this._onProfilingStateChanged, this);
+    },
+
+    /**
+     * @param {!WebInspector.Target} target
+     */
+    targetRemoved: function(target)
+    {
+        target.profilingLock.removeEventListener(WebInspector.Lock.Events.StateChanged, this._onProfilingStateChanged, this);
+    },
+
     /**
      * @return {!WebInspector.SearchableView}
      */
@@ -600,7 +617,7 @@ WebInspector.ProfilesPanel.prototype = {
     {
         if (WebInspector.experimentsSettings.disableAgentsWhenProfile.isEnabled())
             WebInspector.inspectorView.setCurrentPanelLocked(toggled);
-        var isAcquiredInSomeTarget = WebInspector.profilingLock.isAcquired();
+        var isAcquiredInSomeTarget = WebInspector.targetManager.targets().some(function(target) { return target.profilingLock.isAcquired(); });
         var enable = toggled || !isAcquiredInSomeTarget;
         this.recordButton.setEnabled(enable);
         this.recordButton.toggled = toggled;

diff --git a/Source/devtools/front_end/sdk/CSSStyleModel.js b/Source/devtools/front_end/sdk/CSSStyleModel.js
@@ -51,7 +51,7 @@ WebInspector.CSSStyleModel = function(target)
     this._styleSheetIdsForURL = new StringMap();
 
     if (WebInspector.experimentsSettings.disableAgentsWhenProfile.isEnabled())
-        WebInspector.profilingLock.addEventListener(WebInspector.Lock.Events.StateChanged, this._profilingStateChanged, this);
+        target.profilingLock.addEventListener(WebInspector.Lock.Events.StateChanged, this._profilingStateChanged, this);
 }
 
 WebInspector.CSSStyleModel.PseudoStatePropertyName = "pseudoState";
@@ -85,7 +85,7 @@ WebInspector.CSSStyleModel.MediaTypes = ["all", "braille", "embossed", "handheld
 WebInspector.CSSStyleModel.prototype = {
     _profilingStateChanged: function()
     {
-        if (WebInspector.profilingLock.isAcquired()) {
+        if (this.target().profilingLock.isAcquired()) {
             this._agent.disable();
             this._isEnabled = false;
             this._resetStyleSheets();

diff --git a/Source/devtools/front_end/sdk/DOMModel.js b/Source/devtools/front_end/sdk/DOMModel.js
@@ -949,7 +949,7 @@ WebInspector.DOMModel = function(target) {
     this._highlighter = this._defaultHighlighter;
 
     if (WebInspector.experimentsSettings.disableAgentsWhenProfile.isEnabled())
-        WebInspector.profilingLock.addEventListener(WebInspector.Lock.Events.StateChanged, this._profilingStateChanged, this);
+        target.profilingLock.addEventListener(WebInspector.Lock.Events.StateChanged, this._profilingStateChanged, this);
 
     this._agent.enable();
 }
@@ -970,7 +970,7 @@ WebInspector.DOMModel.Events = {
 WebInspector.DOMModel.prototype = {
     _profilingStateChanged: function()
     {
-        if (WebInspector.profilingLock.isAcquired())
+        if (this.target().profilingLock.isAcquired())
             this._agent.disable();
         else
             this._agent.enable();

diff --git a/Source/devtools/front_end/sdk/DebuggerModel.js b/Source/devtools/front_end/sdk/DebuggerModel.js
@@ -55,7 +55,7 @@ WebInspector.DebuggerModel = function(target)
     WebInspector.settings.pauseOnCaughtException.addChangeListener(this._pauseOnExceptionStateChanged, this);
 
     WebInspector.settings.enableAsyncStackTraces.addChangeListener(this._asyncStackTracesStateChanged, this);
-    WebInspector.profilingLock.addEventListener(WebInspector.Lock.Events.StateChanged, this._profilingStateChanged, this);
+    target.profilingLock.addEventListener(WebInspector.Lock.Events.StateChanged, this._profilingStateChanged, this);
 
     this.enableDebugger();
 
@@ -171,7 +171,7 @@ WebInspector.DebuggerModel.prototype = {
     _profilingStateChanged: function()
     {
         if (WebInspector.experimentsSettings.disableAgentsWhenProfile.isEnabled()) {
-            if (WebInspector.profilingLock.isAcquired())
+            if (this.target().profilingLock.isAcquired())
                 this.disableDebugger();
             else
                 this.enableDebugger();
@@ -182,7 +182,7 @@ WebInspector.DebuggerModel.prototype = {
     _asyncStackTracesStateChanged: function()
     {
         const maxAsyncStackChainDepth = 4;
-        var enabled = WebInspector.settings.enableAsyncStackTraces.get() && !WebInspector.profilingLock.isAcquired();
+        var enabled = WebInspector.settings.enableAsyncStackTraces.get() && !this.target().profilingLock.isAcquired();
         this._agent.setAsyncCallStackDepth(enabled ? maxAsyncStackChainDepth : 0);
     },
 

diff --git a/Source/devtools/front_end/sdk/Target.js b/Source/devtools/front_end/sdk/Target.js
@@ -32,6 +32,9 @@ WebInspector.Target = function(name, connection, callback)
     this.workerAgent().canInspectWorkers(this._initializeCapability.bind(this, WebInspector.Target.Capabilities.CanInspectWorkers, this._loadedWithCapabilities.bind(this, callback)));
     if (WebInspector.experimentsSettings.timelineOnTraceEvents.isEnabled())
         this.consoleAgent().setTracingBasedTimeline(true);
+
+    /** @type {!WebInspector.Lock} */
+    this.profilingLock = new WebInspector.Lock();
 }
 
 /**

diff --git a/Source/devtools/front_end/sdk/TimelineManager.js b/Source/devtools/front_end/sdk/TimelineManager.js
@@ -69,7 +69,7 @@ WebInspector.TimelineManager.prototype = {
     start: function(maxCallStackDepth, liveEvents, includeCounters, includeGPUEvents, callback)
     {
         this._enablementCount++;
-        WebInspector.profilingLock.acquire();
+        this.target().profilingLock.acquire();
         if (WebInspector.experimentsSettings.timelineJSCPUProfile.isEnabled() && maxCallStackDepth) {
             this._configureCpuProfilerSamplingInterval();
             this._jsProfilerStarted = true;
@@ -103,7 +103,7 @@ WebInspector.TimelineManager.prototype = {
         if (!this._enablementCount)
             this.target().timelineAgent().stop(callbackBarrier.createCallback(timelineCallback));
 
-        callbackBarrier.callWhenDone(allDoneCallback);
+        callbackBarrier.callWhenDone(allDoneCallback.bind(this));
 
         /**
          * @param {?Protocol.Error} error
@@ -123,9 +123,12 @@ WebInspector.TimelineManager.prototype = {
             masterProfile = profile;
         }
 
+        /**
+         * @this {WebInspector.TimelineManager}
+         */
         function allDoneCallback()
         {
-            WebInspector.profilingLock.release();
+            this.target().profilingLock.release();
             callback(masterError, masterProfile);
         }
     },

diff --git a/Source/devtools/front_end/sdk/TracingModel.js b/Source/devtools/front_end/sdk/TracingModel.js
@@ -101,7 +101,7 @@ WebInspector.TracingModel.prototype = {
      */
     start: function(categoryFilter, options, callback)
     {
-        WebInspector.profilingLock.acquire();
+        this.target().profilingLock.acquire();
         this.reset();
         var bufferUsageReportingIntervalMs = 500;
         TracingAgent.start(categoryFilter, options, bufferUsageReportingIntervalMs, callback);
@@ -113,7 +113,7 @@ WebInspector.TracingModel.prototype = {
         if (!this._active)
             return;
         TracingAgent.end(this._onStop.bind(this));
-        WebInspector.profilingLock.release();
+        this.target().profilingLock.release();
     },
 
     /**

diff --git a/Source/devtools/front_end/sdk/WorkerTargetManager.js b/Source/devtools/front_end/sdk/WorkerTargetManager.js
@@ -13,14 +13,14 @@ WebInspector.WorkerTargetManager = function(mainTarget, targetManager)
     this._mainTarget = mainTarget;
     this._targetManager = targetManager;
     mainTarget.workerManager.addEventListener(WebInspector.WorkerManager.Events.WorkerAdded, this._onWorkerAdded, this);
-    WebInspector.profilingLock.addEventListener(WebInspector.Lock.Events.StateChanged, this._onProfilingStateChanged, this);
+    mainTarget.profilingLock.addEventListener(WebInspector.Lock.Events.StateChanged, this._onProfilingStateChanged, this);
     this._onProfilingStateChanged();
 }
 
 WebInspector.WorkerTargetManager.prototype = {
     _onProfilingStateChanged: function()
     {
-        var acquired = WebInspector.profilingLock.isAcquired();
+        var acquired = this._mainTarget.profilingLock.isAcquired();
         this._mainTarget.workerAgent().setAutoconnectToWorkers(!acquired);
     },