Fix an edge case where early disconnect() can omit delivering of observation. crbug/607324

Fix an edge case where early disconnect() can omit delivering of observation.

BUG=607324
Committed: https://crrev.com/32102c0baf205ff54cd16644ddd823a6cb6538f5
Cr-Commit-Position: refs/heads/master@{#392436}

[panicker, 2016-05-03 22:16:53]

panicker@chromium.org changed reviewers:
+ michaelblain@google.com

[panicker, 2016-05-03 22:16:53]

Hey Michael,
Can you take a quick look and sanity check.

[panicker, 2016-05-03 22:17:11]

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

[commit-bot: I haz the power, 2016-05-03 22:17:28]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1945953002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1945953002/1

[commit-bot: I haz the power, 2016-05-03 22:17:29]

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

[commit-bot: I haz the power, 2016-05-03 22:17:30]

Dry run: No L-G-T-M from a valid reviewer yet. 
CQ run can only be started by full committers or once the patch has
received an L-G-T-M from a full committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[MikeB, 2016-05-03 22:19:23]

On 2016/05/03 22:17:30, commit-bot: I haz the power wrote:
> Dry run: No L-G-T-M from a valid reviewer yet. 
> CQ run can only be started by full committers or once the patch has
> received an L-G-T-M from a full committer.
> Even if an L-G-T-M may have been provided, it was from a non-committer,
> _not_ a full super star committer.
> See http://www.chromium.org/getting-involved/become-a-committer
> Note that this has nothing to do with OWNERS files.

LGTM
Looks good, thanks.

[fmeawad, 2016-05-03 22:20:14]

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

[commit-bot: I haz the power, 2016-05-03 22:21:08]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1945953002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1945953002/1

[panicker, 2016-05-03 22:23:28]

panicker@chromium.org changed reviewers:
+ skobes@chromium.org

[panicker, 2016-05-03 22:23:28]

PTAL :)

[commit-bot: I haz the power, 2016-05-03 22:24:05]

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

[commit-bot: I haz the power, 2016-05-03 22:24:06]

Dry run: Try jobs failed on following builders:
  ios_rel_device_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_gn...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)

[skobes, 2016-05-03 22:51:05]

It would be nice to have a unit test for this.

[panicker, 2016-05-06 17:26:37]

On 2016/05/03 22:51:05, skobes wrote:
> It would be nice to have a unit test for this.

PTAL.
Added basic unittests for PerformanceBase.cpp
I will add unittests for PerformanceObserver.cpp as a follow up, because it
requires making some changes to make the code testable in the first place (eg.
its a final class with private members), I'll address that next.
Thanks for your help with setting this up!

[skobes, 2016-05-06 18:54:52]

LGTM

Nit: no need for crbug # in the first line of the description (that's what the
BUG= line is for).

[panicker, 2016-05-06 19:31:34]

Description was changed from

==========
Fix an edge case where early disconnect() can omit delivering of observation.
crbug/607324

BUG=607324
==========

to

==========
Fix an edge case where early disconnect() can omit delivering of observation.

BUG=607324
==========

[panicker, 2016-05-06 19:31:43]

On 2016/05/06 18:54:52, skobes wrote:
> LGTM
> 
> Nit: no need for crbug # in the first line of the description (that's what the
> BUG= line is for).

Done.

[panicker, 2016-05-06 19:32:09]

The CQ bit was checked by panicker@chromium.org

[panicker, 2016-05-06 19:32:09]

The patchset sent to the CQ was uploaded after l-g-t-m from mpb@chromium.org
Link to the patchset: https://codereview.chromium.org/1945953002/#ps20001
(title: "Added basic unittests for PerformanceBase")

[commit-bot: I haz the power, 2016-05-06 19:32:45]

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

[panicker, 2016-05-06 19:34:59]

The CQ bit was unchecked by panicker@chromium.org

[panicker, 2016-05-06 19:39:41]

The CQ bit was checked by panicker@chromium.org

[commit-bot: I haz the power, 2016-05-06 19:40:14]

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

[commit-bot: I haz the power, 2016-05-06 19:50:13]

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

[commit-bot: I haz the power, 2016-05-06 19:50:14]

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

[panicker, 2016-05-06 23:44:21]

The CQ bit was checked by panicker@chromium.org

[panicker, 2016-05-06 23:44:21]

The patchset sent to the CQ was uploaded after l-g-t-m from skobes@chromium.org,
mpb@chromium.org
Link to the patchset: https://codereview.chromium.org/1945953002/#ps40001
(title: "Add missing CORE_EXPORT to PerformanceObserver class")

[commit-bot: I haz the power, 2016-05-06 23:44:42]

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

[commit-bot: I haz the power, 2016-05-07 00:46:47]

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

[commit-bot: I haz the power, 2016-05-07 00:46:48]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[panicker, 2016-05-09 20:46:27]

The CQ bit was checked by panicker@chromium.org

[commit-bot: I haz the power, 2016-05-09 20:46:50]

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

[commit-bot: I haz the power, 2016-05-09 22:07:27]

Description was changed from

==========
Fix an edge case where early disconnect() can omit delivering of observation.

BUG=607324
==========

to

==========
Fix an edge case where early disconnect() can omit delivering of observation.

BUG=607324
==========

[commit-bot: I haz the power, 2016-05-09 22:07:28]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-05-09 22:08:39]

Description was changed from

==========
Fix an edge case where early disconnect() can omit delivering of observation.

BUG=607324
==========

to

==========
Fix an edge case where early disconnect() can omit delivering of observation.

BUG=607324

Committed: https://crrev.com/32102c0baf205ff54cd16644ddd823a6cb6538f5
Cr-Commit-Position: refs/heads/master@{#392436}
==========

[commit-bot: I haz the power, 2016-05-09 22:08:40]

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