[IndexedDB] Use consistent enums on both sides of IPC.

[IndexedDB] Use consistent enums on both sides of IPC.

Numerous IndexedDB IPC messages sent renderer->browser are defined members that are enums from Blink (public/platform), and on receipt are static_cast to types
defined in Chromium:

blink::WebIDBCursor::Direction => indexed_db::CursorDirection
blink::WebIDBDatabase::TaskType => IndexedDBDatabase::TaskType
blink::WebIDBDatabase::PutMode => IndexedDBDatabase::PutMode
blink::WebIDBDatabase::TransactionMode => uint16 => indexed_db::TransactionMode

Nothing enforces the equality of these enums at compile time.

So the approach adopted here is to move the Blink-side enums into public/platform/WebIDBTypes.h and use it everywhere.

Patch spread across chromium content side as well as on Blink side.
Chromium side: https://codereview.chromium.org/320833002
Blink side: https://codereview.chromium.org/325683002

BUG=381848
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=278953

[Pritam Nikam, 2014-06-09 10:44:05]

Hi,

I've adopted 2nd approach i.e. to move the Blink-side enums into
public/platform/WebIDBTypes.h and use it everywhere.

Patch spread across chromium content side as well as on Blink side.
Chromium side: https://codereview.chromium.org/320833002
Blink side: https://codereview.chromium.org/325683002 

Please have a look.

[jsbell, 2014-06-09 16:38:27]

Initial feedback.

I commented on the blink side that enum names like "blink::Direction" should be
expanded to "blink::WebIDBCursorDirection" - and similarly expanded for the
values. That'll cause a lot of churn on this patch.

https://codereview.chromium.org/320833002/diff/30001/content/browser/indexed_...
File content/browser/indexed_db/indexed_db_backing_store.cc (right):

https://codereview.chromium.org/320833002/diff/30001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_backing_store.cc:33: #include
"third_party/WebKit/public/platform/WebIDBTypes.h"
This include can be removed when added to the .h

https://codereview.chromium.org/320833002/diff/30001/content/browser/indexed_...
File content/browser/indexed_db/indexed_db_backing_store.h (right):

https://codereview.chromium.org/320833002/diff/30001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_backing_store.h:29: #include
"third_party/leveldatabase/src/include/leveldb/status.h"
Need to #include "third_party/WebKit/public/platform/WebIDBTypes.h"

https://codereview.chromium.org/320833002/diff/30001/content/browser/indexed_...
File content/browser/indexed_db/indexed_db_cursor.h (right):

https://codereview.chromium.org/320833002/diff/30001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_cursor.h:15: #include
"content/common/indexed_db/indexed_db_key_range.h"
Need to #include "third_party/WebKit/public/platform/WebIDBTypes.h"

https://codereview.chromium.org/320833002/diff/30001/content/browser/indexed_...
File content/browser/indexed_db/indexed_db_database.h (right):

https://codereview.chromium.org/320833002/diff/30001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_database.h:23: #include "url/gurl.h"
Need to #include "third_party/WebKit/public/platform/WebIDBTypes.h"

https://codereview.chromium.org/320833002/diff/30001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_database.h:40: enum PutMode {
Remove this enum too?

https://codereview.chromium.org/320833002/diff/30001/content/browser/indexed_...
File content/browser/indexed_db/indexed_db_transaction.h (right):

https://codereview.chromium.org/320833002/diff/30001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_transaction.h:19: #include
"content/browser/indexed_db/indexed_db_database_error.h"
Need to #include "third_party/WebKit/public/platform/WebIDBTypes.h"

https://codereview.chromium.org/320833002/diff/30001/content/browser/indexed_...
File content/browser/indexed_db/indexed_db_transaction_coordinator.cc (right):

https://codereview.chromium.org/320833002/diff/30001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_transaction_coordinator.cc:9: #include
"content/browser/indexed_db/indexed_db_transaction.h"
need to include WebIDBTypes.h

https://codereview.chromium.org/320833002/diff/30001/content/child/indexed_db...
File content/child/indexed_db/indexed_db_dispatcher.h (right):

https://codereview.chromium.org/320833002/diff/30001/content/child/indexed_db...
content/child/indexed_db/indexed_db_dispatcher.h:19: #include
"third_party/WebKit/public/platform/WebIDBCallbacks.h"
Need to #include "third_party/WebKit/public/platform/WebIDBTypes.h"

https://codereview.chromium.org/320833002/diff/30001/content/child/indexed_db...
content/child/indexed_db/indexed_db_dispatcher.h:20: #include
"third_party/WebKit/public/platform/WebIDBCursor.h"
Can this be removed now?

