Summary: | IndexedDB: open-in-upgradeneeded layout test | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | David Grogan <dgrogan> | ||||||||||
Component: | New Bugs | Assignee: | David Grogan <dgrogan> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | alecflett, dglazkov, jsbell, tony, webkit.review.bot | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | 90411 | ||||||||||||
Bug Blocks: | |||||||||||||
Attachments: |
|
Description
David Grogan
2012-09-05 16:52:50 PDT
Created attachment 162370 [details]
Patch
This should wait until after 90411 lands. This currently behaves differently in DRT and content_shell, though I'm not sure which is more correct. Suppose we call the result of this test in DRT "A" and the result from content_shell "B". "B" is included in this patch. It's also what firefox produces. If the code from 90411 is patched in, the result of this test in DRT is now "B". But then the result from content_shell ("C") is different from either "A" or "B". "C" is definitely wrong. So I'll wait until 90411 lands and sort this out after. Comment on attachment 162370 [details] Patch Attachment 162370 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13785566 New failing tests: storage/indexeddb/intversion-open-in-upgradeneeded.html I just tried this patch again, and it passes in DRT and content_shell's output matches. Did we simplify 90411 enough to eliminate the "B" => "C" change? Created attachment 165022 [details]
Patch
Yeah, seems to work. Can you look it over for an LGTM? Comment on attachment 165022 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=165022&action=review lgtm > LayoutTests/storage/indexeddb/resources/intversion-open-in-upgradeneeded.js:36 > + transaction.oncomplete = function() { Just as a style thing, if this was written as: transaction.oncomplete = function transactionOnComplete(evt) { ... then you could use preamble(evt); and get the name printed out while still (IMHO) keeping the readability benefits of a function expression. (I haven't done that anywhere myself, was just pondering it while reading this patch.) > LayoutTests/storage/indexeddb/resources/intversion-open-in-upgradeneeded.js:55 > + evalAndLog("transaction = db.transaction('os')"); Is this just to confirm that the connection is still open? Maybe a debug() line explaining it would help. Is this waiting on anything? Created attachment 167664 [details]
Patch
Created attachment 167665 [details]
Patch
Tony, could you review this test? Comment on attachment 167665 [details] Patch Clearing flags on attachment: 167665 Committed r130791: <http://trac.webkit.org/changeset/130791> All reviewed patches have been landed. Closing bug. |