[webkit-reviews] review granted: [Bug 29616] Web Inspector: completions are always evaluated against window (discarding call frames) : [Attachment 39889] patch

[bugzilla-daemon at webkit.org]

Timothy Hatcher <timothy at hatcher.name> has granted Pavel Feldman
<pfeldman at chromium.org>'s request for review:
Bug 29616: Web Inspector: completions are always evaluated against window
(discarding call frames)
https://bugs.webkit.org/show_bug.cgi?id=29616

Attachment 39889: patch
https://bugs.webkit.org/attachment.cgi?id=39889&action=review

------- Additional Comments from Timothy Hatcher <timothy at hatcher.name>

> +	   var callFrameId = WebInspector.panels.scripts &&
WebInspector.panels.scripts.paused ?
WebInspector.panels.scripts.selectedCallFrameId() : null;

Thats a bit long. Maybe just have it be:

if (WebInspector.panels.scripts && WebInspector.panels.scripts.paused)
    var callFrameId = WebInspector.panels.scripts.selectedCallFrameId();

And it will be undefined by default for the normal case.

> +	   InjectedScriptAccess.getCompletions(expressionString,
includeInspectorCommandLineAPI, callFrameId, reportCompletions);
>      },
>  
>      _reportCompletions: function(bestMatchOnly, completionsReadyCallback,
dotNotation, bracketNotation, prefix, result, isException) {
> diff --git a/WebCore/inspector/front-end/InjectedScript.js
b/WebCore/inspector/front-end/InjectedScript.js
> index 4d96f68..726c7cc 100644
> --- a/WebCore/inspector/front-end/InjectedScript.js
> +++ b/WebCore/inspector/front-end/InjectedScript.js
> @@ -499,11 +499,20 @@ InjectedScript.setPropertyValue = function(objectProxy,
propertyName, expression
>  }
>  
>  
> -InjectedScript.getCompletions = function(expression,
includeInspectorCommandLineAPI)
> +InjectedScript.getCompletions = function(expression,
includeInspectorCommandLineAPI, callFrameId)
>  {
>      var props = {};
>      try {
> -	   var expressionResult =
InjectedScript._evaluateOn(InjectedScript._window().eval,
InjectedScript._window(), expression);
> +	   var expressionResult;
> +	   // Evaluate on call frame if call frame id is available.
> +	   if (typeof callFrameId === "number") {
> +	       var callFrame = InjectedScript._callFrameForId(callFrameId);
> +	       if (!callFrame)
> +		   return props;
> +	       expressionResult =
InjectedScript._evaluateOn(callFrame.evaluate, callFrame, expression);
> +	   } else {
> +	       expressionResult =
InjectedScript._evaluateOn(InjectedScript._window().eval,
InjectedScript._window(), expression);
> +	   }
>	   for (var prop in expressionResult)
>	       props[prop] = true;
>	   if (includeInspectorCommandLineAPI)
> diff --git a/WebCore/inspector/front-end/ScriptsPanel.js
b/WebCore/inspector/front-end/ScriptsPanel.js
> index 04f27bb..ae918d1 100644
> --- a/WebCore/inspector/front-end/ScriptsPanel.js
> +++ b/WebCore/inspector/front-end/ScriptsPanel.js
> @@ -351,6 +351,14 @@ WebInspector.ScriptsPanel.prototype = {
>	       sourceFrame.removeBreakpoint(breakpoint);
>      },
>  
> +    selectedCallFrameId: function()
> +    {
> +	   var selectedCallFrame =
this.sidebarPanes.callstack.selectedCallFrame;
> +	   if (!selectedCallFrame)
> +	       return null;
> +	   return selectedCallFrame.id;
> +    },
> +
>      evaluateInSelectedCallFrame: function(code, updateInterface, callback)
>      {
>	   var selectedCallFrame =
this.sidebarPanes.callstack.selectedCallFrame;