Replace setIndexedDBClientCreateFunction madness with Supplements

Replace setIndexedDBClientCreateFunction madness with Supplements

IndexedDBClient was a special snowflake, created on demand using
a function pointer. Meanwhile, all of the other module clients
(that give access to web/ features like content settings) are
implemented using Supplements on Frame/Worker and minted during
context creation. This has led to bugs during unrelated refactoring
including races.

So just make IndexedDBClient a Supplement like the rest.

R=sigbjornf@opera.com,haraken@chromium.org
BUG=472754
Committed: https://crrev.com/da287954e2164abe44794f1e0ffb257d5a20cb97
Cr-Commit-Position: refs/heads/master@{#387117}

[jsbell, 2016-04-08 19:07:02]

haraken@, sof@ - please take a look?

https://codereview.chromium.org/1865213006/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/web/IndexedDBClientImpl.h (right):

https://codereview.chromium.org/1865213006/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/IndexedDBClientImpl.h:38: // TODO(haraken): Remove
IndexedDBClientImpl.
We can probably make comment less scary, given that this is now a common
pattern.

[sof, 2016-04-08 19:51:58]

lgtm, that's much better - what's there not to like about this piece of
clear-headed thinking?

https://codereview.chromium.org/1865213006/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/web/WorkerContentSettingsClient.cpp (right):

https://codereview.chromium.org/1865213006/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/WorkerContentSettingsClient.cpp:36: #include
"web/IndexedDBClientImpl.h"
nit: can be removed.

[jsbell, 2016-04-08 20:04:51]

https://codereview.chromium.org/1865213006/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/web/WorkerContentSettingsClient.cpp (right):

https://codereview.chromium.org/1865213006/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/WorkerContentSettingsClient.cpp:36: #include
"web/IndexedDBClientImpl.h"
On 2016/04/08 19:51:58, sof wrote:
> nit: can be removed.

Done.

[jsbell, 2016-04-08 21:39:20]

Description was changed from

==========
Replace setIndexedDBClientCreateFunction madness with Supplements

IndexedDBClient was a special snowflake, created on demand using
a function pointer. Meanwhile, all of the other module clients
(that give access to web/ features like content settings) are
implemented using Supplements on Frame/Worker and minted during
context creation. This has led to bugs during unrelated refactoring
including races.

So just make IndexedDBClient a Supplement like the rest.

R=sigbjornf@opera.com,haraken@chromium.org
==========

to

==========
Replace setIndexedDBClientCreateFunction madness with Supplements

IndexedDBClient was a special snowflake, created on demand using
a function pointer. Meanwhile, all of the other module clients
(that give access to web/ features like content settings) are
implemented using Supplements on Frame/Worker and minted during
context creation. This has led to bugs during unrelated refactoring
including races.

So just make IndexedDBClient a Supplement like the rest.

R=sigbjornf@opera.com,haraken@chromium.org
BUG=472754
==========

[haraken, 2016-04-09 01:12:40]

LGTM, this is a nice clean-up!

https://codereview.chromium.org/1865213006/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/IndexedDBClientImpl.h (right):

https://codereview.chromium.org/1865213006/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/IndexedDBClientImpl.h:38: // TODO(haraken): Remove
IndexedDBClientImpl.

You can now remove this comment :)

https://codereview.chromium.org/1865213006/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/IndexedDBClientImpl.h:44: class
IndexedDBClientImpl final : public
GarbageCollectedFinalized<IndexedDBClientImpl>, public IndexedDBClient {

This will work, but it may be a bit cleaner to make IndexedDBClient
GarbageCollectedFinalized (and move USING_GARBAGE_COLLECTED_MIXIN to
IndexedDBClient)?

[jsbell, 2016-04-13 18:57:49]

The CQ bit was checked by jsbell@chromium.org

[jsbell, 2016-04-13 18:57:50]

The patchset sent to the CQ was uploaded after l-g-t-m from sigbjornf@opera.com,
haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/1865213006/#ps40001
(title: "Simplify per review feedback")

[jsbell, 2016-04-13 18:57:58]

Thanks!

https://codereview.chromium.org/1865213006/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/IndexedDBClientImpl.h (right):

https://codereview.chromium.org/1865213006/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/IndexedDBClientImpl.h:38: // TODO(haraken): Remove
IndexedDBClientImpl.
On 2016/04/09 01:12:40, haraken wrote:
> 
> You can now remove this comment :)

