Fix 'noopener' targeting and return value.

Fix 'noopener' targeting and return value.

Boris noted a few cases in which Chrome wasn't following the spec for
'noopener'. This patch addresses two of them by ensuring that the call
to 'window.open' return 'null' (rather than 'undefined'), and that we
don't reuse existing windows when processing a 'noopener' request.

BUG=651578, 651579
Committed: https://crrev.com/2fde9de3e9ffe7794844ddf9ea3640c5b418b6e5
Cr-Commit-Position: refs/heads/master@{#423493}

[Mike West, 2016-09-30 09:44:26]

The CQ bit was checked by mkwst@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-30 09:44:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Mike West, 2016-09-30 09:44:56]

mkwst@chromium.org changed reviewers:
+ jochen@chromium.org

[Mike West, 2016-09-30 09:44:57]

WDYT, Jochen? I think this is a sane usage of `windowCount`, but, you know..
ugly.

[jochen (gone - plz use gerrit), 2016-09-30 09:47:32]

I'm not sure whether noopener should return a new window object, or the existing
one and just remove the opener? I commented on the bug...

[commit-bot: I haz the power, 2016-09-30 10:05:55]

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

[commit-bot: I haz the power, 2016-09-30 10:05:56]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[Mike West, 2016-09-30 10:57:37]

The CQ bit was checked by mkwst@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-30 10:57:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jochen (gone - plz use gerrit), 2016-09-30 11:14:28]

wanna split out the return null fix while we discuss the other?

[commit-bot: I haz the power, 2016-09-30 12:02:59]

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

[commit-bot: I haz the power, 2016-09-30 12:03:00]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Mike West, 2016-09-30 12:36:35]

On 2016/09/30 at 11:14:28, jochen wrote:
> wanna split out the return null fix while we discuss the other?

Eh. I think we'll resolve it pretty quickly.

[Mike West, 2016-10-05 12:23:48]

The CQ bit was checked by mkwst@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-05 12:24:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Mike West, 2016-10-05 12:24:44]

Based on the discussion at https://github.com/whatwg/html/pull/1842
(https://github.com/whatwg/html/pull/1842#issuecomment-251273247 in particular),
I think this reflects the behavior we're converging on.

[jochen (gone - plz use gerrit), 2016-10-05 12:26:40]

lgtm

[commit-bot: I haz the power, 2016-10-05 13:59:21]

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

[commit-bot: I haz the power, 2016-10-05 13:59:22]

Dry run: This issue passed the CQ dry run.

[Mike West, 2016-10-06 06:38:30]

The CQ bit was checked by mkwst@chromium.org

[commit-bot: I haz the power, 2016-10-06 06:38:47]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-06 06:50:59]

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

[commit-bot: I haz the power, 2016-10-06 06:51:01]

Failed to apply patch for
third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp:
While running git apply --index -3 -p1;
  error: patch failed:
third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp:221
  Falling back to three-way merge...
  Applied patch to
'third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp' with
conflicts.
  U third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp

Patch:      
third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp
Index: third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp
diff --git
a/third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp
b/third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp
index
f85000566d7588d11973c32d0ae9d0a7ac00e9ed..762d642c6132a93898329cac476533b7ff4842ac
100644
--- a/third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp
+++ b/third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp
@@ -221,8 +221,10 @@ void V8Window::openMethodCustom(const
v8::FunctionCallbackInfo<v8::Value>& info)
     // |impl| has to be a LocalDOMWindow, since RemoteDOMWindows wouldn't have
     // passed the BindingSecurity check above.
     DOMWindow* openedWindow = toLocalDOMWindow(impl)->open(urlString,
frameName, windowFeaturesString, currentDOMWindow(info.GetIsolate()),
enteredDOMWindow(info.GetIsolate()));
-    if (!openedWindow)
+    if (!openedWindow) {
+        v8SetReturnValueNull(info);
         return;
+    }
 
     v8SetReturnValueFast(info, openedWindow, impl);
 }

[Mike West, 2016-10-06 08:57:03]

The CQ bit was checked by mkwst@chromium.org

[Mike West, 2016-10-06 08:57:04]

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

[commit-bot: I haz the power, 2016-10-06 08:57:23]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-06 10:15:13]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-10-06 10:16:54]

Description was changed from

==========
Fix 'noopener' targeting and return value.

Boris noted a few cases in which Chrome wasn't following the spec for
'noopener'. This patch addresses two of them by ensuring that the call
to 'window.open' return 'null' (rather than 'undefined'), and that we
don't reuse existing windows when processing a 'noopener' request.

BUG=651578,651579
==========

to

==========
Fix 'noopener' targeting and return value.

Boris noted a few cases in which Chrome wasn't following the spec for
'noopener'. This patch addresses two of them by ensuring that the call
to 'window.open' return 'null' (rather than 'undefined'), and that we
don't reuse existing windows when processing a 'noopener' request.

BUG=651578,651579

Committed: https://crrev.com/2fde9de3e9ffe7794844ddf9ea3640c5b418b6e5
Cr-Commit-Position: refs/heads/master@{#423493}
==========

[commit-bot: I haz the power, 2016-10-06 10:16:55]

Patchset 4 (id:??) landed as
https://crrev.com/2fde9de3e9ffe7794844ddf9ea3640c5b418b6e5
Cr-Commit-Position: refs/heads/master@{#423493}