https://codereview.chromium.org/320833002/diff/30001/content/child/indexed_db...
content/child/indexed_db/indexed_db_dispatcher.h:21: #include
"third_party/WebKit/public/platform/WebIDBDatabase.h"
Can this be removed now?

https://codereview.chromium.org/320833002/diff/30001/content/child/indexed_db...
File content/child/indexed_db/webidbdatabase_impl.h (right):

https://codereview.chromium.org/320833002/diff/30001/content/child/indexed_db...
content/child/indexed_db/webidbdatabase_impl.h:11: #include
"third_party/WebKit/public/platform/WebIDBDatabase.h"
Need to #include "third_party/WebKit/public/platform/WebIDBTypes.h"

https://codereview.chromium.org/320833002/diff/30001/content/common/indexed_d...
File content/common/indexed_db/indexed_db_messages.h (right):

https://codereview.chromium.org/320833002/diff/30001/content/common/indexed_d...
content/common/indexed_db/indexed_db_messages.h:17: #include
"third_party/WebKit/public/platform/WebIDBDatabase.h"
Need to #include "third_party/WebKit/public/platform/WebIDBTypes.h"

And you should be able to remove the other public/platform/WebIDB*.h includes.

[Pritam Nikam, 2014-06-11 14:05:16]

Thanks jsbell!
Incorporated review comments and new patch is ready for review. Please have a
look.

https://codereview.chromium.org/320833002/diff/30001/content/browser/indexed_...
File content/browser/indexed_db/indexed_db_backing_store.cc (right):

https://codereview.chromium.org/320833002/diff/30001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_backing_store.cc:33: #include
"third_party/WebKit/public/platform/WebIDBTypes.h"
On 2014/06/09 16:38:27, jsbell wrote:
> This include can be removed when added to the .h

Done.

https://codereview.chromium.org/320833002/diff/30001/content/browser/indexed_...
File content/browser/indexed_db/indexed_db_backing_store.h (right):

https://codereview.chromium.org/320833002/diff/30001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_backing_store.h:29: #include
"third_party/leveldatabase/src/include/leveldb/status.h"
On 2014/06/09 16:38:27, jsbell wrote:
> Need to #include "third_party/WebKit/public/platform/WebIDBTypes.h"

Done.

https://codereview.chromium.org/320833002/diff/30001/content/browser/indexed_...
File content/browser/indexed_db/indexed_db_cursor.h (right):

https://codereview.chromium.org/320833002/diff/30001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_cursor.h:15: #include
"content/common/indexed_db/indexed_db_key_range.h"
On 2014/06/09 16:38:27, jsbell wrote:
> Need to #include "third_party/WebKit/public/platform/WebIDBTypes.h"

Done.

https://codereview.chromium.org/320833002/diff/30001/content/browser/indexed_...
File content/browser/indexed_db/indexed_db_database.h (right):

https://codereview.chromium.org/320833002/diff/30001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_database.h:23: #include "url/gurl.h"
On 2014/06/09 16:38:27, jsbell wrote:
> Need to #include "third_party/WebKit/public/platform/WebIDBTypes.h"

Done.

https://codereview.chromium.org/320833002/diff/30001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_database.h:40: enum PutMode {
On 2014/06/09 16:38:27, jsbell wrote:
> Remove this enum too?

Done.

https://codereview.chromium.org/320833002/diff/30001/content/browser/indexed_...
File content/browser/indexed_db/indexed_db_transaction.h (right):

https://codereview.chromium.org/320833002/diff/30001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_transaction.h:19: #include
"content/browser/indexed_db/indexed_db_database_error.h"
On 2014/06/09 16:38:27, jsbell wrote:
> Need to #include "third_party/WebKit/public/platform/WebIDBTypes.h"

Done.

https://codereview.chromium.org/320833002/diff/30001/content/browser/indexed_...
File content/browser/indexed_db/indexed_db_transaction_coordinator.cc (right):

https://codereview.chromium.org/320833002/diff/30001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_transaction_coordinator.cc:9: #include
"content/browser/indexed_db/indexed_db_transaction.h"
On 2014/06/09 16:38:27, jsbell wrote:
> need to include WebIDBTypes.h

Done.

https://codereview.chromium.org/320833002/diff/30001/content/child/indexed_db...
File content/child/indexed_db/indexed_db_dispatcher.h (right):

