bluetooth: Fix BluetoothInternalsTest timeout issue

bluetooth: Fix BluetoothInternalsTest timeout issue

Timeouts caused by race condition where the web page executes before
the test has fully initialized the proxy objects has caused
BluetoothInternalsTest to fail.

To fix this problem, the proxy objects are fully initialized with a copy of the test data right after they are created in the test fixture
so that the order of execution doesn't matter.

BUG=667970
Committed: https://crrev.com/5a8c26b53389fc1e0717a529075f71d0e2b4bf91
Cr-Commit-Position: refs/heads/master@{#437644}

[mbrunson, 2016-12-01 19:15:36]

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

[commit-bot: I haz the power, 2016-12-01 19:15:55]

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-12-01 20:04:40]

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

[commit-bot: I haz the power, 2016-12-01 20:04:41]

Dry run: This issue passed the CQ dry run.

[mbrunson, 2016-12-01 20:50:36]

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

[commit-bot: I haz the power, 2016-12-01 20:51:05]

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-12-01 20:56:24]

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

[commit-bot: I haz the power, 2016-12-01 20:56:28]

Dry run: This issue passed the CQ dry run.

[mbrunson, 2016-12-02 18:35:03]

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

[commit-bot: I haz the power, 2016-12-02 18:35:35]

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-12-02 19:18:10]

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

[commit-bot: I haz the power, 2016-12-02 19:18:11]

Dry run: This issue passed the CQ dry run.

[mbrunson, 2016-12-02 20:55:29]

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

[commit-bot: I haz the power, 2016-12-02 20:56:33]

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-12-02 21:57:02]

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

[commit-bot: I haz the power, 2016-12-02 21:57:03]

Dry run: This issue passed the CQ dry run.

[mbrunson, 2016-12-07 20:14:55]

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

[commit-bot: I haz the power, 2016-12-07 20:15:33]

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

[mbrunson, 2016-12-07 21:26:40]

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

[commit-bot: I haz the power, 2016-12-07 21:27:07]

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

[mbrunson, 2016-12-07 21:28:59]

Description was changed from

==========
bluetooth: Fix chrome://bluetooth-internals timeout issue

Timeouts caused by race condition where the web page executes before the test
has fully initialized the proxy objects.


BUG=667970
==========

to

==========
bluetooth: Fix BluetoothInternalsTest timeout issue

Timeouts caused by race condition where the web page executes before the test
has fully initialized the proxy objects.

A set of Promises to enforce the proper initialization sequence has
been added in the TestAdapterProxy. These Promises block in the
getter until the setter has been called.


BUG=667970
==========

[mbrunson, 2016-12-07 21:29:19]

Description was changed from

==========
bluetooth: Fix BluetoothInternalsTest timeout issue

Timeouts caused by race condition where the web page executes before the test
has fully initialized the proxy objects.

A set of Promises to enforce the proper initialization sequence has
been added in the TestAdapterProxy. These Promises block in the
getter until the setter has been called.


BUG=667970
==========

to

==========
bluetooth: Fix BluetoothInternalsTest timeout issue

Timeouts caused by race condition where the web page executes before the test
has fully initialized the proxy objects.

A set of Promises to enforce the proper initialization sequence has
been added in the TestAdapterProxy. These Promises block in each
getter until the corresponding setter has been called.


BUG=667970
==========

[mbrunson, 2016-12-07 22:32:39]

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

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

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

[mbrunson, 2016-12-07 23:26:50]

mbrunson@chromium.org changed reviewers:
+ dpapad@chromium.org, ortuno@chromium.org, scheib@chromium.org

[commit-bot: I haz the power, 2016-12-07 23:26:52]

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

[commit-bot: I haz the power, 2016-12-07 23:26:52]

Dry run: This issue passed the CQ dry run.

[mbrunson, 2016-12-08 04:28:38]

mbrunson@chromium.org changed reviewers:
- dpapad@chromium.org

[ortuno, 2016-12-08 05:37:31]

yay! now we don't need PromiseResolver :). lgtm bar some whitespace issues and a
fix in the description.

