Make window's navigator / history properties non replaceable (issue 243203002)

[ch.d...@samsung.com]

Reviewers: arv, tkent,

Description:
Make window's navigator / history properties non replaceable

Make window's navigator / history properties non replaceable to match
the specification:
http://www.whatwg.org/specs/web-apps/current-work/multipage/browsers.html#window

The new behavior also matches the behavior of Firefox 28 and IE11.

R=a...@chromium.org, tk...@chromium.org
BUG=364512
TEST=fast/dom/Window/get-set-properties.html

Please review this at https://codereview.chromium.org/243203002/

SVN Base: svn://svn.chromium.org/blink/trunk

Affected files (+18, -24 lines):
M LayoutTests/fast/dom/Window/get-set-properties.html
M LayoutTests/fast/dom/Window/get-set-properties-expected.txt
M LayoutTests/fast/dom/Window/window-property-shadowing.html
M LayoutTests/fast/dom/Window/window-property-shadowing-expected.txt
M LayoutTests/fast/js/var-declarations-shadowing.html
M LayoutTests/fast/js/var-declarations-shadowing-expected.txt
M
LayoutTests/http/tests/history/cross-origin-replace-history-object-child-expected.txt
M
LayoutTests/http/tests/history/resources/cross-origin-replaces-history-object-child-iframe.html
M Source/core/frame/Window.idl


