Web Inspector: inspected object wrapper should be released by InjectedScript when...

https://bugs.webkit.org/show_bug.cgi?id=77972

When object popover is shown the object under cursor is resolved and its
wrapper is put into 'popover' object wrapper group. The group is discarded
when the popover closes.

Reviewed by Pavel Feldman.

* bindings/js/ScriptProfiler.cpp:
(WebCore::ScriptProfiler::objectByHeapObjectId):
* bindings/js/ScriptProfiler.h:
(WebCore):
(ScriptProfiler):
* bindings/v8/ScriptProfiler.cpp:
(WebCore::ScriptProfiler::objectByHeapObjectId):
(WebCore):
* bindings/v8/ScriptProfiler.h:
(WebCore):
(ScriptProfiler):
* inspector/Inspector.json:
* inspector/InspectorProfilerAgent.cpp:
(WebCore::InspectorProfilerAgent::getObjectByHeapObjectId):
* inspector/InspectorProfilerAgent.h:
(InspectorProfilerAgent):
* inspector/front-end/DetailedHeapshotGridNodes.js:
(WebInspector.HeapSnapshotGenericObjectNode.prototype.queryObjectContent):
* inspector/front-end/DetailedHeapshotView.js:
(WebInspector.DetailedHeapshotView.prototype._resolveObjectForPopover):
* inspector/front-end/JavaScriptSourceFrame.js:
(WebInspector.JavaScriptSourceFrame):
(WebInspector.JavaScriptSourceFrame.prototype._resolveObjectForPopover):
(WebInspector.JavaScriptSourceFrame.prototype._onHidePopover):
* inspector/front-end/ObjectPopoverHelper.js:
(WebInspector.ObjectPopoverHelper):
(WebInspector.ObjectPopoverHelper.prototype._showObjectPopover):
(WebInspector.ObjectPopoverHelper.prototype._onHideObjectPopover):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@107089 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 21cddb8..53c0ccf 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,43 @@
+2012-02-07  Yury Semikhatsky  <yurys@chromium.org>
+
+        Web Inspector: inspected object wrapper should be released by InjectedScript when popover closes
+        https://bugs.webkit.org/show_bug.cgi?id=77972
+
+        When object popover is shown the object under cursor is resolved and its
+        wrapper is put into 'popover' object wrapper group. The group is discarded
+        when the popover closes.
+
+        Reviewed by Pavel Feldman.
+
+        * bindings/js/ScriptProfiler.cpp:
+        (WebCore::ScriptProfiler::objectByHeapObjectId):
+        * bindings/js/ScriptProfiler.h:
+        (WebCore):
+        (ScriptProfiler):
+        * bindings/v8/ScriptProfiler.cpp:
+        (WebCore::ScriptProfiler::objectByHeapObjectId):
+        (WebCore):
+        * bindings/v8/ScriptProfiler.h:
+        (WebCore):
+        (ScriptProfiler):
+        * inspector/Inspector.json:
+        * inspector/InspectorProfilerAgent.cpp:
+        (WebCore::InspectorProfilerAgent::getObjectByHeapObjectId):
+        * inspector/InspectorProfilerAgent.h:
+        (InspectorProfilerAgent):
+        * inspector/front-end/DetailedHeapshotGridNodes.js:
+        (WebInspector.HeapSnapshotGenericObjectNode.prototype.queryObjectContent):
+        * inspector/front-end/DetailedHeapshotView.js:
+        (WebInspector.DetailedHeapshotView.prototype._resolveObjectForPopover):
+        * inspector/front-end/JavaScriptSourceFrame.js:
+        (WebInspector.JavaScriptSourceFrame):
+        (WebInspector.JavaScriptSourceFrame.prototype._resolveObjectForPopover):
+        (WebInspector.JavaScriptSourceFrame.prototype._onHidePopover):
+        * inspector/front-end/ObjectPopoverHelper.js:
+        (WebInspector.ObjectPopoverHelper):
+        (WebInspector.ObjectPopoverHelper.prototype._showObjectPopover):
+        (WebInspector.ObjectPopoverHelper.prototype._onHideObjectPopover):
+
 2012-02-08  Mario Sanchez Prada  <msanchez@igalia.com>
 
         [Gtk] atk_text_get_text_at_offset() fails to provide the correct line for list items whose text wraps