https://codereview.chromium.org/320833002/diff/30001/content/child/indexed_db...
content/child/indexed_db/indexed_db_dispatcher.h:19: #include
"third_party/WebKit/public/platform/WebIDBCallbacks.h"
On 2014/06/09 16:38:27, jsbell wrote:
> Need to #include "third_party/WebKit/public/platform/WebIDBTypes.h"

Done.

https://codereview.chromium.org/320833002/diff/30001/content/child/indexed_db...
content/child/indexed_db/indexed_db_dispatcher.h:20: #include
"third_party/WebKit/public/platform/WebIDBCursor.h"
On 2014/06/09 16:38:27, jsbell wrote:
> Can this be removed now?

Done.

https://codereview.chromium.org/320833002/diff/30001/content/child/indexed_db...
content/child/indexed_db/indexed_db_dispatcher.h:21: #include
"third_party/WebKit/public/platform/WebIDBDatabase.h"
On 2014/06/09 16:38:27, jsbell wrote:
> Can this be removed now?

Done.

https://codereview.chromium.org/320833002/diff/30001/content/child/indexed_db...
File content/child/indexed_db/webidbdatabase_impl.h (right):

https://codereview.chromium.org/320833002/diff/30001/content/child/indexed_db...
content/child/indexed_db/webidbdatabase_impl.h:11: #include
"third_party/WebKit/public/platform/WebIDBDatabase.h"
On 2014/06/09 16:38:27, jsbell wrote:
> Need to #include "third_party/WebKit/public/platform/WebIDBTypes.h"

Done.

https://codereview.chromium.org/320833002/diff/30001/content/common/indexed_d...
File content/common/indexed_db/indexed_db_messages.h (right):

https://codereview.chromium.org/320833002/diff/30001/content/common/indexed_d...
content/common/indexed_db/indexed_db_messages.h:17: #include
"third_party/WebKit/public/platform/WebIDBDatabase.h"
On 2014/06/09 16:38:27, jsbell wrote:
> Need to #include "third_party/WebKit/public/platform/WebIDBTypes.h"
> 
> And you should be able to remove the other public/platform/WebIDB*.h includes.

Done.

[jsbell, 2014-06-11 16:42:02]

Overall looks good, a few tweaks necessary. Blink side will need to land first.

tsepez@ is the usual reviewer for indexed_db_messages.h changes - you should ask
him for a review once the macro change is made.

https://codereview.chromium.org/320833002/diff/50001/content/browser/indexed_...
File content/browser/indexed_db/indexed_db_database.cc (right):

https://codereview.chromium.org/320833002/diff/50001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_database.cc:1439: uint16 mode) {
Can you change this to use blink::WebIDBTransactionMode ?

https://codereview.chromium.org/320833002/diff/50001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_database.cc:1453:
static_cast<blink::WebIDBTransactionMode>(mode),
... and then this cast can go away.

https://codereview.chromium.org/320833002/diff/50001/content/browser/indexed_...
File content/browser/indexed_db/indexed_db_database.h (right):

https://codereview.chromium.org/320833002/diff/50001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_database.h:84: uint16 mode);
Can you update this to use the blink::WebIDBTransactionMode enum?

https://codereview.chromium.org/320833002/diff/50001/content/browser/indexed_...
File content/browser/indexed_db/indexed_db_dispatcher_host.cc (right):

https://codereview.chromium.org/320833002/diff/50001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_dispatcher_host.cc:669:
static_cast<blink::WebIDBPutMode>(params.put_mode),
This static_cast should no longer be necessary.

https://codereview.chromium.org/320833002/diff/50001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_dispatcher_host.cc:728:
static_cast<blink::WebIDBCursorDirection>(params.direction),
This static_cast should no longer be necessary.

https://codereview.chromium.org/320833002/diff/50001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_dispatcher_host.cc:730:
static_cast<blink::WebIDBTaskType>(params.task_type),
This static_cast should no longer be necessary.

https://codereview.chromium.org/320833002/diff/50001/content/common/indexed_d...
File content/common/indexed_db/indexed_db_messages.h (right):

https://codereview.chromium.org/320833002/diff/50001/content/common/indexed_d...
content/common/indexed_db/indexed_db_messages.h:28:
IPC_ENUM_TRAITS(blink::WebIDBTransactionMode)
Change this to IPC_ENUM_TRAITS_MAX_VALUE once Last is added on the Blink side?

[Pritam Nikam, 2014-06-12 11:19:20]

Thanks jsbell!
Incorporated review comments and new patch uploaded for review. Please have a
look.

@Tom,
Please review index_db_messages.h changes.

https://codereview.chromium.org/320833002/diff/50001/content/browser/indexed_...
File content/browser/indexed_db/indexed_db_database.cc (right):