Index: LayoutTests/fast/dom/Window/get-set-properties-expected.txt
diff --git a/LayoutTests/fast/dom/Window/get-set-properties-expected.txt
b/LayoutTests/fast/dom/Window/get-set-properties-expected.txt
index
ae22c697f580b806a3f9ef60e384a02028d3e7b8..290197a7b4d5069d0ad57753ea3908de534df8fc
100644
--- a/LayoutTests/fast/dom/Window/get-set-properties-expected.txt
+++ b/LayoutTests/fast/dom/Window/get-set-properties-expected.txt
@@ -209,8 +209,6 @@ PASS: canGet('event') should be 'true' and is.
PASS: canSet('event') should be 'true' and is.
PASS: canGet('frames') should be 'true' and is.
PASS: canSet('frames') should be 'true' and is.
-PASS: canGet('history') should be 'true' and is.
-PASS: canSet('history') should be 'true' and is.
PASS: canGet('innerHeight') should be 'true' and is.
PASS: canSet('innerHeight') should be 'true' and is.
PASS: canGet('innerWidth') should be 'true' and is.
@@ -221,8 +219,6 @@ PASS: canGet('locationbar') should be 'true' and is.
PASS: canSet('locationbar') should be 'true' and is.
PASS: canGet('menubar') should be 'true' and is.
PASS: canSet('menubar') should be 'true' and is.
-PASS: canGet('navigator') should be 'true' and is.
-PASS: canSet('navigator') should be 'true' and is.
PASS: canGet('offscreenBuffering') should be 'true' and is.
PASS: canSet('offscreenBuffering') should be 'true' and is.
PASS: canGet('opener') should be 'true' and is.
@@ -262,6 +258,10 @@ PASS: canGet('closed') should be 'true' and is.
PASS: canSet('closed') should be 'false' and is.
PASS: canGet('document') should be 'true' and is.
PASS: canSet('document') should be 'false' and is.
+PASS: canGet('history') should be 'true' and is.
+PASS: canSet('history') should be 'false' and is.
+PASS: canGet('navigator') should be 'true' and is.
+PASS: canSet('navigator') should be 'false' and is.
PASS: canGet('pageXOffset') should be 'true' and is.
PASS: canSet('pageXOffset') should be 'false' and is.
PASS: canGet('pageYOffset') should be 'true' and is.
Index: LayoutTests/fast/dom/Window/get-set-properties.html
diff --git a/LayoutTests/fast/dom/Window/get-set-properties.html
b/LayoutTests/fast/dom/Window/get-set-properties.html
index
59fc5bf30b97dfb84ae615888a1224c70516f2ef..8e2b9cef067ecec219a739794baecce95335440f
100644
--- a/LayoutTests/fast/dom/Window/get-set-properties.html
+++ b/LayoutTests/fast/dom/Window/get-set-properties.html
@@ -185,13 +185,11 @@ var windowReadWriteProperties = [
"devicePixelRatio",
"event",
"frames",
- "history",
"innerHeight",
"innerWidth",
"length",
"locationbar",
"menubar",
- "navigator",
"offscreenBuffering",
"opener",
"outerHeight",
@@ -213,6 +211,8 @@ var windowReadWriteProperties = [
var windowReadOnlyProperties = [
"closed",
"document",
+ "history",
+ "navigator",
"pageXOffset",
"pageYOffset",
"window",
Index: LayoutTests/fast/dom/Window/window-property-shadowing-expected.txt
diff --git
a/LayoutTests/fast/dom/Window/window-property-shadowing-expected.txt
b/LayoutTests/fast/dom/Window/window-property-shadowing-expected.txt
index
74bf21577cdb524de4803013559c22ab65ac8668..b0d06b42b04b49372524fd1af1d01ecbd7e8c1a6
100644
--- a/LayoutTests/fast/dom/Window/window-property-shadowing-expected.txt
+++ b/LayoutTests/fast/dom/Window/window-property-shadowing-expected.txt
@@ -13,7 +13,6 @@ PASS: innerHeight successfully shadowed
PASS: innerWidth successfully shadowed
PASS: length successfully shadowed
PASS: name successfully shadowed
-PASS: navigator successfully shadowed
PASS: clientInformation successfully shadowed
PASS: screen successfully shadowed
PASS: offscreenBuffering successfully shadowed
@@ -28,7 +27,6 @@ PASS: screenTop successfully shadowed
PASS: scrollX successfully shadowed
PASS: scrollY successfully shadowed
PASS: self successfully shadowed
-PASS: history successfully shadowed
PASS: getSelection successfully shadowed
PASS: getComputedStyle successfully shadowed
PASS: getMatchedCSSRules successfully shadowed
Index: LayoutTests/fast/dom/Window/window-property-shadowing.html
diff --git a/LayoutTests/fast/dom/Window/window-property-shadowing.html
b/LayoutTests/fast/dom/Window/window-property-shadowing.html
index
e20576e914cc8f32bec761016dd1aef8a9459c6a..6153a85c72ce7f881004cb66c1e66843d839668d
100644
--- a/LayoutTests/fast/dom/Window/window-property-shadowing.html
+++ b/LayoutTests/fast/dom/Window/window-property-shadowing.html
@@ -45,8 +45,6 @@
log(length == 1 ? "PASS: length successfully shadowed" : "FAIL:
length was not shadowed");
var name = 1;
log(name == 1 ? "PASS: name successfully shadowed" : "FAIL: name
was not shadowed");
- var navigator = 1;
- log(navigator == 1 ? "PASS: navigator successfully
shadowed" : "FAIL: navigator was not shadowed");
var clientInformation = 1;
log(clientInformation == 1 ? "PASS: clientInformation successfully
shadowed" : "FAIL: clientInformation was not shadowed");
var screen = 1;
@@ -75,8 +73,6 @@
log(scrollY == 1 ? "PASS: scrollY successfully shadowed" : "FAIL:
scrollY was not shadowed");
var self = 1;
log(self == 1 ? "PASS: self successfully shadowed" : "FAIL: self
was not shadowed");
- var history = 1;
- log(history == 1 ? "PASS: history successfully shadowed" : "FAIL:
history was not shadowed");

// Window functions
var getSelection = 1;
Index: LayoutTests/fast/js/var-declarations-shadowing-expected.txt
diff --git a/LayoutTests/fast/js/var-declarations-shadowing-expected.txt
b/LayoutTests/fast/js/var-declarations-shadowing-expected.txt
index
37111524f09300832a12501bd051e9f272cc790e..352853e97f96d7a6c8657cd2ebb4a4e43a1b9df6
100644
--- a/LayoutTests/fast/js/var-declarations-shadowing-expected.txt
+++ b/LayoutTests/fast/js/var-declarations-shadowing-expected.txt
@@ -68,8 +68,8 @@ PASS: devicePixelRatio == marker should be true and is.
PASS: eval('devicePixelRatio == marker') should be true and is.
PASS: devicePixelRatio == marker should be true and is.
PASS: eval('devicePixelRatio == marker') should be true and is.
-PASS: navigator == marker should be true and is.
-PASS: eval('navigator == marker') should be true and is.
+PASS: navigator == marker should be false and is.
+PASS: eval('navigator == marker') should be false and is.
PASS: clientInformation == marker should be true and is.
PASS: eval('clientInformation == marker') should be true and is.
PASS: status == marker should be true and is.
@@ -80,8 +80,8 @@ PASS: defaultstatus == marker should be true and is.
PASS: eval('defaultstatus == marker') should be true and is.
PASS: screen == marker should be true and is.
PASS: eval('screen == marker') should be true and is.
-PASS: history == marker should be true and is.
-PASS: eval('history == marker') should be true and is.
+PASS: history == marker should be false and is.
+PASS: eval('history == marker') should be false and is.
-----
PASS: frameElement == marker should be false and is.
PASS: eval('frameElement == marker') should be false and is.
Index: LayoutTests/fast/js/var-declarations-shadowing.html
diff --git a/LayoutTests/fast/js/var-declarations-shadowing.html
b/LayoutTests/fast/js/var-declarations-shadowing.html
index
8440fd7e0437054074898f5ebc48dad27b97718f..d810d9d2abdfbf2f1b4d210082896810738ea9ed
100644
--- a/LayoutTests/fast/js/var-declarations-shadowing.html
+++ b/LayoutTests/fast/js/var-declarations-shadowing.html
@@ -220,8 +220,8 @@ shouldBe(eval('devicePixelRatio ==
marker'), "eval('devicePixelRatio == marker')
try {
eval("var navigator = marker");
} catch(e) { }
-shouldBe(navigator == marker, "navigator == marker", true);
-shouldBe(eval('navigator == marker'), "eval('navigator == marker')", true);
+shouldBe(navigator == marker, "navigator == marker", false);
+shouldBe(eval('navigator == marker'), "eval('navigator == marker')",
false);

try {
eval("var clientInformation = marker");
@@ -256,8 +256,8 @@ shouldBe(eval('screen == marker'), "eval('screen ==
marker')", true);
try {
eval("var history = marker");
} catch(e) { }
-shouldBe(history == marker, "history == marker", true);
-shouldBe(eval('history == marker'), "eval('history == marker')", true);
+shouldBe(history == marker, "history == marker", false);
+shouldBe(eval('history == marker'), "eval('history == marker')", false);

log("-----");

Index:
LayoutTests/http/tests/history/cross-origin-replace-history-object-child-expected.txt
diff --git
a/LayoutTests/http/tests/history/cross-origin-replace-history-object-child-expected.txt
b/LayoutTests/http/tests/history/cross-origin-replace-history-object-child-expected.txt
index
c65cf60fe2933619f1a20d61a22fa331537a9fe3..9776399f45194c29f2b5da86f1a702c0ae7071b7
100644
---
a/LayoutTests/http/tests/history/cross-origin-replace-history-object-child-expected.txt
+++
b/LayoutTests/http/tests/history/cross-origin-replace-history-object-child-expected.txt
@@ -1,5 +1,5 @@
ALERT: PASS: Access to window.frames[0].history threw an exception.
ALERT: About to shadow child window's history object: [object History]
-ALERT: Shadowed child window's history object:
+ALERT: Child window's history object should not be shadowed: [object
History]
ALERT: PASS: Access to window.frames[0].history threw an exception.

Index:
LayoutTests/http/tests/history/resources/cross-origin-replaces-history-object-child-iframe.html
diff --git
a/LayoutTests/http/tests/history/resources/cross-origin-replaces-history-object-child-iframe.html
b/LayoutTests/http/tests/history/resources/cross-origin-replaces-history-object-child-iframe.html
index
664d04061a15467ba95c08900a6c07976d1bb605..cdd1513bc34dcc099c19e82ca08acb83d6f798ca
100644
---
a/LayoutTests/http/tests/history/resources/cross-origin-replaces-history-object-child-iframe.html
+++
b/LayoutTests/http/tests/history/resources/cross-origin-replaces-history-object-child-iframe.html
@@ -16,7 +16,7 @@ function setHistoryLength()
{
alert("About to shadow child window's history object: " +
window.history);
window.history = "";
- alert("Shadowed child window's history object: " + window.history);
+ alert("Child window's history object should not be shadowed: " +
window.history);
parent.window.postMessage("done", "*");
}

Index: Source/core/frame/Window.idl
diff --git a/Source/core/frame/Window.idl b/Source/core/frame/Window.idl
index
77c6d97667138856a3c51af345e28d1c1f4b0f44..2f3ce58fdbe4999923341163ea8855430109ec1b
100644
--- a/Source/core/frame/Window.idl
+++ b/Source/core/frame/Window.idl
@@ -35,14 +35,14 @@
] interface Window : EventTarget {
// DOM Level 0
[Replaceable] readonly attribute Screen screen;
- [Replaceable] readonly attribute History history;
+ readonly attribute History history;
[Replaceable] readonly attribute BarProp locationbar;
[Replaceable] readonly attribute BarProp menubar;
[Replaceable] readonly attribute BarProp personalbar;
[Replaceable] readonly attribute BarProp scrollbars;
[Replaceable] readonly attribute BarProp statusbar;
[Replaceable] readonly attribute BarProp toolbar;
- [Replaceable, PerWorldBindings,
ActivityLogging=GetterForIsolatedWorlds] readonly attribute Navigator
navigator;
+ [PerWorldBindings, ActivityLogging=GetterForIsolatedWorlds] readonly
attribute Navigator navigator;
[Replaceable] readonly attribute Navigator clientInformation;
[DoNotCheckSecurity, Unforgeable, Replaceable, PerWorldBindings,
ActivityLogging=ForIsolatedWorlds, PutForwards=href] readonly attribute
Location location;
[Custom, MeasureAs=WindowEvent, NotEnumerable] attribute Event event;

[a...@chromium.org]

I'm not sure we want to change this. It will break code that uses these
names
for global variables.

Also, these are replaceable in Firefox.

https://codereview.chromium.org/243203002/

[ch.d...@samsung.com]

On 2014/04/18 18:44:01, arv wrote:
> I'm not sure we want to change this. It will break code that uses these
> names
> for global variables.

> Also, these are replaceable in Firefox.

Based on my testing, they are not replaceable in Firefox 28. I used this to
verify:
http://jsfiddle.net/QeL6a/1/

https://codereview.chromium.org/243203002/

[ch.d...@samsung.com]

I don't know how likely it is that real sites are using "navigator"
or "history"
as variable names if this fails on Firefox and IE already.

https://codereview.chromium.org/243203002/

[Erik Arvidsson]

I take it back. I was testing in Chrome... damn all these browsers that look identical.

LGTM

--
erik

[a...@chromium.org]

LGTM

https://codereview.chromium.org/243203002/

[tk...@chromium.org]

lgtm



https://codereview.chromium.org/243203002/

[commi...@chromium.org]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ch.d...@samsung.com/243203002/20001


https://codereview.chromium.org/243203002/

[commi...@chromium.org]

Change committed as 172006

https://codereview.chromium.org/243203002/