DevTools: remove setLastEvaluationResult from the protocol.

DevTools: remove setLastEvaluationResult from the protocol.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=190999

[pfeldman, 2015-02-27 07:50:31]

pfeldman@chromium.org changed reviewers:
+ aandrey@chromium.org, dgozman@chromium.org, yurys@chromium.org

[dgozman, 2015-02-27 07:56:56]

https://codereview.chromium.org/967493002/diff/1/LayoutTests/inspector/consol...
File LayoutTests/inspector/console/set-last-eval-result.html (right):

https://codereview.chromium.org/967493002/diff/1/LayoutTests/inspector/consol...
LayoutTests/inspector/console/set-last-eval-result.html:17: remote = result;
This one is no longer used.

https://codereview.chromium.org/967493002/diff/1/LayoutTests/inspector/consol...
LayoutTests/inspector/console/set-last-eval-result.html:23:
InspectorTest.evaluateInConsole("$_", completeTest);
It seems this test no longer tests anything. Remove?

https://codereview.chromium.org/967493002/diff/1/Source/core/inspector/Inject...
File Source/core/inspector/InjectedScriptSource.js (right):

https://codereview.chromium.org/967493002/diff/1/Source/core/inspector/Inject...
Source/core/inspector/InjectedScriptSource.js:365: clearLastEvaluationResult:
function()
Is this one used? Remove it too?

[pfeldman, 2015-02-27 08:00:20]

https://codereview.chromium.org/967493002/diff/1/LayoutTests/inspector/consol...
File LayoutTests/inspector/console/set-last-eval-result.html (right):

https://codereview.chromium.org/967493002/diff/1/LayoutTests/inspector/consol...
LayoutTests/inspector/console/set-last-eval-result.html:17: remote = result;
On 2015/02/27 07:56:56, dgozman wrote:
> This one is no longer used.

Done.

https://codereview.chromium.org/967493002/diff/1/LayoutTests/inspector/consol...
LayoutTests/inspector/console/set-last-eval-result.html:23:
InspectorTest.evaluateInConsole("$_", completeTest);
What do you mean? This is the essence of this test.

https://codereview.chromium.org/967493002/diff/1/Source/core/inspector/Inject...
File Source/core/inspector/InjectedScriptSource.js (right):

https://codereview.chromium.org/967493002/diff/1/Source/core/inspector/Inject...
Source/core/inspector/InjectedScriptSource.js:365: clearLastEvaluationResult:
function()
This one is used in injectedscriptnative.

[aandrey, 2015-02-27 08:01:54]

https://codereview.chromium.org/967493002/diff/1/LayoutTests/inspector/consol...
File LayoutTests/inspector/console/set-last-eval-result.html (right):

https://codereview.chromium.org/967493002/diff/1/LayoutTests/inspector/consol...
LayoutTests/inspector/console/set-last-eval-result.html:23:
InspectorTest.evaluateInConsole("$_", completeTest);
On 2015/02/27 07:56:56, dgozman wrote:
> It seems this test no longer tests anything. Remove?

+1 to remove the test

https://codereview.chromium.org/967493002/diff/1/Source/core/inspector/Inject...
File Source/core/inspector/InjectedScriptSource.js (right):

https://codereview.chromium.org/967493002/diff/1/Source/core/inspector/Inject...
Source/core/inspector/InjectedScriptSource.js:365: clearLastEvaluationResult:
function()
On 2015/02/27 07:56:56, dgozman wrote:
> Is this one used? Remove it too?

This is called from native.

https://codereview.chromium.org/967493002/diff/1/Source/devtools/front_end/pr...
File Source/devtools/front_end/promises/PromisePane.js (right):

https://codereview.chromium.org/967493002/diff/1/Source/devtools/front_end/pr...
Source/devtools/front_end/promises/PromisePane.js:493: console.log(this);
This should be tested, maybe? e.g. now there will be a non-empty call stack, and
the anchor location will point to a VM script, right?

[dgozman, 2015-02-27 08:03:22]

https://codereview.chromium.org/967493002/diff/1/LayoutTests/inspector/consol...
File LayoutTests/inspector/console/set-last-eval-result.html (right):

https://codereview.chromium.org/967493002/diff/1/LayoutTests/inspector/consol...
LayoutTests/inspector/console/set-last-eval-result.html:23:
InspectorTest.evaluateInConsole("$_", completeTest);
On 2015/02/27 08:00:20, pfeldman wrote:
> What do you mean? This is the essence of this test.

This is now a subset of inspector/console/console-last-result.html.

[aandrey, 2015-02-27 08:03:47]

https://codereview.chromium.org/967493002/diff/1/LayoutTests/inspector/consol...
File LayoutTests/inspector/console/set-last-eval-result.html (right):

https://codereview.chromium.org/967493002/diff/1/LayoutTests/inspector/consol...
LayoutTests/inspector/console/set-last-eval-result.html:23:
InspectorTest.evaluateInConsole("$_", completeTest);
On 2015/02/27 08:00:20, pfeldman wrote:
> What do you mean? This is the essence of this test.

The "essence" is tested in inspector/console/console-last-result.html

[pfeldman, 2015-02-27 08:05:34]

https://codereview.chromium.org/967493002/diff/1/LayoutTests/inspector/consol...
File LayoutTests/inspector/console/set-last-eval-result.html (right):

https://codereview.chromium.org/967493002/diff/1/LayoutTests/inspector/consol...
LayoutTests/inspector/console/set-last-eval-result.html:23:
InspectorTest.evaluateInConsole("$_", completeTest);
On 2015/02/27 08:03:22, dgozman wrote:
> On 2015/02/27 08:00:20, pfeldman wrote:
> > What do you mean? This is the essence of this test.
> 
> This is now a subset of inspector/console/console-last-result.html.

Done.

https://codereview.chromium.org/967493002/diff/1/Source/devtools/front_end/pr...
File Source/devtools/front_end/promises/PromisePane.js (right):

https://codereview.chromium.org/967493002/diff/1/Source/devtools/front_end/pr...
Source/devtools/front_end/promises/PromisePane.js:493: console.log(this);
On 2015/02/27 08:01:54, aandrey wrote:
> This should be tested, maybe? e.g. now there will be a non-empty call stack,
and
> the anchor location will point to a VM script, right?

No, console.log does not dump any stacks.

[pfeldman, 2015-02-27 08:09:41]

PTAL

[aandrey, 2015-02-27 08:21:32]

lgtm

[dgozman, 2015-02-27 08:23:15]

lgtm

[pfeldman, 2015-02-27 08:39:26]

The CQ bit was checked by pfeldman@chromium.org

[commit-bot: I haz the power, 2015-02-27 08:39:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/967493002/20001

[commit-bot: I haz the power, 2015-02-27 08:45:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-02-27 08:45:13]

Try jobs failed on following builders:
  android_blink_compile_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/android_blink_compile_re...)

[yurys, 2015-02-27 08:47:22]

lgtm, thanks! I shouldn't have approved this method in the protocol.

[pfeldman, 2015-02-27 12:40:58]

The CQ bit was checked by pfeldman@chromium.org

[pfeldman, 2015-02-27 12:40:58]

The patchset sent to the CQ was uploaded after l-g-t-m from
dgozman@chromium.org, aandrey@chromium.org, yurys@chromium.org
Link to the patchset: https://codereview.chromium.org/967493002/#ps40001 (title:
"rebaselined")

[commit-bot: I haz the power, 2015-02-27 12:41:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/967493002/40001

[commit-bot: I haz the power, 2015-02-27 14:06:26]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=190999