https://codereview.chromium.org/2532383005/diff/60001/chrome/test/data/webui/...
File chrome/test/data/webui/bluetooth_internals_browsertest.js (right):

https://codereview.chromium.org/2532383005/diff/60001/chrome/test/data/webui/...
chrome/test/data/webui/bluetooth_internals_browsertest.js:125: return
Promise.resolve({info: this.adapterInfo_});
nit: tabbing.

https://codereview.chromium.org/2532383005/diff/60001/chrome/test/data/webui/...
chrome/test/data/webui/bluetooth_internals_browsertest.js:130: return
Promise.resolve({devices: this.devices_});
nit: tabbing.

[scheib, 2016-12-08 06:20:22]

LGTM

[mbrunson, 2016-12-08 19:30:58]

Description was changed from

==========
bluetooth: Fix BluetoothInternalsTest timeout issue

Timeouts caused by race condition where the web page executes before the test
has fully initialized the proxy objects.

A set of Promises to enforce the proper initialization sequence has
been added in the TestAdapterProxy. These Promises block in each
getter until the corresponding setter has been called.


BUG=667970
==========

to

==========
bluetooth: Fix BluetoothInternalsTest timeout issue

Timeouts caused by race condition where the web page executes before
the test has fully initialized the proxy objects has caused
BluetoothInternalsTest to fail.

To fix this problem, the proxy objects are fully initialized with a copy of the
test data right after they are created in the test fixture
so that the order of execution doesn't matter.

BUG=667970
==========

[mbrunson, 2016-12-08 19:31:08]

https://codereview.chromium.org/2532383005/diff/60001/chrome/test/data/webui/...
File chrome/test/data/webui/bluetooth_internals_browsertest.js (right):

https://codereview.chromium.org/2532383005/diff/60001/chrome/test/data/webui/...
chrome/test/data/webui/bluetooth_internals_browsertest.js:125: return
Promise.resolve({info: this.adapterInfo_});
On 2016/12/08 05:37:31, ortuno wrote:
> nit: tabbing.

Done.

https://codereview.chromium.org/2532383005/diff/60001/chrome/test/data/webui/...
chrome/test/data/webui/bluetooth_internals_browsertest.js:130: return
Promise.resolve({devices: this.devices_});
On 2016/12/08 05:37:31, ortuno wrote:
> nit: tabbing.

Done.

[mbrunson, 2016-12-08 19:31:34]

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

[commit-bot: I haz the power, 2016-12-08 19:32:25]

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-12-08 20:52:15]

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

[commit-bot: I haz the power, 2016-12-08 20:52:16]

Dry run: This issue passed the CQ dry run.

[mbrunson, 2016-12-08 21:21:28]

mbrunson@chromium.org changed reviewers:
+ dpapad@chromium.org

[dpapad, 2016-12-08 22:36:15]

https://codereview.chromium.org/2532383005/diff/80001/chrome/test/data/webui/...
File chrome/test/data/webui/bluetooth_internals_browsertest.js (right):

https://codereview.chromium.org/2532383005/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/bluetooth_internals_browsertest.js:163:
this.setupResolver.resolve();
Instead of calling setTestDevices(), setTestAdapter() and resolve() here, how
about doing all that in suiteSetup()? This is the approach I am following at
https://codereview.chromium.org/2563743002, which fixes a similar race condition
(and I think it would result in slightly simpler code). WDYT?

