MD Settings: Fix initialization timing of browser proxy in change-picture.

MD Settings: Fix initialization timing of browser proxy in change-picture.

BUG=714554
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2841653002
Cr-Commit-Position: refs/heads/master@{#466710}
Committed: https://chromium.googlesource.com/chromium/src/+/29374c5d9bb82e3bb15274d1e049785a1d785e68

[dpapad, 2017-04-24 17:41:12]

Description was changed from

==========
MD Settings: Fix initialization timing of browser proxy in change-picture.

BUG=714554
==========

to

==========
MD Settings: Fix initialization timing of browser proxy in change-picture.

BUG=714554
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[dpapad, 2017-04-24 17:57:03]

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

[commit-bot: I haz the power, 2017-04-24 17:57:55]

Dry run: CQ is trying da patch.

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

[dpapad, 2017-04-24 18:07:03]

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

[dpapad, 2017-04-24 18:07:03]

FYI, I investigated a bit on why the existing tests did not catch this bug.
Interesting findings, there might be an actual bug hiding in the code,
explaining below.

 - WebUI event listeners are registered in the element's attached callback.
 - browserProxy.initialize() is called from currentRouteChanged, which is called
before attached() has been called, and triggers C++ events.
 - It seems that there could be a time window where the C++ side fires a bunch
of events, but the element has not regisetered listeners for those events yet (I
can actually simulate this happening in the tests by swapping the two
highlighted lines at [1]).

[1]
https://cs.chromium.org/chromium/src/chrome/test/data/webui/settings/people_p...

Eitherway, eliminating the potential race condition is out of the scope of this
CL, I just happened to discover it because I wondered why the tests did not
already catch this regression.

[stevenjb, 2017-04-24 18:41:47]

Yeah, unfortunately testing timing issues is tricky at best. Thanks for
investigating further, I'm happy to discuss or review additional changes going
forward. In the meanwhile, this LGTM.

[dpapad, 2017-04-24 19:04:46]

The CQ bit was unchecked by dpapad@chromium.org

[dpapad, 2017-04-24 19:04:47]

The CQ bit was checked by dpapad@chromium.org

[commit-bot: I haz the power, 2017-04-24 19:05:11]

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, 2017-04-24 19:17:47]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1493060687599900, "parent_rev":
"d5b4dbdb62c46e7d2617d812b7fa9c5d201261fe", "commit_rev":
"29374c5d9bb82e3bb15274d1e049785a1d785e68"}

[commit-bot: I haz the power, 2017-04-24 19:17:56]

Description was changed from

==========
MD Settings: Fix initialization timing of browser proxy in change-picture.

BUG=714554
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: Fix initialization timing of browser proxy in change-picture.

BUG=714554
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2841653002
Cr-Commit-Position: refs/heads/master@{#466710}
Committed:
https://chromium.googlesource.com/chromium/src/+/29374c5d9bb82e3bb15274d1e049...
==========

[commit-bot: I haz the power, 2017-04-24 19:17:57]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/29374c5d9bb82e3bb15274d1e049...