diff --git a/Source/WebCore/bindings/js/ScriptProfiler.cpp b/Source/WebCore/bindings/js/ScriptProfiler.cpp
index 4c84fb9..a63fe4d 100644 (file)
--- a/Source/WebCore/bindings/js/ScriptProfiler.cpp
+++ b/Source/WebCore/bindings/js/ScriptProfiler.cpp
@@ -31,8 +31,8 @@
 #include "ScriptProfiler.h"
 
 #include "GCController.h"
-#include "InspectorValues.h"
 #include "JSDOMBinding.h"
+#include "ScriptObject.h"
 #include <profiler/Profiler.h>
 
 namespace WebCore {
@@ -42,9 +42,9 @@ void ScriptProfiler::collectGarbage()
     gcController().garbageCollectNow();
 }
 
-PassRefPtr<InspectorValue> ScriptProfiler::objectByHeapObjectId(unsigned, InjectedScriptManager*)
+ScriptObject ScriptProfiler::objectByHeapObjectId(unsigned)
 {
-    return InspectorValue::null();
+    return ScriptObject();
 }
 
 void ScriptProfiler::start(ScriptState* state, const String& title)

diff --git a/Source/WebCore/bindings/js/ScriptProfiler.h b/Source/WebCore/bindings/js/ScriptProfiler.h
index 2211971..a21eada 100644 (file)
--- a/Source/WebCore/bindings/js/ScriptProfiler.h
+++ b/Source/WebCore/bindings/js/ScriptProfiler.h
@@ -28,7 +28,6 @@
 #define ScriptProfiler_h
 
 #if ENABLE(JAVASCRIPT_DEBUGGER)
-#include "InspectorValues.h"
 #include "ScriptHeapSnapshot.h"
 #include "ScriptProfile.h"
 #include "ScriptState.h"
@@ -38,7 +37,7 @@
 namespace WebCore {
 
 class DOMWrapperVisitor;
-class InjectedScriptManager;
+class ScriptObject;
 
 class ScriptProfiler {
     WTF_MAKE_NONCOPYABLE(ScriptProfiler);
@@ -53,7 +52,7 @@ public:
     };
 
     static void collectGarbage();
-    static PassRefPtr<InspectorValue> objectByHeapObjectId(unsigned id, InjectedScriptManager*);
+    static ScriptObject objectByHeapObjectId(unsigned id);
     static void start(ScriptState* state, const String& title);
     static PassRefPtr<ScriptProfile> stop(ScriptState* state, const String& title);
     static PassRefPtr<ScriptHeapSnapshot> takeHeapSnapshot(const String&, HeapSnapshotProgress*) { return 0; }

diff --git a/Source/WebCore/bindings/v8/ScriptProfiler.cpp b/Source/WebCore/bindings/v8/ScriptProfiler.cpp
index 4007a8f..a7f65fa 100644 (file)
--- a/Source/WebCore/bindings/v8/ScriptProfiler.cpp
+++ b/Source/WebCore/bindings/v8/ScriptProfiler.cpp
@@ -32,9 +32,8 @@
 #include "ScriptProfiler.h"
 
 #include "DOMWrapperVisitor.h"
-#include "InjectedScript.h"
-#include "InspectorValues.h"
 #include "RetainedDOMInfo.h"
+#include "ScriptObject.h"
 #include "V8Binding.h"
 #include "V8DOMMap.h"
 #include "V8Node.h"
@@ -64,7 +63,7 @@ void ScriptProfiler::collectGarbage()
     v8::V8::LowMemoryNotification();
 }
 