https://codereview.chromium.org/2532383005/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/bluetooth_internals_browsertest.js:174: fakeAdapterInfo:
function() {
Does this have to be a function? Can it just be a member variable inside the
constructor (near line 19)? Similar question for fakeDeviceInfo1,2,3 below.

[mbrunson, 2016-12-09 00:08:23]

https://codereview.chromium.org/2532383005/diff/80001/chrome/test/data/webui/...
File chrome/test/data/webui/bluetooth_internals_browsertest.js (right):

https://codereview.chromium.org/2532383005/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/bluetooth_internals_browsertest.js:163:
this.setupResolver.resolve();
On 2016/12/08 22:36:15, dpapad wrote:
> Instead of calling setTestDevices(), setTestAdapter() and resolve() here, how
> about doing all that in suiteSetup()? This is the approach I am following at
> https://codereview.chromium.org/2563743002, which fixes a similar race
condition
> (and I think it would result in slightly simpler code). WDYT?

The order that my test has to execute is a little different from yours. Since
you only have one layer of Mojo interfaces, you can create browserProxy outside
of the setupFn. I have to create my proxy and its contained proxies inside the
setupFn and block the test until it's created.

If I moved the resolve and set calls, I would then have to block the  web page
code so I could set the test data before it's read. There's no way to do this
without putting more test code in the app or adding multiple Promise guards in
the test code. I figured this was the simpler alternative.

https://codereview.chromium.org/2532383005/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/bluetooth_internals_browsertest.js:174: fakeAdapterInfo:
function() {
On 2016/12/08 22:36:15, dpapad wrote:
> Does this have to be a function? Can it just be a member variable inside the
> constructor (near line 19)? Similar question for fakeDeviceInfo1,2,3 below.

The bluetooth-internals page is directly manipulating these objects so the
values tend to change throughout the tests. It saves some code in the tests to
have a function that vends copies of the same object rather than manually doing
it in the tests. It also more closely mimics the way objects are handed to the
web page code from Mojo as new objects.

[dpapad, 2016-12-09 00:53:15]

https://codereview.chromium.org/2532383005/diff/80001/chrome/test/data/webui/...
File chrome/test/data/webui/bluetooth_internals_browsertest.js (right):

https://codereview.chromium.org/2532383005/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/bluetooth_internals_browsertest.js:147:
adapter.AdapterFactory.name,
Indentation off by 2.

https://codereview.chromium.org/2532383005/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/bluetooth_internals_browsertest.js:152:
this.adapterFactory = new TestAdapterFactoryProxy();
I am slightly confused here by the naming.

1) TestAdapterFactoryProxy contains a TestAdapter
2) TestAdapter contains a TestAdapterProxy

The two "Proxy" terms in 1 and 2 seem to have different semantics. Anyway,
probably nothing to do here, just my lack of understanding of the complex
hierarchy.

https://codereview.chromium.org/2532383005/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/bluetooth_internals_browsertest.js:163:
this.setupResolver.resolve();
On 2016/12/09 at 00:08:22, mbrunson wrote:
> On 2016/12/08 22:36:15, dpapad wrote:
> > Instead of calling setTestDevices(), setTestAdapter() and resolve() here,
how
> > about doing all that in suiteSetup()? This is the approach I am following at
> > https://codereview.chromium.org/2563743002, which fixes a similar race
condition
> > (and I think it would result in slightly simpler code). WDYT?
> 
> The order that my test has to execute is a little different from yours. Since
you only have one layer of Mojo interfaces, you can create browserProxy outside
of the setupFn. I have to create my proxy and its contained proxies inside the
setupFn and block the test until it's created.
> 
> If I moved the resolve and set calls, I would then have to block the  web page
code so I could set the test data before it's read. There's no way to do this
without putting more test code in the app or adding multiple Promise guards in
the test code. I figured this was the simpler alternative.

Thanks for the explanation. I think I sort-of understand the differences between
with the PluginsTest. How about this?

1) Create the 3 objects (TestAdapterFactoryProxy, TestAdapter, TestAdapterProxy)
involved in your tests first, without any references between them.
2) Return setupFnResolver.promise from setupFn (to block the prod code from
running).
3) Inside your suiteSetup() do someting as follows

suiteSetup(function() {
  // Prepare testAdapterProxy.
  testAdapterProxy.setFakeStuff();
  testAdapterProxy.setMoreFakeStuff();

  // Prepare testAdapter.
  testAdapter.setProxy(testAdapterProxy);

  // Prepare testAdapterFactory.
  testAdapterFactory.setAdapter(testAdapter);

  // Allow code being tested to proceed.
  setupFnResolver.resolve();

  return Promise.all([...]);
});

