| Summary: | IndexedDB: Re-enable indexeddb in test_shell | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | David Grogan <dgrogan> | ||||||||
| Component: | New Bugs | Assignee: | David Grogan <dgrogan> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | alecflett, jsbell, tony, webkit.review.bot | ||||||||
| Priority: | P2 | ||||||||||
| Version: | 528+ (Nightly build) | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Attachments: |
|
||||||||||
|
Description
David Grogan
2012-07-12 16:17:44 PDT
Created attachment 152091 [details]
Patch
Josh, could you take a look? Oops, thanks. LGTM. I misread the boolean logic when making the patch in 90310. It wasn't intentional.
I think you can dispense with the comment. I was reading the logic as "must have a permission client, and permission client must say okay" since the whole thing was inside an if (...) { error } structure. This patch makes it clearer: "allowed if either no permission client, or permission client says okay".
Created attachment 152101 [details]
Patch
Tony, could you review this? Josh, good to know it was accidental. I left the comment in because it's otherwise hard to know the effect of us erring on the side of permissiveness there. Comment on attachment 152101 [details]
Patch
OK since it's fixing a regression.
It seems weird to me that we are allowed to run if there's no permissionClient. What does content_shell do? Would it be better to provide a mock permission client in test_shell/content_shell?
Created attachment 152316 [details]
Patch for landing
(In reply to comment #6) > (From update of attachment 152101 [details]) > OK since it's fixing a regression. > > It seems weird to me that we are allowed to run if there's no permissionClient. What does content_shell do? Would it be better to provide a mock permission client in test_shell/content_shell? Agreed. content_shell also returns NULL. Bug filed for mock permission client: http://crbug.com/137269 Comment on attachment 152316 [details] Patch for landing Clearing flags on attachment: 152316 Committed r122622: <http://trac.webkit.org/changeset/122622> All reviewed patches have been landed. Closing bug. |