Reland "[dart:html] Throw exception if Window.open opens null window" and

"[dart:html] Move NullWindowException to implementation"

This is a revert of bd3e6fa1a3 and
2b250992f9.

This adds some small code change to avoid a null-assertion being emitted
that would lead to a browser SecurityError.

CoreLibraryReviewExempt: Reland.
Change-Id: Iab52bb728b14fd0b2378b8923b0e1ea8ea930b12
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/311922
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Srujan Gaddam <srujzs@google.com>

sdk/lib/html/dart2js/html_dart2js.dart

@@ -32160,11 +32160,9 @@ class Window extends EventTarget
    *   from MDN.
    */
   WindowBase open(String url, String name, [String? options]) {
-    if (options == null) {
-      return _DOMWindowCrossFrame._createSafe(_open2(url, name));
-    } else {
-      return _DOMWindowCrossFrame._createSafe(_open3(url, name, options));
-    }
+    final win =
+        options == null ? _open2(url, name) : _open3(url, name, options);
+    return _DOMWindowCrossFrame._createSafe(win);
   }
 
   // API level getter and setter for Location.
@@ -33777,6 +33775,13 @@ class Window extends EventTarget
       ? JS<num>('num', '#.scrollY', this).round()
       : document.documentElement!.scrollTop;
 }
+
+class NullWindowException implements Exception {
+  @override
+  String toString() {
+    return 'Attempting to use a null window opened in Window.open.';
+  }
+}
 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
@@ -40017,7 +40022,7 @@ EventTarget? _convertNativeToDart_EventTarget(e) {
     return e;
 }
 
-EventTarget? _convertDartToNative_EventTarget(e) {
+_convertDartToNative_EventTarget(e) {
   if (e is _DOMWindowCrossFrame) {
     return e._window;
   } else {
@@ -40116,7 +40121,19 @@ class _DOMWindowCrossFrame implements WindowBase {
   // Private window.  Note, this is a window in another frame, so it
   // cannot be typed as "Window" as its prototype is not patched
   // properly.  Its fields and methods can only be accessed via JavaScript.
-  final _window;
+  final Object? __window;
+
+  Object get _window {
+    // Note that we use a local variable here to avoid a null-assertion. This is
+    // needed as [__window] can be a cross-origin iframe. dart2js emits
+    // null-assertions using a field access like .toString, and since accessing
+    // properties on a cross-origin iframe (with the exception of postMessage)
+    // is a SecurityError, the null-assertion could trigger a SecurityError.
+    // This avoids the need to type [__window] as dynamic.
+    final w = __window;
+    if (w == null) throw new NullWindowException();
+    return w;
+  }
 
   // Fields.
   HistoryBase get history =>
@@ -40153,7 +40170,7 @@ class _DOMWindowCrossFrame implements WindowBase {
   }
 
   // Implementation support.
-  _DOMWindowCrossFrame(this._window);
+  _DOMWindowCrossFrame(this.__window);
 
   static WindowBase _createSafe(w) {
     if (identical(w, window)) {

tests/lib/html/window_test.dart

@@ -11,4 +11,16 @@ main() {
     expect(window.scrollX, 0);
     expect(window.scrollY, 0);
   });
+  test('open', () {
+    final valid = window.open('', 'blank');
+    valid.closed;
+    // A blank page with no access to the original window (noopener) should
+    // result in null.
+    final invalid = window.open('', 'invalid', 'noopener=true');
+    try {
+      // Should result in an exception since the underlying window is null.
+      invalid.closed;
+      fail('Expected invalid.closed to throw.');
+    } on NullWindowException {}
+  });
 }

tests/lib_2/html/window_test.dart

@@ -13,4 +13,16 @@ main() {
     expect(window.scrollX, 0);
     expect(window.scrollY, 0);
   });
+  test('open', () {
+    final valid = window.open('', 'blank');
+    valid.closed;
+    // A blank page with no access to the original window (noopener) should
+    // result in null.
+    final invalid = window.open('', 'invalid', 'noopener=true');
+    try {
+      // Should result in an exception since the underlying window is null.
+      invalid.closed;
+      fail('Expected invalid.closed to throw.');
+    } on NullWindowException {}
+  });
 }

tools/dom/src/dart2js_Conversions.dart

@@ -33,7 +33,7 @@ EventTarget? _convertNativeToDart_EventTarget(e) {
     return e;
 }
 
-EventTarget? _convertDartToNative_EventTarget(e) {
+_convertDartToNative_EventTarget(e) {
   if (e is _DOMWindowCrossFrame) {
     return e._window;
   } else {

tools/dom/src/dart2js_DOMImplementation.dart

@@ -9,7 +9,19 @@ class _DOMWindowCrossFrame implements WindowBase {
   // Private window.  Note, this is a window in another frame, so it
   // cannot be typed as "Window" as its prototype is not patched
   // properly.  Its fields and methods can only be accessed via JavaScript.
-  final _window;
+  final Object? __window;
+
+  Object get _window {
+    // Note that we use a local variable here to avoid a null-assertion. This is
+    // needed as [__window] can be a cross-origin iframe. dart2js emits
+    // null-assertions using a field access like .toString, and since accessing
+    // properties on a cross-origin iframe (with the exception of postMessage)
+    // is a SecurityError, the null-assertion could trigger a SecurityError.
+    // This avoids the need to type [__window] as dynamic.
+    final w = __window;
+    if (w == null) throw new NullWindowException();
+    return w;
+  }
 
   // Fields.
   HistoryBase get history =>
@@ -46,7 +58,7 @@ class _DOMWindowCrossFrame implements WindowBase {
   }
 
   // Implementation support.
-  _DOMWindowCrossFrame(this._window);
+  _DOMWindowCrossFrame(this.__window);
 
   static WindowBase _createSafe(w) {
     if (identical(w, window)) {

tools/dom/templates/html/impl/impl_Window.darttemplate

@@ -56,11 +56,9 @@ $(CLASS_MODIFIERS)class $CLASSNAME$EXTENDS$IMPLEMENTS {
    *   from MDN.
    */
   WindowBase open(String url, String name, [String$NULLABLE options]) {
-    if (options == null) {
-      return _DOMWindowCrossFrame._createSafe(_open2(url, name));
-    } else {
-      return _DOMWindowCrossFrame._createSafe(_open3(url, name, options));
-    }
+    final win =
+        options == null ? _open2(url, name) : _open3(url, name, options);
+    return _DOMWindowCrossFrame._createSafe(win);
   }
 
   // API level getter and setter for Location.
@@ -254,3 +252,10 @@ $!MEMBERS
       JS<num>('num', '#.scrollY', this).round() :
       document.documentElement$NULLASSERT.scrollTop;
 }
+
+class NullWindowException implements Exception {
+  @override
+  String toString() {
+    return 'Attempting to use a null window opened in Window.open.';
+  }
+}

tools/dom/web_library_bindings.dart

@@ -9581,6 +9581,7 @@ final Map<String, Set<String>> dartTypeToNativeTypes = {
   'NoncedElement': {'NoncedElement'},
   'Notification': {'Notification'},
   'NotificationEvent': {'NotificationEvent'},
+  'NullWindowException': {'NullWindowException'},
   'Number': {'SVGNumber'},
   'NumberInputElement': {'NumberInputElement'},
   'NumberList': {'SVGNumberList'},