Pardon me if that suggestion would not work either. Just thought that this makes
the complex hierarchy more understandable (by creating it in one place).

https://codereview.chromium.org/2532383005/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/bluetooth_internals_browsertest.js:174: fakeAdapterInfo:
function() {
On 2016/12/09 at 00:08:22, mbrunson wrote:
> On 2016/12/08 22:36:15, dpapad wrote:
> > Does this have to be a function? Can it just be a member variable inside the
> > constructor (near line 19)? Similar question for fakeDeviceInfo1,2,3 below.
> 
> The bluetooth-internals page is directly manipulating these objects so the
values tend to change throughout the tests. It saves some code in the tests to
have a function that vends copies of the same object rather than manually doing
it in the tests. It also more closely mimics the way objects are handed to the
web page code from Mojo as new objects.

Makes sense.

[mbrunson, 2016-12-09 02:51:04]

https://codereview.chromium.org/2532383005/diff/80001/chrome/test/data/webui/...
File chrome/test/data/webui/bluetooth_internals_browsertest.js (right):

https://codereview.chromium.org/2532383005/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/bluetooth_internals_browsertest.js:147:
adapter.AdapterFactory.name,
On 2016/12/09 00:53:15, dpapad wrote:
> Indentation off by 2.

Done.

https://codereview.chromium.org/2532383005/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/bluetooth_internals_browsertest.js:152:
this.adapterFactory = new TestAdapterFactoryProxy();
On 2016/12/09 00:53:15, dpapad wrote:
> I am slightly confused here by the naming.
> 
> 1) TestAdapterFactoryProxy contains a TestAdapter
> 2) TestAdapter contains a TestAdapterProxy
> 
> The two "Proxy" terms in 1 and 2 seem to have different semantics. Anyway,
> probably nothing to do here, just my lack of understanding of the complex
> hierarchy.

The "Proxy" on the end refers to the fact that that class is a subclass of
settings.TestBrowserProxy. TestAdapter is a subclass of
adapter.Adapter.stubClass so it doesn't have "Proxy" on the end. TestAdapter has
to be a subclass of a Mojo-generated stub class so I can get a Mojo Handle for
the object which causes this weird hierarchy.

https://codereview.chromium.org/2532383005/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/bluetooth_internals_browsertest.js:163:
this.setupResolver.resolve();
On 2016/12/09 00:53:15, dpapad wrote:
> On 2016/12/09 at 00:08:22, mbrunson wrote:
> > On 2016/12/08 22:36:15, dpapad wrote:
> > > Instead of calling setTestDevices(), setTestAdapter() and resolve() here,
> how
> > > about doing all that in suiteSetup()? This is the approach I am following
at
> > > https://codereview.chromium.org/2563743002, which fixes a similar race
> condition
> > > (and I think it would result in slightly simpler code). WDYT?
> > 
> > The order that my test has to execute is a little different from yours.
Since
> you only have one layer of Mojo interfaces, you can create browserProxy
outside
> of the setupFn. I have to create my proxy and its contained proxies inside the
> setupFn and block the test until it's created.
> > 
> > If I moved the resolve and set calls, I would then have to block the  web
page
> code so I could set the test data before it's read. There's no way to do this
> without putting more test code in the app or adding multiple Promise guards in
> the test code. I figured this was the simpler alternative.
> 
> Thanks for the explanation. I think I sort-of understand the differences
between
> with the PluginsTest. How about this?
> 
> 1) Create the 3 objects (TestAdapterFactoryProxy, TestAdapter,
TestAdapterProxy)
> involved in your tests first, without any references between them.
> 2) Return setupFnResolver.promise from setupFn (to block the prod code from
> running).
> 3) Inside your suiteSetup() do someting as follows
> 
> suiteSetup(function() {
>   // Prepare testAdapterProxy.
>   testAdapterProxy.setFakeStuff();
>   testAdapterProxy.setMoreFakeStuff();
> 
>   // Prepare testAdapter.
>   testAdapter.setProxy(testAdapterProxy);
> 
>   // Prepare testAdapterFactory.
>   testAdapterFactory.setAdapter(testAdapter);
> 
>   // Allow code being tested to proceed.
>   setupFnResolver.resolve();
> 
>   return Promise.all([...]);
> });
> 
> Pardon me if that suggestion would not work either. Just thought that this
makes
> the complex hierarchy more understandable (by creating it in one place).