-PassRefPtr<InspectorValue> ScriptProfiler::objectByHeapObjectId(unsigned id, InjectedScriptManager* injectedScriptManager)
+ScriptObject ScriptProfiler::objectByHeapObjectId(unsigned id)
 {
     // As ids are unique, it doesn't matter which HeapSnapshot owns HeapGraphNode.
     // We need to find first HeapSnapshot containing a node with the specified id.
@@ -76,20 +75,16 @@ PassRefPtr<InspectorValue> ScriptProfiler::objectByHeapObjectId(unsigned id, Inj
             break;
     }
     if (!node)
-        return InspectorValue::null();
+        return ScriptObject();
 
     v8::HandleScope scope;
     v8::Handle<v8::Value> value = node->GetHeapValue();
     if (!value->IsObject())
-        return InspectorValue::null();
-
-    v8::Handle<v8::Object> object(value.As<v8::Object>());
-    v8::Local<v8::Context> creationContext = object->CreationContext();
-    v8::Context::Scope creationScope(creationContext);
-    ScriptState* scriptState = ScriptState::forContext(creationContext);
-    InjectedScript injectedScript = injectedScriptManager->injectedScriptFor(scriptState);
-    return !injectedScript.hasNoValue() ?
-            RefPtr<InspectorValue>(injectedScript.wrapObject(value, "")).release() : InspectorValue::null();
+        return ScriptObject();
+
+    v8::Handle<v8::Object> object = value.As<v8::Object>();
+    ScriptState* scriptState = ScriptState::forContext(object->CreationContext());
+    return ScriptObject(scriptState, object);
 }
 
 namespace {

diff --git a/Source/WebCore/bindings/v8/ScriptProfiler.h b/Source/WebCore/bindings/v8/ScriptProfiler.h
index 9f02f4b..8e8a161 100644 (file)
--- a/Source/WebCore/bindings/v8/ScriptProfiler.h
+++ b/Source/WebCore/bindings/v8/ScriptProfiler.h
@@ -31,7 +31,6 @@
 #ifndef ScriptProfiler_h
 #define ScriptProfiler_h
 
-#include "InspectorValues.h"
 #include "PlatformString.h"
 #include "ScriptHeapSnapshot.h"
 #include "ScriptProfile.h"
@@ -42,7 +41,7 @@
 namespace WebCore {
 
 class DOMWrapperVisitor;
-class InjectedScriptManager;
+class ScriptObject;
 
 class ScriptProfiler {
     WTF_MAKE_NONCOPYABLE(ScriptProfiler);
@@ -57,7 +56,7 @@ public:
     };
 
     static void collectGarbage();
-    static PassRefPtr<InspectorValue> objectByHeapObjectId(unsigned id, InjectedScriptManager*);
+    static ScriptObject objectByHeapObjectId(unsigned id);
     static void start(ScriptState* state, const String& title);
     static PassRefPtr<ScriptProfile> stop(ScriptState* state, const String& title);
     static PassRefPtr<ScriptHeapSnapshot> takeHeapSnapshot(const String& title, HeapSnapshotProgress*);

diff --git a/Source/WebCore/inspector/Inspector.json b/Source/WebCore/inspector/Inspector.json
index e204d9c..92639d5 100644 (file)
--- a/Source/WebCore/inspector/Inspector.json
+++ b/Source/WebCore/inspector/Inspector.json
@@ -2410,7 +2410,8 @@
             {
                 "name": "getObjectByHeapObjectId",
                 "parameters": [
-                    { "name": "objectId", "type": "integer" }
+                    { "name": "objectId", "type": "integer" },
+                    { "name": "objectGroup", "type": "string", "optional": true, "description": "Symbolic group name that can be used to release multiple objects." }
                 ],
                 "returns": [
                     { "name": "result", "$ref": "Runtime.RemoteObject", "description": "Evaluation result." }

diff --git a/Source/WebCore/inspector/InspectorProfilerAgent.cpp b/Source/WebCore/inspector/InspectorProfilerAgent.cpp
index 6adaa34..8774276 100644 (file)
--- a/Source/WebCore/inspector/InspectorProfilerAgent.cpp
+++ b/Source/WebCore/inspector/InspectorProfilerAgent.cpp
@@ -33,6 +33,7 @@
 #if ENABLE(JAVASCRIPT_DEBUGGER) && ENABLE(INSPECTOR)
 
 #include "Console.h"
+#include "InjectedScript.h"
 #include "InspectorConsoleAgent.h"
 #include "InspectorFrontend.h"
 #include "InspectorState.h"
@@ -42,6 +43,7 @@
 #include "Page.h"
 #include "PageScriptDebugServer.h"
 #include "ScriptHeapSnapshot.h"
+#include "ScriptObject.h"
 #include "ScriptProfile.h"
 #include "ScriptProfiler.h"
 #include <wtf/OwnPtr.h>
@@ -389,13 +391,21 @@ void InspectorProfilerAgent::toggleRecordButton(bool isProfiling)
         m_frontend->setRecordingProfile(isProfiling);
 }
 
-void InspectorProfilerAgent::getObjectByHeapObjectId(ErrorString* error, int id, RefPtr<InspectorObject>& result)
+void InspectorProfilerAgent::getObjectByHeapObjectId(ErrorString* error, int id, const String* objectGroup, RefPtr<InspectorObject>& result)
 {
-    RefPtr<InspectorValue> heapObject = ScriptProfiler::objectByHeapObjectId(id, m_injectedScriptManager);
-    if (!heapObject->isNull())
-        heapObject->asObject(&result);
-    else
+    ScriptObject heapObject = ScriptProfiler::objectByHeapObjectId(id);
+    if (heapObject.hasNoValue()) {
         *error = "Object is not available.";
+        return;
+    }
+    InjectedScript injectedScript = m_injectedScriptManager->injectedScriptFor(heapObject.scriptState());
+    if (injectedScript.hasNoValue()) {
+        *error = "Object is not available. Inspected context is gone.";
+        return;
+    }
+    result = injectedScript.wrapObject(heapObject, objectGroup ? *objectGroup : "");
+    if (!result)
+        *error = "Failed to wrap object.";
 }
 
 } // namespace WebCore

diff --git a/Source/WebCore/inspector/InspectorProfilerAgent.h b/Source/WebCore/inspector/InspectorProfilerAgent.h
index 63f250c..a5576d8 100644 (file)
--- a/Source/WebCore/inspector/InspectorProfilerAgent.h
+++ b/Source/WebCore/inspector/InspectorProfilerAgent.h
@@ -92,7 +92,7 @@ public:
     virtual void takeHeapSnapshot(ErrorString*);
     void toggleRecordButton(bool isProfiling);
 
-    virtual void getObjectByHeapObjectId(ErrorString*, int id, RefPtr<InspectorObject>& result);
+    virtual void getObjectByHeapObjectId(ErrorString*, int id, const String* objectGroup, RefPtr<InspectorObject>& result);
 
 private:
     typedef HashMap<unsigned int, RefPtr<ScriptProfile> > ProfilesMap;

diff --git a/Source/WebCore/inspector/front-end/DetailedHeapshotGridNodes.js b/Source/WebCore/inspector/front-end/DetailedHeapshotGridNodes.js
index 9fc1ece..e127eb1 100644 (file)
--- a/Source/WebCore/inspector/front-end/DetailedHeapshotGridNodes.js
+++ b/Source/WebCore/inspector/front-end/DetailedHeapshotGridNodes.js
@@ -308,7 +308,7 @@ WebInspector.HeapSnapshotGenericObjectNode.prototype = {
         return this._enhanceData ? this._enhanceData(data) : data;
     },
 
-    queryObjectContent: function(callback)
+    queryObjectContent: function(callback, objectGroupName)
     {
         if (this._type === "string")
             callback(WebInspector.RemoteObject.fromPrimitiveValue(this._name));
@@ -320,7 +320,7 @@ WebInspector.HeapSnapshotGenericObjectNode.prototype = {
                 else
                     callback(WebInspector.RemoteObject.fromPrimitiveValue(WebInspector.UIString("Not available")));
             }
-            ProfilerAgent.getObjectByHeapObjectId(this.snapshotNodeId, formatResult);
+            ProfilerAgent.getObjectByHeapObjectId(this.snapshotNodeId, objectGroupName, formatResult);
         }
     },
 

diff --git a/Source/WebCore/inspector/front-end/DetailedHeapshotView.js b/Source/WebCore/inspector/front-end/DetailedHeapshotView.js
index 03bca46..375ffd6 100644 (file)
--- a/Source/WebCore/inspector/front-end/DetailedHeapshotView.js
+++ b/Source/WebCore/inspector/front-end/DetailedHeapshotView.js
@@ -502,7 +502,7 @@ WebInspector.DetailedHeapshotView = function(parent, profile)
     this.helpButton = new WebInspector.StatusBarButton("", "heapshot-help-status-bar-item status-bar-item");
     this.helpButton.addEventListener("click", this._helpClicked.bind(this), false);
 
-    this._popoverHelper = new WebInspector.ObjectPopoverHelper(this.element, this._getHoverAnchor.bind(this), this._showObjectPopover.bind(this), null, true);
+    this._popoverHelper = new WebInspector.ObjectPopoverHelper(this.element, this._getHoverAnchor.bind(this), this._resolveObjectForPopover.bind(this), null, true);
 
     this._loadProfile(this._profileUid, profileCallback.bind(this));
 
@@ -935,9 +935,9 @@ WebInspector.DetailedHeapshotView.prototype = {
         this.refreshShowPercents();
     },
 
-    _showObjectPopover: function(element, showCallback)
+    _resolveObjectForPopover: function(element, showCallback, objectGroupName)
     {
-        element.node.queryObjectContent(showCallback);
+        element.node.queryObjectContent(showCallback, objectGroupName);
     },
 
     _helpClicked: function(event)

diff --git a/Source/WebCore/inspector/front-end/JavaScriptSourceFrame.js b/Source/WebCore/inspector/front-end/JavaScriptSourceFrame.js
index f2723ae..32d7aff 100644 (file)
--- a/Source/WebCore/inspector/front-end/JavaScriptSourceFrame.js
+++ b/Source/WebCore/inspector/front-end/JavaScriptSourceFrame.js
@@ -40,13 +40,12 @@ WebInspector.JavaScriptSourceFrame = function(scriptsPanel, model, uiSourceCode)
     this._scriptsPanel = scriptsPanel;
     this._model = model;
     this._uiSourceCode = uiSourceCode;
-    this._popoverObjectGroup = "popover";
     this._breakpoints = {};
 
     WebInspector.SourceFrame.call(this, uiSourceCode.url);
 
     this._popoverHelper = new WebInspector.ObjectPopoverHelper(this.textViewer.element,
-            this._getPopoverAnchor.bind(this), this._onShowPopover.bind(this), this._onHidePopover.bind(this), true);
+            this._getPopoverAnchor.bind(this), this._resolveObjectForPopover.bind(this), this._onHidePopover.bind(this), true);
 
     this.textViewer.element.addEventListener("mousedown", this._onMouseDown.bind(this), true);
     this._uiSourceCode.addEventListener(WebInspector.UISourceCode.Events.ContentChanged, this._onContentChanged, this);
