Make arguments to window.{move,resize}{To,By} non-optional

Make arguments to window.{move,resize}{To,By} non-optional

When missing these arguments used to default to "the current value" so
calling the functions without arguments was a no-op. This is not
compatible with other implementations.

In Firefox (35), the arguments are not optional, so calling the functions
with fewer than two arguments throws a TypeError.

In IE (11), the arguments are optional with zero default value.

This patch aligns with Firefox's behavior, and also simplifies the
implementation by not dealing with missing arguments.

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

[Jens Widell, 2015-01-29 13:53:43]

jl@opera.com changed reviewers:
+ mkwst@chromium.org

[Jens Widell, 2015-01-29 13:53:44]

Any comments before I run this by blink-dev@?

https://codereview.chromium.org/886463010/diff/1/LayoutTests/fast/forms/resou...
File LayoutTests/fast/forms/resources/picker-common.js (right):

https://codereview.chromium.org/886463010/diff/1/LayoutTests/fast/forms/resou...
LayoutTests/fast/forms/resources/picker-common.js:23:
window.moveTo(window.screenX, window.screenY);
This one is funny. This call is required to make the test work, but it doesn't
seem to matter what arguments are used. "0, 0" works too, and "100, 100". (No
arguments no longer works, though.)

The call to Chrome::setWindowRect() (or something "behind" it) is what affects
the test's outcome somehow.

[Mike West, 2015-01-29 14:01:51]

On 2015/01/29 13:53:44, Jens Widell wrote:
> Any comments before I run this by blink-dev@?

Nope. LGTM, assuming the API OWNERS are OK with breaking the net.

>
https://codereview.chromium.org/886463010/diff/1/LayoutTests/fast/forms/resou...
> File LayoutTests/fast/forms/resources/picker-common.js (right):
> 
>
https://codereview.chromium.org/886463010/diff/1/LayoutTests/fast/forms/resou...
> LayoutTests/fast/forms/resources/picker-common.js:23:
> window.moveTo(window.screenX, window.screenY);
> This one is funny. This call is required to make the test work, but it doesn't
> seem to matter what arguments are used. "0, 0" works too, and "100, 100". (No
> arguments no longer works, though.)
> 
> The call to Chrome::setWindowRect() (or something "behind" it) is what affects
> the test's outcome somehow.

Strange. It doesn't look like anything in
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/r...
would be problematic. It might be worthwhile to take a look at the test runner
under content/shell to see if there's something we've forgotten to initialize.

[Jens Widell, 2015-06-11 12:26:47]

The CQ bit was checked by jl@opera.com to run a CQ dry run

[Jens Widell, 2015-06-11 12:26:47]

The patchset sent to the CQ was uploaded after l-g-t-m from mkwst@chromium.org
Link to the patchset: https://codereview.chromium.org/886463010/#ps60001 (title:
"rebased")

[commit-bot: I haz the power, 2015-06-11 12:27:13]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/886463010/60001

[commit-bot: I haz the power, 2015-06-11 13:24:58]

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

[commit-bot: I haz the power, 2015-06-11 13:24:59]

Dry run: This issue passed the CQ dry run.

[Jens Widell, 2015-06-12 09:00:43]

jl@opera.com changed reviewers:
+ philipj@opera.com

[Jens Widell, 2015-06-12 09:00:44]

According to the counter[1], these methods are hardly ever called with too few
arguments; it seems to be hovering between zero and 0.0001 %. I think it's safe
to just land this CL.

philipj@, are you happy with this, or should we revive the blink-dev thread
first? (It did get three LGTMs the last time around, before we decided to
count.)

1: https://www.chromestatus.com/metrics/feature/timeline/popularity/684

[philipj_slow, 2015-06-12 09:04:01]

Found the blink-dev thread again:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/-qvbisAzaPc/x1c_ndpj...

Given the usage I definitely think there's no reason to bother blink-dev with
this again.

[philipj_slow, 2015-06-12 09:15:16]

The CQ bit was checked by philipj@opera.com

[philipj_slow, 2015-06-12 09:15:16]

lgtm

[commit-bot: I haz the power, 2015-06-12 09:15:30]

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

[philipj_slow, 2015-06-12 09:16:04]

https://codereview.chromium.org/886463010/diff/60001/Source/core/frame/LocalD...
File Source/core/frame/LocalDOMWindow.cpp (left):

https://codereview.chromium.org/886463010/diff/60001/Source/core/frame/LocalD...
Source/core/frame/LocalDOMWindow.cpp:1234: UseCounter::count(document(),
UseCounter::WindowMoveResizeMissingArguments);
Oh wait, can you remove the now-unused counters?

[commit-bot: I haz the power, 2015-06-12 09:19:46]

Committed patchset #4 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=197024

[philipj_slow, 2015-06-12 09:23:13]

https://codereview.chromium.org/886463010/diff/60001/Source/core/frame/LocalD...
File Source/core/frame/LocalDOMWindow.cpp (left):

https://codereview.chromium.org/886463010/diff/60001/Source/core/frame/LocalD...
Source/core/frame/LocalDOMWindow.cpp:1234: UseCounter::count(document(),
UseCounter::WindowMoveResizeMissingArguments);
On 2015/06/12 09:16:04, philipj wrote:
> Oh wait, can you remove the now-unused counters?

Too late, but no need for a follow-up if you don't feel like it, I clean out
unused ones from time to time.