Where is testAdapter being created in this situation? It's required to be a
subclass of a Mojo-generated class which isn't available until setupFn runs.

May I suggest as a compromise, I move the setting of the test data and add a
setupFnResolver to the setupFn that gets resolved in suiteSetup().

That would give an execution order like this:
1) test is blocked on setupResolver (not setupFnResolver)
----(in setupFn)----
2) set the testAdapterFactory
3) resolve setupResolver, unblocking the test
4) setupFn blocks on setupFnResolver
----(in suiteSetup)----
5) set the test data since it now has access to testAdapterFactory
6) resolve setupFnResolver, unblocking setupFn
7) suiteSetup blocks on Promise.all
----(in setupFn)----
8) setupFn finishes, prod code runs
9) Promise.all is resolved
10) test finishes

I've made the change so you can see it. How does that sound?

[dpapad, 2016-12-09 03:44:45]

LGTM for patchset 5. See comment for more details (feel free to follow my
suggestion or stick with patchset 5). Thanks for bearing with me, trying to
simplify.

Ultimately this code is complicated because of having to load Mojo generated
code asynchronously. Mojo team has acknowledged that the right approach moving
forward is to include the Mojo generated code via <script> tags, just like any
other JS code, and remove the need for async loading. This will simplify things
a lot.

https://codereview.chromium.org/2532383005/diff/80001/chrome/test/data/webui/...
File chrome/test/data/webui/bluetooth_internals_browsertest.js (right):

https://codereview.chromium.org/2532383005/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/bluetooth_internals_browsertest.js:163:
this.setupResolver.resolve();
> I've made the change so you can see it. How does that sound?

Actually patchset 5 looks simpler than latest version patch 6. Having two
PromiseResolvers is unnecessarily complicated. Here is my version,
https://codereview.chromium.org/2552883013 (tested locally and tests pass).

Basically here are the main differences
1) Move setupFn() call in the prod code to the entry point (DOMContentLoaded),
instead of having it at interfaces.js. This guarantees that nothing runs until
that setupFnResolver is resolved.
2) Split setupFn in the test code to two methods,
 - testSetup() which returns a Promise when all test all Mojo related classes
have been defined and instantiated (no need for a PromiseResolver). suiteSetup()
calls this method.
 - setupFn() resolved after fake data has been set, inside suiteSetup().

[ortuno, 2016-12-09 03:44:55]