https://codereview.chromium.org/320833002/diff/50001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_database.cc:1439: uint16 mode) {
On 2014/06/11 16:42:01, jsbell wrote:
> Can you change this to use blink::WebIDBTransactionMode ?

Done.

https://codereview.chromium.org/320833002/diff/50001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_database.cc:1453:
static_cast<blink::WebIDBTransactionMode>(mode),
On 2014/06/11 16:42:01, jsbell wrote:
> ... and then this cast can go away.

Done.

https://codereview.chromium.org/320833002/diff/50001/content/browser/indexed_...
File content/browser/indexed_db/indexed_db_database.h (right):

https://codereview.chromium.org/320833002/diff/50001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_database.h:84: uint16 mode);
On 2014/06/11 16:42:01, jsbell wrote:
> Can you update this to use the blink::WebIDBTransactionMode enum?

Done.

https://codereview.chromium.org/320833002/diff/50001/content/browser/indexed_...
File content/browser/indexed_db/indexed_db_dispatcher_host.cc (right):

https://codereview.chromium.org/320833002/diff/50001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_dispatcher_host.cc:669:
static_cast<blink::WebIDBPutMode>(params.put_mode),
On 2014/06/11 16:42:02, jsbell wrote:
> This static_cast should no longer be necessary.

Done.

https://codereview.chromium.org/320833002/diff/50001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_dispatcher_host.cc:728:
static_cast<blink::WebIDBCursorDirection>(params.direction),
On 2014/06/11 16:42:01, jsbell wrote:
> This static_cast should no longer be necessary.

Done.

https://codereview.chromium.org/320833002/diff/50001/content/browser/indexed_...
content/browser/indexed_db/indexed_db_dispatcher_host.cc:730:
static_cast<blink::WebIDBTaskType>(params.task_type),
On 2014/06/11 16:42:01, jsbell wrote:
> This static_cast should no longer be necessary.

Done.

https://codereview.chromium.org/320833002/diff/50001/content/common/indexed_d...
File content/common/indexed_db/indexed_db_messages.h (right):

https://codereview.chromium.org/320833002/diff/50001/content/common/indexed_d...
content/common/indexed_db/indexed_db_messages.h:28:
IPC_ENUM_TRAITS(blink::WebIDBTransactionMode)
On 2014/06/11 16:42:02, jsbell wrote:
> Change this to IPC_ENUM_TRAITS_MAX_VALUE once Last is added on the Blink side?

Done.

[jsbell, 2014-06-12 16:26:36]

lgtm!

So what you'll want to do is land the Blink side, note what revision it lands
as, then watch for src/DEPS to update past that revision:

https://code.google.com/p/chromium/codesearch#chromium/src/DEPS&sq=package:ch...

(Don't rely on codesearch though, it's data is at least a few hours stale.)

Once that revision is rolled into Chromium, you can land this side. Once this
side has landed, you can immediately land the cleanup patch for the Blink side,
since Blink always builds with ToT Chromium.

[Tom Sepez, 2014-06-12 18:28:59]

Messages LGTM.  Thanks for making things more consistent.

[jsbell, 2014-06-18 17:13:41]

On 2014/06/12 16:26:36, jsbell wrote:
> lgtm!
> 
> So what you'll want to do is land the Blink side, note what revision it lands
> as, then watch for src/DEPS to update past that revision:

It's in Blink as r176435, so we just have to wait for a roll now, then this can
land.

[jsbell, 2014-06-20 17:43:08]

The CQ bit was checked by jsbell@chromium.org

[commit-bot: I haz the power, 2014-06-20 17:48:20]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pritam.nikam@samsung.com/320833002/70001

[jsbell, 2014-06-20 18:38:36]

The CQ bit was unchecked by jsbell@chromium.org

[jsbell, 2014-06-20 18:38:42]

The CQ bit was checked by jsbell@chromium.org

[commit-bot: I haz the power, 2014-06-20 19:03:59]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pritam.nikam@samsung.com/320833002/70001

[commit-bot: I haz the power, 2014-06-20 20:19:43]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/8...)
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/19...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/19709)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/23313)

[commit-bot: I haz the power, 2014-06-20 20:23:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-20 20:23:20]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/19754)

[Pritam Nikam, 2014-06-21 05:25:43]

The CQ bit was checked by pritam.nikam@samsung.com

[commit-bot: I haz the power, 2014-06-21 05:29:37]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pritam.nikam@samsung.com/320833002/90001

[commit-bot: I haz the power, 2014-06-21 13:14:17]

Change committed as 278953