Bug 32913 – Do not a new Database pointer to any structure until its version has been verified

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Dumitru Daniliuc]

We should add the pointer to a new Database to DatabaseTracker, Document, and DatabaseThread only after opening the database and successfully verifying its version.

[Dumitru Daniliuc]

Created attachment 45458 [details]
patch

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 45458 [details] without any errors.

[Eric Seidel (no email)]

Comment on attachment 45458 [details]
patch

Change looks great!  How do we test this?

[Eric Seidel (no email)]

Comment on attachment 45458 [details]
patch

Please update the ChangeLog when you land to point out:

1.  What test this fixes.
2.  What revision caused this regression.
3.  That this is in fact a regression fix.

A common way to do that is for the bug title to have REGRESSION(r12345): in it, and to link to the regrssion commit from the Changelog:
http://trac.webkit.org/changeset/12345

And then just to list the test that it fixes!

I certainly don't need to see this again. :)  Thanks for the quick fix!

[Dumitru Daniliuc]

Sorry, didn't know that: REGRESSION(r52530). Updated the ChangeLog file accordingly too.

Landed as r52536.

[Shinichiro Hamaji]

Committed r52554: <http://trac.webkit.org/changeset/52554>

[Daniel Bates]

This change (r52554) caused layout test failures on the Leopard Intel Debug (Tests) bot.

The following are the failing tests (*):

storage/open-database-while-transaction-in-progress.html -> crashed
svg/W3C-SVG-1.1/filters-conv-01-f.svg -> failed

See <http://build.webkit.org/results/Leopard%20Intel%20Debug%20(Tests)/r52554%20(8651)/> for test output/stderrs.

(*) From <http://build.webkit.org/builders/Leopard%20Intel%20Debug%20%28Tests%29/builds/8651/steps/layout-test/logs/stdio>

[Eric Seidel (no email)]

Shinichiro, what broke?  Your rollout change does not mention what broke:
http://trac.webkit.org/changeset/52554

Also please be sure to re-open bugs if you roll them out.

Also, it looks like your rollout broke two tests.  :(
https://bugs.webkit.org/show_bug.cgi?id=32955

I'm sure you didn't mean to do all this, but it seems you were very unlucky this time. :(

I'm not sure how the rollout could have caused the SVG failure, but it started failing after r52554 and has been failing since.

[Eric Seidel (no email)]

I'm going to roll-out the rollout for now.  I'll test it locally first to make sure all tests pass on Leopard.

[Dumitru Daniliuc]

I figured out the problem. My patch was technically correct (and that's why WebKit was happy with it). However, it changed the ordering of some actions in a subtle way and that broke an assumption on which the Chromium implementation was built.

I opened bug 33005 to track this issue and will soon upload/commit a patch that should make both WebKit and Chromium happy.

Closing this bug.