[webkit-reviews] review granted: [Bug 38152] sandbox iframes have access to top.history methods : [Attachment 55951] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Adam Barth <abarth at webkit.org>'s
request for review:
Bug 38152: sandbox iframes have access to top.history methods
https://bugs.webkit.org/show_bug.cgi?id=38152

Attachment 55951: Patch
https://bugs.webkit.org/attachment.cgi?id=55951&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +	       # The GObject bindings have no notion of an active Frame, so we
> +	       # pass 0 to indicate that all the access checks should pass.
> +	       $callImplParams = "0";

Another option is to overload these functions and have a version without a
Frame* argument. This would allow us to distinguish "no concept of frame in
this binding" from the other cases where Frame* could be zero.

Another option is to make the Frame* argument default to zero.

In either of these cases, the GTK and Objective-C binding-generation code could
just ignore the [CallWith=Frame] attribute entirely.

Why doesn't this break the Objective-C binding?

> +			   if ($callWith eq "Frame") {
> +			       push(@implContent, "    Frame* lexicalFrame =
toLexicalFrame(exec);\n");
> +			       push(@implContent, "    if (!lexicalFrame)\n");
> +			       push(@implContent, "	   return
jsUndefined();\n");
> +			       $callWithArg = "lexicalFrame";

I think we should move in the direction of the term "active frame" to sync up
with HTML5 terminology, if we can do so without sacrificing clarity. Or we
could sync up the V8 and JSC terminology for the frames.

> +    if (activeFrame &&
!activeFrame->loader()->shouldAllowNavigation(m_frame))
> +	   return;
>      m_frame->redirectScheduler()->scheduleHistoryNavigation(-1);

Is there any problem with the distance in time between this check and the
actual navigation? Perhaps the document in the active frame at the time the
navigation was scheduled should be recorded by the redirect scheduler so the
test can be done at the correct time?