In webdatabase, it has used constants vairables as well as using SQL constants directly. If SQL constants are changed, we should modify many files. Besides it causes to use if~else statement which consumes more cpu cycles compared to switch~case. This patch introduces SQLResultType eunm type, and use it. Additionally if~else statment is changed by switch~case.
Created attachment 249963 [details] Patch
Created attachment 249964 [details] Patch
Created attachment 250016 [details] Patch
Created attachment 250067 [details] Patch
Comment on attachment 250067 [details] Patch Attachment 250067 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/4584866775564288 New failing tests: fast/fixed-layout/fixed-layout.html
Created attachment 250069 [details] Archive of layout-test-results from ews107 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
(In reply to comment #5) > Comment on attachment 250067 [details] > Patch > > Attachment 250067 [details] did not pass mac-wk2-ews (mac-wk2): > Output: http://webkit-queues.appspot.com/results/4584866775564288 > > New failing tests: > fast/fixed-layout/fixed-layout.html Looks unrelated test failure.
Created attachment 250074 [details] Patch
Comment on attachment 250074 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=250074&action=review I don’t understand why we are defining our own enum. Why not use the SQLite3 constants directly? > Source/WebCore/platform/sql/SQLiteDatabase.h:48 > +typedef enum { In C++ preferred syntax is enum X { } rather than typedef enum { } X; Also, in new code we prefer to use enum class. So this should be: enum class SQLResult { Done = SQLITE_DONE, ... } Then the constants are named SQLResult::Done.
Created attachment 250113 [details] Patch
(In reply to comment #9) > Comment on attachment 250074 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=250074&action=review > > I don’t understand why we are defining our own enum. Why not use the SQLite3 > constants directly? As I wrote in ChangeLog, it can cause to modify many files if constants of SQLite3 are changed. However If you think to use the constants directly, I don't object to do it. Updated patch uses the constants directly. > > Source/WebCore/platform/sql/SQLiteDatabase.h:48 > > +typedef enum { > > In C++ preferred syntax is enum X { } rather than typedef enum { } X; > > Also, in new code we prefer to use enum class. So this should be: > > enum class SQLResult { > Done = SQLITE_DONE, > ... > } > > Then the constants are named SQLResult::Done. If I change name of type, I had to modify more files which have used existing own constants variables. However new patch just uses SQLite3 constatns directly.
Created attachment 250117 [details] Patch
Darin, I wonder what do you think about updated patch.
Comment on attachment 250117 [details] Patch Clearing flags on attachment: 250117 Committed r182365: <http://trac.webkit.org/changeset/182365>
All reviewed patches have been landed. Closing bug.