RESOLVED FIXED 39216
Hook Chromium's WebIndexedDatabaseImpl up to IndexedDatabaseImpl 
https://bugs.webkit.org/show_bug.cgi?id=39216
Summary Hook Chromium's WebIndexedDatabaseImpl up to IndexedDatabaseImpl
Jeremy Orlow
Reported 2010-05-17 07:01:39 PDT
find . | grep CMake
Attachments
Patch (26.15 KB, patch)
2010-05-17 07:12 PDT, Jeremy Orlow 		no flags
DetailsFormatted DiffDiff
Patch (26.15 KB, patch)
2010-05-17 08:47 PDT, Jeremy Orlow 		no flags
DetailsFormatted DiffDiff
Patch (29.51 KB, patch)
2010-05-18 03:25 PDT, Jeremy Orlow 		no flags
DetailsFormatted DiffDiff
Patch (29.54 KB, patch)
2010-05-18 04:28 PDT, Jeremy Orlow 		fishd: review+
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Jeremy Orlow
Comment 1 2010-05-17 07:03:13 PDT
Oops...user error.
Jeremy Orlow
Comment 2 2010-05-17 07:12:33 PDT
Created attachment 56239 [details] Patch
WebKit Review Bot
Comment 3 2010-05-17 07:49:08 PDT
Attachment 56239 [details] did not build on chromium: Build output: http://webkit-commit-queue.appspot.com/results/2269238
Jeremy Orlow
Comment 4 2010-05-17 08:47:33 PDT
Created attachment 56245 [details] Patch
Jeremy Orlow
Comment 5 2010-05-17 09:55:07 PDT
+ Darin since it is mostly WebKit API changes (though I think any Chromium reviewer could look at it)
Nate Chapin
Comment 6 2010-05-17 10:05:33 PDT
Comment on attachment 56245 [details] Patch > - virtual void open(const String& name, const String& description, bool modifyDatabase, PassRefPtr<IDBCallbacks>, Frame*, ExceptionCode&); > + virtual void open(const String& name, const String& description, bool modifyDatabase, PassRefPtr<IDBCallbacks>, const String& origin, Frame*, ExceptionCode&); Here and a bunch of other places, you're passing around the origin as a String/WebString. It probably makes more sense to use SecurityOrigin/WebSecurityOrigin. That was the main thing I saw, but I'm not terribly confident on my understanding of the Chromium API layering and organization, so fishd's opinion will be appreciated. :)
Jeremy Orlow
Comment 7 2010-05-18 03:25:13 PDT
Created attachment 56349 [details] Patch
Jeremy Orlow
Comment 8 2010-05-18 03:29:08 PDT
Nate: Fixed the issues you raised. FishD: Please take a look.
WebKit Review Bot
Comment 9 2010-05-18 04:25:02 PDT
Attachment 56349 [details] did not build on chromium: Build output: http://webkit-commit-queue.appspot.com/results/2293260
Jeremy Orlow
Comment 10 2010-05-18 04:28:31 PDT
Created attachment 56360 [details] Patch
Darin Fisher (:fishd, Google)
Comment 11 2010-05-18 08:04:39 PDT
Comment on attachment 56360 [details] Patch WebKit/chromium/src/WebIndexedDatabaseImpl.cpp:63 + nit: extraneous new line WebKit/chromium/src/WebIDBDatabaseImpl.cpp:51 + nit: extraneous new line WebKit/chromium/src/WebIDBCallbacksImpl.h:61 + nit: extraneous new line
Jeremy Orlow
Comment 12 2010-05-19 03:04:53 PDT
Committed r59755: <http://trac.webkit.org/changeset/59755>
WebKit Review Bot
Comment 13 2010-05-19 03:19:18 PDT
http://trac.webkit.org/changeset/59755 might have broken Chromium Win Release
Note You need to log in before you can comment on or make changes to this bug.