[chromium] Helper Plugin does not have a URL for its Document

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[David Dorwin]

WebHelperPluginImpl creates a Document using DocumentWriter, but it does not provide a URL to begin(). As a result, the Document does not have a URL, meaning the browser cannot make decisions based on the host. Since the Helper Plugin is working on behalf or an element, it should have the same origin as the element.

[David Dorwin]

Created attachment 192325 [details]
Patch

[Adam Barth]

Comment on attachment 192325 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=192325&action=review

> Source/WebKit/chromium/src/WebHelperPluginImpl.cpp:67
> +    // Give the new document the same URL as the hose frame so that content
> +    // settings and other decisions can be made based on the correct origin.
> +    const WebURL& hostFrameURL = hostFrame->document().url();

Why do we pass in a hostFrame instead of a hostDocument?

> Source/WebKit/chromium/src/WebHelperPluginImpl.cpp:71
> +    writer.begin(hostFrameURL);

Frames don't have URLs.  Documents have URLs.

> Source/WebKit/chromium/src/WebViewImpl.cpp:168
> +
> +

These changes look spurious.

[David Dorwin]

Created attachment 192329 [details]
Patch

[David Dorwin]

Comment on attachment 192325 [details]
Patch

Thanks.

Any concerns about how |url| is used while creating the Document or during its lifetime? During creation, it seems to mostly be important whether it is a plugin (i.e. foo.swf), which is not the case here.

View in context: https://bugs.webkit.org/attachment.cgi?id=192325&action=review

>> Source/WebKit/chromium/src/WebHelperPluginImpl.cpp:67
>> +    const WebURL& hostFrameURL = hostFrame->document().url();
> 
> Why do we pass in a hostFrame instead of a hostDocument?

Done.

No reason really. Now, it's a const ref, which is nice.

>> Source/WebKit/chromium/src/WebHelperPluginImpl.cpp:71
>> +    writer.begin(hostFrameURL);
> 
> Frames don't have URLs.  Documents have URLs.

Done.

>> Source/WebKit/chromium/src/WebViewImpl.cpp:168
>> +
> 
> These changes look spurious.

Done.

[Adam Barth]

Comment on attachment 192329 [details]
Patch

ok

[WebKit Review Bot]

Comment on attachment 192329 [details]
Patch

Clearing flags on attachment: 192329

Committed r145319: <http://trac.webkit.org/changeset/145319>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.