RESOLVED FIXED 56259
StorageTracker constructor shouldn't have initialization code and isMainThread() assertion 
https://bugs.webkit.org/show_bug.cgi?id=56259
Summary StorageTracker constructor shouldn't have initialization code and isMainThrea...
Anton D'Auria
Reported 2011-03-12 20:20:46 PST
All initialization code should be in StorageTracker::initializeTracker(). This removes the requirement that StorageTracker constructor must be called on the main thread.
Attachments
Patch (2.31 KB, patch)
2011-03-12 20:27 PST, Anton D'Auria 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Anton D'Auria
Comment 1 2011-03-12 20:27:11 PST
Created attachment 85605 [details] Patch
Alice Liu
Comment 2 2011-03-12 22:46:31 PST
I have just a couple questions. Pardon my lack of understanding of when and where StorageTracker is supposed to exist, but it seems that your patch does not address StorageTracker constructor running on non-main threads, but instead moves the assertion that it's running on the main thread to the initialization of StorageTracker. This seems to me that it doesn't prevent all the non-main threadsfrom creating a StorageTracker instance that doesn't do anything. Maybe i just need a brief overview of StorageTracker ...
Anton D'Auria
Comment 3 2011-03-13 00:21:40 PST
(In reply to comment #2) > I have just a couple questions. Pardon my lack of understanding of when and where StorageTracker is supposed to exist, but it seems that your patch does not address StorageTracker constructor running on non-main threads, but instead moves the assertion that it's running on the main thread to the initialization of StorageTracker. This seems to me that it doesn't prevent all the non-main threadsfrom creating a StorageTracker instance that doesn't do anything. Maybe i just need a brief overview of StorageTracker ... The constructor started the LocalStorageTask thread. In LocalStorageTask, thread creation isn't protected by a mutex, so it must happen on the main thread. I moved this code out of the constructor to the initialization function, which now must run on the main thread, and so I removed the assert in this patch.
Anton D'Auria
Comment 4 2011-03-13 00:22:56 PST
(In reply to comment #3) > (In reply to comment #2) > > I have just a couple questions. Pardon my lack of understanding of when and where StorageTracker is supposed to exist, but it seems that your patch does not address StorageTracker constructor running on non-main threads, but instead moves the assertion that it's running on the main thread to the initialization of StorageTracker. This seems to me that it doesn't prevent all the non-main threadsfrom creating a StorageTracker instance that doesn't do anything. Maybe i just need a brief overview of StorageTracker ... > > The constructor started the LocalStorageTask thread. In LocalStorageTask, thread creation isn't protected by a mutex, so it must happen on the main thread. I moved this code out of the constructor to the initialization function, which now must run on the main thread, and so I removed the assert in this patch. So now, it's fine for the constructor to be called on any thread, but the StorageTracker::initializeTracker() must be called on the main thread for the reason stated above.
Alice Liu
Comment 5 2011-03-13 00:25:01 PST
ok. r=me
WebKit Commit Bot
Comment 6 2011-03-13 12:08:09 PDT
Comment on attachment 85605 [details] Patch Clearing flags on attachment: 85605 Committed r80964: <http://trac.webkit.org/changeset/80964>
WebKit Commit Bot
Comment 7 2011-03-13 12:08:14 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.