On 2016/12/09 at 02:51:04, mbrunson wrote:
>
https://codereview.chromium.org/2532383005/diff/80001/chrome/test/data/webui/...
> File chrome/test/data/webui/bluetooth_internals_browsertest.js (right):
> 
>
https://codereview.chromium.org/2532383005/diff/80001/chrome/test/data/webui/...
> chrome/test/data/webui/bluetooth_internals_browsertest.js:147:
adapter.AdapterFactory.name,
> On 2016/12/09 00:53:15, dpapad wrote:
> > Indentation off by 2.
> 
> Done.
> 
>
https://codereview.chromium.org/2532383005/diff/80001/chrome/test/data/webui/...
> chrome/test/data/webui/bluetooth_internals_browsertest.js:152:
this.adapterFactory = new TestAdapterFactoryProxy();
> On 2016/12/09 00:53:15, dpapad wrote:
> > I am slightly confused here by the naming.
> > 
> > 1) TestAdapterFactoryProxy contains a TestAdapter
> > 2) TestAdapter contains a TestAdapterProxy
> > 
> > The two "Proxy" terms in 1 and 2 seem to have different semantics. Anyway,
> > probably nothing to do here, just my lack of understanding of the complex
> > hierarchy.
> 
> The "Proxy" on the end refers to the fact that that class is a subclass of
settings.TestBrowserProxy. TestAdapter is a subclass of
adapter.Adapter.stubClass so it doesn't have "Proxy" on the end. TestAdapter has
to be a subclass of a Mojo-generated stub class so I can get a Mojo Handle for
the object which causes this weird hierarchy.
> 
>
https://codereview.chromium.org/2532383005/diff/80001/chrome/test/data/webui/...
> chrome/test/data/webui/bluetooth_internals_browsertest.js:163:
this.setupResolver.resolve();
> On 2016/12/09 00:53:15, dpapad wrote:
> > On 2016/12/09 at 00:08:22, mbrunson wrote:
> > > On 2016/12/08 22:36:15, dpapad wrote:
> > > > Instead of calling setTestDevices(), setTestAdapter() and resolve()
here,
> > how
> > > > about doing all that in suiteSetup()? This is the approach I am
following at
> > > > https://codereview.chromium.org/2563743002, which fixes a similar race
> > condition
> > > > (and I think it would result in slightly simpler code). WDYT?
> > > 
> > > The order that my test has to execute is a little different from yours.
Since
> > you only have one layer of Mojo interfaces, you can create browserProxy
outside
> > of the setupFn. I have to create my proxy and its contained proxies inside
the
> > setupFn and block the test until it's created.
> > > 
> > > If I moved the resolve and set calls, I would then have to block the  web
page
> > code so I could set the test data before it's read. There's no way to do
this
> > without putting more test code in the app or adding multiple Promise guards
in
> > the test code. I figured this was the simpler alternative.
> > 
> > Thanks for the explanation. I think I sort-of understand the differences
between
> > with the PluginsTest. How about this?
> > 
> > 1) Create the 3 objects (TestAdapterFactoryProxy, TestAdapter,
TestAdapterProxy)
> > involved in your tests first, without any references between them.
> > 2) Return setupFnResolver.promise from setupFn (to block the prod code from
> > running).
> > 3) Inside your suiteSetup() do someting as follows
> > 
> > suiteSetup(function() {
> >   // Prepare testAdapterProxy.
> >   testAdapterProxy.setFakeStuff();
> >   testAdapterProxy.setMoreFakeStuff();
> > 
> >   // Prepare testAdapter.
> >   testAdapter.setProxy(testAdapterProxy);
> > 
> >   // Prepare testAdapterFactory.
> >   testAdapterFactory.setAdapter(testAdapter);
> > 
> >   // Allow code being tested to proceed.
> >   setupFnResolver.resolve();
> > 
> >   return Promise.all([...]);
> > });
> > 
> > Pardon me if that suggestion would not work either. Just thought that this
makes
> > the complex hierarchy more understandable (by creating it in one place).
> 
> Where is testAdapter being created in this situation? It's required to be a
subclass of a Mojo-generated class which isn't available until setupFn runs.
> 
> May I suggest as a compromise, I move the setting of the test data and add a
setupFnResolver to the setupFn that gets resolved in suiteSetup().
> 
> That would give an execution order like this:
> 1) test is blocked on setupResolver (not setupFnResolver)
> ----(in setupFn)----
> 2) set the testAdapterFactory
> 3) resolve setupResolver, unblocking the test
> 4) setupFn blocks on setupFnResolver
> ----(in suiteSetup)----
> 5) set the test data since it now has access to testAdapterFactory
> 6) resolve setupFnResolver, unblocking setupFn
> 7) suiteSetup blocks on Promise.all
> ----(in setupFn)----
> 8) setupFn finishes, prod code runs
> 9) Promise.all is resolved
> 10) test finishes
> 
> I've made the change so you can see it. How does that sound?

If the goal is to have everything defined in one place then I'm not sure this
helps much.

Please correct me if I'm wrong but it seems that there are two dependencies:

1. Adapter needs to be setup before the page calls getAdapter().
2. The page needs to get the adapter and set up the table before the test
starts.

Could we make both part of suiteSetup?

preLoad: function() {
  window.setupFn = setupFnResolver.promise;
}

