Summary: | Remove the incorrectly implemented and unused SQLiteStatement::getColumnBlob(). | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Lyon Chen <liachen> | ||||||
Component: | Platform | Assignee: | Lyon Chen <liachen> | ||||||
Status: | RESOLVED DUPLICATE | ||||||||
Severity: | Normal | CC: | beidson, darin, joenotcharles | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Lyon Chen
2013-05-16 07:40:55 PDT
cc: Joe Mason, Darin, Beidson. Created attachment 201957 [details]
Patch for 116223.
Comment on attachment 201957 [details] Patch for 116223. View in context: https://bugs.webkit.org/attachment.cgi?id=201957&action=review > Source/WebCore/ChangeLog:15 > + No new tests needed as no one use SQLiteStatement::getColumnBlob() yet. Still needs a test, to make sure it's actually working when someone wants to use it. (In reply to comment #3) > (From update of attachment 201957 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=201957&action=review > > > Source/WebCore/ChangeLog:15 > > + No new tests needed as no one use SQLiteStatement::getColumnBlob() yet. > > Still needs a test, to make sure it's actually working when someone wants to use it. So it will be a unit test, instead of layout test? I'd say that if SQLiteStatement::getColumnBlob() is never called, we should strongly consider removing it completely. (In reply to comment #5) > I'd say that if SQLiteStatement::getColumnBlob() is never called, we should strongly consider removing it completely. But I am planning to use it, and then found the issue. Created attachment 201969 [details]
Remove it instead of fixing it.
(In reply to comment #7) > Created an attachment (id=201969) [details] > Remove it instead of fixing it. Are you planning to re-add it along with our platform specific patch once we're finished the code that uses it? (In reply to comment #8) > (In reply to comment #7) > > Created an attachment (id=201969) [details] [details] > > Remove it instead of fixing it. > > Are you planning to re-add it along with our platform specific patch once we're finished the code that uses it? Never mind, I actually looked at the patch and I see your comment now. :) (In reply to comment #9) > (In reply to comment #8) > > (In reply to comment #7) > > > Created an attachment (id=201969) [details] [details] [details] > > > Remove it instead of fixing it. > > > > Are you planning to re-add it along with our platform specific patch once we're finished the code that uses it? > > Never mind, I actually looked at the patch and I see your comment now. :) Yeah, I thought getColumnBlob name more correct than getColumnBlobAsVector, and can avoid unnecessary full copy. But then I think maybe this is not that terrible. At least IconDatabase::getImageDataForIconURLFromSQLDatabase() is doing it this way. (I think it should use SharedBuffer::adoptVector() instead of SharedBuffer::create(), but it is another story), and our password is short. Comment on attachment 201969 [details] Remove it instead of fixing it. View in context: https://bugs.webkit.org/attachment.cgi?id=201969&action=review > Source/WebCore/ChangeLog:12 > + No new tests needed. Whaaaaat? ;) Comment on attachment 201969 [details]
Remove it instead of fixing it.
Let's cq- actually:
-Please change the title to reflect what is in the patch.
Comment on attachment 201969 [details]
Remove it instead of fixing it.
Re-set the commit-queue flag after changing the bug title to reflect the patch.
Ben: do you feel the changed title is OK now?
Comment on attachment 201969 [details] Remove it instead of fixing it. Bug 127868 made the same change. *** This bug has been marked as a duplicate of bug 127868 *** |