[webkit-reviews] review denied: [Bug 95790] Allow third-party storage blocking setting to change while a page is loaded : [Attachment 163710] More tests
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Sep 12 15:29:58 PDT 2012
Brady Eidson <beidson at apple.com> has denied Jeffrey Pfau
<jeffrey at endrift.com>'s request for review:
Bug 95790: Allow third-party storage blocking setting to change while a page is
loaded
https://bugs.webkit.org/show_bug.cgi?id=95790
Attachment 163710: More tests
https://bugs.webkit.org/attachment.cgi?id=163710&action=review
------- Additional Comments from Brady Eidson <beidson at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=163710&action=review
The comment about asynchronous init doesn't necessarily need to be resolved
now, but it does need a bug filed about it.
My other comments are more pressing to me.
> Source/WebKit2/PluginProcess/PluginControllerProxy.cpp:573
> + m_storageBlockingPolicy =
static_cast<SecurityOrigin::StorageBlockingPolicy>(storageBlockingPolicy);
> +
> + if (!m_isPrivateBrowsingEnabled)
> + m_plugin->storageBlockingStateChanged(m_storageBlockingPolicy);
> +}
> +
We only update the storage blocking setting on each plug-in while private
browsing is disabled?
So what if the storage blocking setting changes while private browsing is
enabled, then the user later disables private browsing? Do the plug-in
instances get told about the storage blocking setting then?
It feels to me like we should always pipe this through.
> Source/WebKit2/WebProcess/Plugins/Netscape/NetscapePlugin.cpp:959
> +void
NetscapePlugin::storageBlockingStateChanged(SecurityOrigin::StorageBlockingPoli
cy storageBlockingPolicy)
> +{
> + privateBrowsingStateChanged(storageBlockingPolicy !=
SecurityOrigin::AllowAllStorage);
> +}
I think we should always pass through a call to storageBlockingStateChanged and
the NetscapePlugin remembers both the current private browsing and storage
blocking settings. Whenever *either* of them changes we calculate whether or
not we need to update the plugin code or not.
> Source/WebKit2/WebProcess/Plugins/PluginView.cpp:591
> +void
PluginView::storageBlockingStateChanged(WebCore::SecurityOrigin::StorageBlockin
gPolicy storageBlockingPolicy)
> +{
> + // The plug-in can be null here if it failed to initialize.
> + if (!m_isInitialized || !m_plugin)
> + return;
> +
> + m_plugin->storageBlockingStateChanged(storageBlockingPolicy);
> +}
In the brave new world of asynchronous plug-in initialization, we probably need
to store off the new StorageBlockingPolicy value (as well as any new private
browsing settings) so we can update the plug-in when it does actually finish
initializing.
More information about the webkit-reviews
mailing list