suiteSetup: function() {
  return new Promise(resolve => {
    importModules(...).then(... => {
      // Set up fake classes
      // ...
      
     
frameInterfaces.addInterfaceOverrideForTesting(adapter.AdapterFactory.name,
(handle) => {
        var stub = connection.bindHandleToStub(handle, adapter.AdapterFactory);
        this.adapterFactory = new TestAdapterFactoryProxy();
        bindings.StubBindings(stub).delegate = this.adapterFactory;

        // Add fake devices and adapter to fake adapter proxy.
        // ...

        // This handles 2.
        resolve();
      });

      // This handles 1.
      setupFnResolver.resolver();
    });
  });
}
mbrunson: Would something like that make sense?
dpapad: What do you think of this approach?

[ortuno, 2016-12-09 03:53:09]

lgtm

Ah dpapad's comment and mine collided. I'm OK with keeping Patchset 5 if dpapad
is happy :)

[mbrunson, 2016-12-09 18:09:36]

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

[commit-bot: I haz the power, 2016-12-09 18:10:01]

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-12-09 20:19:20]

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

[commit-bot: I haz the power, 2016-12-09 20:19:21]

Dry run: This issue passed the CQ dry run.

[mbrunson, 2016-12-09 20:29:04]

The CQ bit was checked by mbrunson@chromium.org

[mbrunson, 2016-12-09 20:29:04]

The patchset sent to the CQ was uploaded after l-g-t-m from scheib@chromium.org,
ortuno@chromium.org, dpapad@chromium.org
Link to the patchset: https://codereview.chromium.org/2532383005/#ps120001
(title: "Revert patch set 6, fix indentation")

[commit-bot: I haz the power, 2016-12-09 20:30:03]

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-12-09 20:56:41]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1481315344626240,
"parent_rev": "73387076f2eca1caaefbe5f98e99563e59d3ec2b", "commit_rev":
"09157acccfdcca4165df425665631be329415420"}

[commit-bot: I haz the power, 2016-12-09 20:57:10]

Description was changed from

==========
bluetooth: Fix BluetoothInternalsTest timeout issue

Timeouts caused by race condition where the web page executes before
the test has fully initialized the proxy objects has caused
BluetoothInternalsTest to fail.

To fix this problem, the proxy objects are fully initialized with a copy of the
test data right after they are created in the test fixture
so that the order of execution doesn't matter.

BUG=667970
==========

to

==========
bluetooth: Fix BluetoothInternalsTest timeout issue

Timeouts caused by race condition where the web page executes before
the test has fully initialized the proxy objects has caused
BluetoothInternalsTest to fail.

To fix this problem, the proxy objects are fully initialized with a copy of the
test data right after they are created in the test fixture
so that the order of execution doesn't matter.

BUG=667970

Review-Url: https://codereview.chromium.org/2532383005
==========

[commit-bot: I haz the power, 2016-12-09 20:57:12]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-12-12 14:40:33]

Description was changed from

==========
bluetooth: Fix BluetoothInternalsTest timeout issue

Timeouts caused by race condition where the web page executes before
the test has fully initialized the proxy objects has caused
BluetoothInternalsTest to fail.

To fix this problem, the proxy objects are fully initialized with a copy of the
test data right after they are created in the test fixture
so that the order of execution doesn't matter.

BUG=667970

Review-Url: https://codereview.chromium.org/2532383005
==========

to

==========
bluetooth: Fix BluetoothInternalsTest timeout issue

Timeouts caused by race condition where the web page executes before
the test has fully initialized the proxy objects has caused
BluetoothInternalsTest to fail.

To fix this problem, the proxy objects are fully initialized with a copy of the
test data right after they are created in the test fixture
so that the order of execution doesn't matter.

BUG=667970

Committed: https://crrev.com/5a8c26b53389fc1e0717a529075f71d0e2b4bf91
Cr-Commit-Position: refs/heads/master@{#437644}
==========

[commit-bot: I haz the power, 2016-12-12 14:40:34]

Patchset 7 (id:??) landed as
https://crrev.com/5a8c26b53389fc1e0717a529075f71d0e2b4bf91
Cr-Commit-Position: refs/heads/master@{#437644}