Done.

https://codereview.chromium.org/1865213006/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/IndexedDBClientImpl.h:44: class
IndexedDBClientImpl final : public
GarbageCollectedFinalized<IndexedDBClientImpl>, public IndexedDBClient {
On 2016/04/09 01:12:40, haraken wrote:
> 
> This will work, but it may be a bit cleaner to make IndexedDBClient
> GarbageCollectedFinalized (and move USING_GARBAGE_COLLECTED_MIXIN to
> IndexedDBClient)?
> 

Done. Also lets me re-inline the IndexedDBClientImpl ctor.

[commit-bot: I haz the power, 2016-04-13 18:58:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1865213006/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1865213006/40001

[commit-bot: I haz the power, 2016-04-13 19:11:19]

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

[commit-bot: I haz the power, 2016-04-13 19:11:20]

Try jobs failed on following builders:
  android_chromium_gn_compile_rel on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_chro...)

[jsbell, 2016-04-13 21:53:10]

The CQ bit was checked by jsbell@chromium.org

[commit-bot: I haz the power, 2016-04-13 21:53:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1865213006/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1865213006/40001

[commit-bot: I haz the power, 2016-04-13 22:15:53]

Description was changed from

==========
Replace setIndexedDBClientCreateFunction madness with Supplements

IndexedDBClient was a special snowflake, created on demand using
a function pointer. Meanwhile, all of the other module clients
(that give access to web/ features like content settings) are
implemented using Supplements on Frame/Worker and minted during
context creation. This has led to bugs during unrelated refactoring
including races.

So just make IndexedDBClient a Supplement like the rest.

R=sigbjornf@opera.com,haraken@chromium.org
BUG=472754
==========

to

==========
Replace setIndexedDBClientCreateFunction madness with Supplements

IndexedDBClient was a special snowflake, created on demand using
a function pointer. Meanwhile, all of the other module clients
(that give access to web/ features like content settings) are
implemented using Supplements on Frame/Worker and minted during
context creation. This has led to bugs during unrelated refactoring
including races.

So just make IndexedDBClient a Supplement like the rest.

R=sigbjornf@opera.com,haraken@chromium.org
BUG=472754
==========

[commit-bot: I haz the power, 2016-04-13 22:15:54]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-04-13 22:17:37]

Description was changed from

==========
Replace setIndexedDBClientCreateFunction madness with Supplements

IndexedDBClient was a special snowflake, created on demand using
a function pointer. Meanwhile, all of the other module clients
(that give access to web/ features like content settings) are
implemented using Supplements on Frame/Worker and minted during
context creation. This has led to bugs during unrelated refactoring
including races.

So just make IndexedDBClient a Supplement like the rest.

R=sigbjornf@opera.com,haraken@chromium.org
BUG=472754
==========

to

==========
Replace setIndexedDBClientCreateFunction madness with Supplements

IndexedDBClient was a special snowflake, created on demand using
a function pointer. Meanwhile, all of the other module clients
(that give access to web/ features like content settings) are
implemented using Supplements on Frame/Worker and minted during
context creation. This has led to bugs during unrelated refactoring
including races.

So just make IndexedDBClient a Supplement like the rest.

R=sigbjornf@opera.com,haraken@chromium.org
BUG=472754

Committed: https://crrev.com/da287954e2164abe44794f1e0ffb257d5a20cb97
Cr-Commit-Position: refs/heads/master@{#387117}
==========

[commit-bot: I haz the power, 2016-04-13 22:17:39]

Patchset 3 (id:??) landed as
https://crrev.com/da287954e2164abe44794f1e0ffb257d5a20cb97
Cr-Commit-Position: refs/heads/master@{#387117}