[IDBObservers] Moving options from constructor to .observe call.

[IDBObservers] Moving options from constructor to .observe call.

Per spec change in
https://github.com/WICG/indexed-db-observers/commit/5aa4abc288ef526801797d3a9209ac0da8e359e7.

I'm also adding more exceptions here, as per
https://github.com/WICG/indexed-db-observers/commit/7723586ef6e5663739ae4800f1e6a7172296f67c.

R=cmumford
BUG=655865
Committed: https://crrev.com/2b06152c63ff8df7b927a9ad7e5d13541aa377ea
Cr-Commit-Position: refs/heads/master@{#430779}

[dmurph, 2016-10-31 21:23:53]

Description was changed from

==========
[IDBObservers] Moving options from constructor to .observe call.

Per spec change in
https://github.com/WICG/indexed-db-observers/commit/5aa4abc288ef526801797d3a9...

R=cmumford
BUG=655865
==========

to

==========
[IDBObservers] Moving options from constructor to .observe call.

Per spec change in
https://github.com/WICG/indexed-db-observers/commit/5aa4abc288ef526801797d3a9....

I'm also adding more exceptions here, as per
https://github.com/WICG/indexed-db-observers/commit/7723586ef6e5663739ae4800f....

R=cmumford
BUG=655865
==========

[dmurph, 2016-10-31 21:24:01]

The CQ bit was checked by dmurph@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-31 21:24:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dmurph, 2016-10-31 21:25:36]

Hi Chris!

Can you PTAL at this change? It moves the observer options to the observe
function call, and is a little more robust w/ exceptions.

Thanks!
Daniel

[commit-bot: I haz the power, 2016-10-31 22:44:35]

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

[commit-bot: I haz the power, 2016-10-31 22:44:36]

Dry run: This issue passed the CQ dry run.

[cmumford, 2016-11-07 21:55:16]

lgtm % two nits.

https://codereview.chromium.org/2459113002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/storage/indexeddb/resources/observer-tests.js
(right):

https://codereview.chromium.org/2459113002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/storage/indexeddb/resources/observer-tests.js:206:
obs.observe(db, txn, {operationTypes: ['clear', 'put', 'add', 'delete']});
Makes me wonder if we should have an "all" type.

https://codereview.chromium.org/2459113002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/storage/indexeddb/resources/observer-tests.js:239:
if (changeNumber > 1) {
Nit: How about assert_less_than_equal(changeNumber, 1, "incorrect
pendingObserves");

https://codereview.chromium.org/2459113002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/indexeddb/WebIDBObserverImpl.h (right):

https://codereview.chromium.org/2459113002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/indexeddb/WebIDBObserverImpl.h:29:
std::bitset<WebIDBOperationTypeCount>& operationTypes);
const

[dmurph, 2016-11-07 22:31:53]

dmurph@chromium.org changed reviewers:
+ esprehn@chromium.org

[dmurph, 2016-11-07 22:31:54]

Thanks!

+Elliot, can you take a look at
third_party/WebKit/Source/bindings/modules/v8/custom/V8IDBObserverCustom.cpp?

Thanks!
Daniel

https://codereview.chromium.org/2459113002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/storage/indexeddb/resources/observer-tests.js
(right):

https://codereview.chromium.org/2459113002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/storage/indexeddb/resources/observer-tests.js:206:
obs.observe(db, txn, {operationTypes: ['clear', 'put', 'add', 'delete']});
On 2016/11/07 21:55:16, cmumford wrote:
> Makes me wonder if we should have an "all" type.

Yeah -- I wanted it to directly reflect the function calls. Since this is a
low-level-ish api, I'm fine with being a little verbose with the advantage that
it's dead-obvious what is being observed.

https://codereview.chromium.org/2459113002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/storage/indexeddb/resources/observer-tests.js:239:
if (changeNumber > 1) {
On 2016/11/07 21:55:15, cmumford wrote:
> Nit: How about assert_less_than_equal(changeNumber, 1, "incorrect
> pendingObserves");

I was looking for that assert! ;) thanks!

https://codereview.chromium.org/2459113002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/indexeddb/WebIDBObserverImpl.h (right):

https://codereview.chromium.org/2459113002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/indexeddb/WebIDBObserverImpl.h:29:
std::bitset<WebIDBOperationTypeCount>& operationTypes);
On 2016/11/07 21:55:16, cmumford wrote:
> const 

Done.

[dmurph, 2016-11-07 22:32:11]

The CQ bit was checked by dmurph@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-07 22:33:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-07 22:52:48]

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

