Summary: | Add a test for FrameLoaderClient::allowImages | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Darin Fisher (:fishd, Google) <fishd> | ||||||||||
Component: | Tools / Tests | Assignee: | jochen | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | abarth, commit-queue, jochen | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
Attachments: |
|
Description
Darin Fisher (:fishd, Google)
2010-01-29 00:47:35 PST
Created attachment 95405 [details]
Patch
Comment on attachment 95405 [details]
Patch
it's quite difficult to detect that an image is not loaded because we're not sending the onerror signal on purpose. So I'm just testing that neither onerror nor onloaded gets called
Created attachment 95408 [details]
Patch
Comment on attachment 95408 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95408&action=review > LayoutTests/platform/chromium/permissionclient/image-permissions.html:24 > + img.src = "resources/boston2.gif"; You don't actually need a separate image. You can just use a query parameter. We have a bunch of images you can use for tests already. Do we the rights to the image you're adding? > Tools/DumpRenderTree/chromium/WebPermissions.cpp:36 > +WebPermissions::WebPermissions() > + : m_imagesAllowed(true) > + , m_storageAllowed(true) Should this just call reset() ? > Tools/DumpRenderTree/chromium/WebPermissions.cpp:52 > +bool WebPermissions::allowImages(WebKit::WebFrame*, bool enabledPerSettings) > +{ > + return enabledPerSettings && m_imagesAllowed; > +} > + > +bool WebPermissions::allowStorage(WebKit::WebFrame*, bool) > +{ > + return m_storageAllowed; > +} Why doesn't allowStorage use enabledPerSettings as well? Comment on attachment 95408 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95408&action=review >> LayoutTests/platform/chromium/permissionclient/image-permissions.html:24 >> + img.src = "resources/boston2.gif"; > > You don't actually need a separate image. You can just use a query parameter. We have a bunch of images you can use for tests already. Do we the rights to the image you're adding? the image is from fast/$something/resources >> Tools/DumpRenderTree/chromium/WebPermissions.cpp:36 >> + , m_storageAllowed(true) > > Should this just call reset() ? i can see advantages for both. I'll change it to reset >> Tools/DumpRenderTree/chromium/WebPermissions.cpp:52 >> +} > > Why doesn't allowStorage use enabledPerSettings as well? the second parameter is "local" as in localStorage vs sessionStorage. There's no setting that controls storage Created attachment 95636 [details]
Patch
Created attachment 95639 [details]
Patch
Comment on attachment 95639 [details] Patch Clearing flags on attachment: 95639 Committed r87860: <http://trac.webkit.org/changeset/87860> All reviewed patches have been landed. Closing bug. |