@@ -279,7 +278,7 @@ WebInspector.JavaScriptSourceFrame.prototype = {
         return element;
     },
 
-    _onShowPopover: function(element, showCallback)
+    _resolveObjectForPopover: function(element, showCallback, objectGroupName)
     {
         if (!this.readOnly) {
             this._popoverHelper.hidePopover();
@@ -300,7 +299,7 @@ WebInspector.JavaScriptSourceFrame.prototype = {
         }
 
         var selectedCallFrame = this._model.selectedCallFrame;
-        selectedCallFrame.evaluate(this._highlightElement.textContent, this._popoverObjectGroup, false, false, showObjectPopover.bind(this));
+        selectedCallFrame.evaluate(this._highlightElement.textContent, objectGroupName, false, false, showObjectPopover.bind(this));
     },
 
     _onHidePopover: function()
@@ -322,7 +321,6 @@ WebInspector.JavaScriptSourceFrame.prototype = {
             parentElement.removeChild(highlightElement);
         }
         delete this._highlightElement;
-        RuntimeAgent.releaseObjectGroup(this._popoverObjectGroup);
     },
 
     _highlightExpression: function(element)

diff --git a/Source/WebCore/inspector/front-end/ObjectPopoverHelper.js b/Source/WebCore/inspector/front-end/ObjectPopoverHelper.js
index cad81e4..3a560db 100644 (file)
--- a/Source/WebCore/inspector/front-end/ObjectPopoverHelper.js
+++ b/Source/WebCore/inspector/front-end/ObjectPopoverHelper.js
@@ -37,6 +37,7 @@ WebInspector.ObjectPopoverHelper = function(panelElement, getAnchor, queryObject
     WebInspector.PopoverHelper.call(this, panelElement, getAnchor, this._showObjectPopover.bind(this), this._onHideObjectPopover.bind(this), disableOnClick);
     this._queryObject = queryObject;
     this._onHideCallback = onHide;
+    this._popoverObjectGroup = "popover";
     panelElement.addEventListener("scroll", this.hidePopover.bind(this), true);
 };
 
@@ -110,7 +111,7 @@ WebInspector.ObjectPopoverHelper.prototype = {
                 popover.show(popoverContentElement, element, popoverWidth, popoverHeight);
             }
         }
-        this._queryObject(element, showObjectPopover.bind(this));
+        this._queryObject(element, showObjectPopover.bind(this), this._popoverObjectGroup);
     },
 
     _onHideObjectPopover: function()
@@ -121,6 +122,7 @@ WebInspector.ObjectPopoverHelper.prototype = {
         }
         if (this._onHideCallback)
             this._onHideCallback();
+        RuntimeAgent.releaseObjectGroup(this._popoverObjectGroup);
     },
 
     _updateHTMLId: function(properties, rootTreeElementConstructor, rootPropertyComparer)