See sister bug... http://code.google.com/p/chromium/issues/detail?id=40245 Not long ago in r56293, the behavior of DatabaseCloseTask was altered. One callsite was updated accordingly, but another callsite was not. The second callsite is busted due to the behavior change.
Created attachment 52446 [details] fix a bug regression
LGTM, FWIW.
Comment on attachment 52446 [details] fix a bug regression A rather pedantic r- since we'll need to update the ChangeLog before setting commit-queue+ anyway. > Index: WebCore/ChangeLog > =================================================================== > --- WebCore/ChangeLog (revision 57019) > +++ WebCore/ChangeLog (working copy) > @@ -1,3 +1,21 @@ > +2010-04-02 Michael Nordman <michaeln@google.com> > + > + Reviewed by NOBODY (OOPS!). > + > + Set the close policy used the by the DatabaseCloseTask in a constructor argument Typo here and in the other ChangeLog. > @@ -110,18 +110,20 @@ private: > > class DatabaseCloseTask : public DatabaseTask { > public: > - static PassOwnPtr<DatabaseCloseTask> create(Database* db, DatabaseTaskSynchronizer* synchronizer) > + static PassOwnPtr<DatabaseCloseTask> create(Database* db, Database::ClosePolicy closePolicy, DatabaseTaskSynchronizer* synchronizer) > { > - return new DatabaseCloseTask(db, synchronizer); > + return new DatabaseCloseTask(db, closePolicy, synchronizer); > } > > private: > - DatabaseCloseTask(Database*, DatabaseTaskSynchronizer*); > + DatabaseCloseTask(Database*, Database::ClosePolicy, DatabaseTaskSynchronizer*); > > virtual void doPerformTask(); > #ifndef NDEBUG > virtual const char* debugTaskName() const; > #endif > + > + Database::ClosePolicy m_closePolicy; > }; Perhaps we should support closePolicy as an optional parameter on the constructor/create function? I assume in the vast majority of cases we will want DoNotRemoveFromDatabaseContext. I'm willing to be convinced otherwise
> Perhaps we should support closePolicy as an optional parameter on the > constructor/create function? I assume > in the vast majority of cases we will want DoNotRemoveFromDatabaseContext. I'm > willing to be convinced otherwise I'd rather not do that. If there are any other clients of this task out there that aren't in webkit's svn repository, i'd like them to have to figure out what value is appropiate for their callsite. For example, if this explicit approach had been done originally, chrome wouldn't have been built with this subtle bug in it.
Created attachment 52453 [details] fix ChangeLog typos
Comment on attachment 52453 [details] fix ChangeLog typos Fair enough. r=me
Comment on attachment 52453 [details] fix ChangeLog typos Clearing flags on attachment: 52453 Committed r57034: <http://trac.webkit.org/changeset/57034>
All reviewed patches have been landed. Closing bug.
http://trac.webkit.org/changeset/57034 might have broken Leopard Intel Release (Tests)
(In reply to comment #9) > http://trac.webkit.org/changeset/57034 might have broken Leopard Intel Release > (Tests) fyi: this change should be a noop for all but chromium, before and after the patch, WebCore::Database::markAsDeletedAndClose() is using the DoNotRemoveDatabaseFromContext policy.
> fyi: this change should be a noop for all but chromium, before and after the > patch, WebCore::Database::markAsDeletedAndClose() is using the > DoNotRemoveDatabaseFromContext policy. My apologies, but I think you were bit by https://bugs.webkit.org/show_bug.cgi?id=36646 occurring twice in a row.