[DevTools] Ban getSelection, requestAnimationFrame, cancelAnimationFrame global functions.

[DevTools] Ban getSelection, requestAnimationFrame, cancelAnimationFrame global functions.

BUG=none

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

[dgozman, 2014-11-14 17:55:11]

dgozman@chromium.org changed reviewers:
+ apavlov@chromium.org, pfeldman@chromium.org

[dgozman, 2014-11-14 17:55:12]

Take a look please.

[apavlov, 2014-11-15 09:57:18]

https://codereview.chromium.org/727823002/diff/1/Source/devtools/front_end/pr...
File Source/devtools/front_end/profiler/CPUProfileFlameChart.js (right):

https://codereview.chromium.org/727823002/diff/1/Source/devtools/front_end/pr...
Source/devtools/front_end/profiler/CPUProfileFlameChart.js:660:
this._updateTimerId =
this.element.window().requestAnimationFrame(this.update.bind(this));
Can we define this as a shortcut on WI.View, too? this.window() is a tad shorter
than this.element.window()

[dgozman, 2014-11-15 11:46:25]

https://codereview.chromium.org/727823002/diff/1/Source/devtools/front_end/pr...
File Source/devtools/front_end/profiler/CPUProfileFlameChart.js (right):

https://codereview.chromium.org/727823002/diff/1/Source/devtools/front_end/pr...
Source/devtools/front_end/profiler/CPUProfileFlameChart.js:660:
this._updateTimerId =
this.element.window().requestAnimationFrame(this.update.bind(this));
On 2014/11/15 09:57:18, apavlov wrote:
> Can we define this as a shortcut on WI.View, too? this.window() is a tad
shorter
> than this.element.window()

The less thoughtful the shortcut, the easier it would be to make a mistake.
I think, |this.element.window()| is pretty short, and it makes author think
about whether |this.element| is already attached to the document.

[apavlov, 2014-11-17 12:33:13]

On 2014/11/15 11:46:25, dgozman wrote:
>
https://codereview.chromium.org/727823002/diff/1/Source/devtools/front_end/pr...
> File Source/devtools/front_end/profiler/CPUProfileFlameChart.js (right):
> 
>
https://codereview.chromium.org/727823002/diff/1/Source/devtools/front_end/pr...
> Source/devtools/front_end/profiler/CPUProfileFlameChart.js:660:
> this._updateTimerId =
> this.element.window().requestAnimationFrame(this.update.bind(this));
> On 2014/11/15 09:57:18, apavlov wrote:
> > Can we define this as a shortcut on WI.View, too? this.window() is a tad
> shorter
> > than this.element.window()
> 
> The less thoughtful the shortcut, the easier it would be to make a mistake.
> I think, |this.element.window()| is pretty short, and it makes author think
> about whether |this.element| is already attached to the document.

I agree.

[apavlov, 2014-11-17 12:45:53]

lgtm

[dgozman, 2014-11-17 12:48:29]

The CQ bit was checked by dgozman@chromium.org

[commit-bot: I haz the power, 2014-11-17 12:48:56]

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

[commit-bot: I haz the power, 2014-11-17 13:56:07]

Committed patchset #1 (id:1) as 185438

[alex clarke (OOO till 29th), 2015-04-20 12:54:02]

Out of curiosity, why was rAF banned?

[dgozman, 2015-04-20 13:00:19]

On 2015/04/20 12:54:02, alexclarke1 wrote:
> Out of curiosity, why was rAF banned?

That's because |window.requestAnimationFrame| is bound to the particular
|window| instance, and wrong rAF is fired when adapting Nodes to another
document.