Bug 41864 – [V8] Navigation policy doesn't play nicely with pepper plugins

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Adam Barth]

[V8] Navigation policy doesn't play nicely with pepper plugins

[Adam Barth]

Created attachment 60898 [details]
Patch

[John Abd-El-Malek]

Created attachment 60913 [details]
Updated patch

[Adam Barth]

Comment on attachment 60913 [details]
Updated patch

This patch does not have a ChangeLog.

V8Utilities:103
 +  Frame* callingFrameForNavigation() {
This isn't the right name for this function.  The calling frame isn't changing because we're interested in navigation.  Maybe the name "activeFrameForNavigation" is better?  It's really the same thing we do for the canAccess check, so it might be worth sharing there too.  Maybe principalFrame (to steal some terminology from Mozilla)?

[Darin Fisher (:fishd, Google)]

I was struggling to suggest a name too.  I like principalFrame since it helps specify how we intend to use the frame.

[Darin Fisher (:fishd, Google)]

Another choice is callingOrEnteredFrame.  It seems common in WebKit to use
function names like that.  It doesn't add any new terms to understand and
just states what the function does.

[John Abd-El-Malek]

Created attachment 60937 [details]
Patch

[John Abd-El-Malek]

callingOrEnteredFrame sounds good to me.

[Adam Barth]

Comment on attachment 60937 [details]
Patch

-    if (!callingFrame)
-        return;

You removed this null check.  Retrieving the frame can fail if the entered context is associated with an inactive document.  In that case, this patch introduces a crash.

[Adam Barth]

Comment on attachment 60937 [details]
Patch

WebCore/bindings/v8/V8Utilities.cpp:119
 +      Frame* callingFrame = callingOrEnteredFrame();
Also, "callingFrame" is not an accurate name anymore.

[John Abd-El-Malek]

Created attachment 60975 [details]
Patch

[Adam Barth]

Thanks John.

[Adam Barth]

Comment on attachment 60975 [details]
Patch

This will actually fail to land in the commit-queue because the ChangeLog says OOPS.  I can land it manually for John.

[John Abd-El-Malek]

Thanks for the initial pointer and review.  I can take care of landing it.

[John Abd-El-Malek]

Committed r62868: <http://trac.webkit.org/changeset/62868>