[commit-bot: I haz the power, 2016-11-07 22:52:49]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[esprehn, 2016-11-07 22:58:54]

lgtm

https://codereview.chromium.org/2459113002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/indexeddb/IDBObserver.cpp (right):

https://codereview.chromium.org/2459113002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/indexeddb/IDBObserver.cpp:54: if
(options.operationTypes().size() == 0) {
isEmpty()

https://codereview.chromium.org/2459113002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/indexeddb/WebIDBObserverImpl.h (right):

https://codereview.chromium.org/2459113002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/indexeddb/WebIDBObserverImpl.h:29: const
std::bitset<WebIDBOperationTypeCount>& operationTypes);
WebIDBOperationTypeCount is so low, you can also pass the bitset by value, it
should be the same as copying a size_t.

[dmurph, 2016-11-08 22:40:18]

https://codereview.chromium.org/2459113002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/indexeddb/IDBObserver.cpp (right):

https://codereview.chromium.org/2459113002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/indexeddb/IDBObserver.cpp:54: if
(options.operationTypes().size() == 0) {
On 2016/11/07 22:58:53, esprehn wrote:
> isEmpty()

Done.

https://codereview.chromium.org/2459113002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/indexeddb/WebIDBObserverImpl.h (right):

https://codereview.chromium.org/2459113002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/indexeddb/WebIDBObserverImpl.h:29: const
std::bitset<WebIDBOperationTypeCount>& operationTypes);
On 2016/11/07 22:58:53, esprehn wrote:
> WebIDBOperationTypeCount is so low, you can also pass the bitset by value, it
> should be the same as copying a size_t.

Done.

[dmurph, 2016-11-08 22:40:24]

The CQ bit was checked by dmurph@chromium.org

[dmurph, 2016-11-08 22:40:24]

The patchset sent to the CQ was uploaded after l-g-t-m from
cmumford@chromium.org, esprehn@chromium.org
Link to the patchset: https://codereview.chromium.org/2459113002/#ps60001
(title: "change bitset to pass by value")

[commit-bot: I haz the power, 2016-11-08 22:41:00]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-09 00:06:40]

Description was changed from

==========
[IDBObservers] Moving options from constructor to .observe call.

Per spec change in
https://github.com/WICG/indexed-db-observers/commit/5aa4abc288ef526801797d3a9....

I'm also adding more exceptions here, as per
https://github.com/WICG/indexed-db-observers/commit/7723586ef6e5663739ae4800f....

R=cmumford
BUG=655865
==========

to

==========
[IDBObservers] Moving options from constructor to .observe call.

Per spec change in
https://github.com/WICG/indexed-db-observers/commit/5aa4abc288ef526801797d3a9....

I'm also adding more exceptions here, as per
https://github.com/WICG/indexed-db-observers/commit/7723586ef6e5663739ae4800f....

R=cmumford
BUG=655865
==========

[commit-bot: I haz the power, 2016-11-09 00:06:41]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-11-09 00:12:31]

Description was changed from

==========
[IDBObservers] Moving options from constructor to .observe call.

Per spec change in
https://github.com/WICG/indexed-db-observers/commit/5aa4abc288ef526801797d3a9....

I'm also adding more exceptions here, as per
https://github.com/WICG/indexed-db-observers/commit/7723586ef6e5663739ae4800f....

R=cmumford
BUG=655865
==========

to

==========
[IDBObservers] Moving options from constructor to .observe call.

Per spec change in
https://github.com/WICG/indexed-db-observers/commit/5aa4abc288ef526801797d3a9....

I'm also adding more exceptions here, as per
https://github.com/WICG/indexed-db-observers/commit/7723586ef6e5663739ae4800f....

R=cmumford
BUG=655865

Committed: https://crrev.com/2b06152c63ff8df7b927a9ad7e5d13541aa377ea
Cr-Commit-Position: refs/heads/master@{#430779}
==========

[commit-bot: I haz the power, 2016-11-09 00:12:32]

Patchset 4 (id:??) landed as
https://crrev.com/2b06152c63ff8df7b927a9ad7e5d13541aa377ea
Cr-Commit-Position: refs/heads/master@{#430779}