Delete the stability file on clean exit.

Delete the stability file on clean exit.

The stability debugging experiment has Chrome store internal state to a
memory mapped file. This CL ensures the file gets deleted on clean exit.

Testing:
- Launch chrome --enable-features=StabilityDebugging --user-data-dir=foo
- Tested cases:
  - exiting by clicking the 'x'
  - browser rdv
  - the quick exit path, by sending WM_ENDSESSION to the top window

BUG=620813
Committed: https://crrev.com/14aff5d238aea71a0ca9b1439d7d06d1e2794c09
Cr-Commit-Position: refs/heads/master@{#434197}

[manzagop (departed), 2016-11-22 17:06:52]

Patchset #1 (id:1) has been deleted

[manzagop (departed), 2016-11-22 17:09:45]

manzagop@chromium.org changed reviewers:
+ bcwhite@chromium.org

[manzagop (departed), 2016-11-22 17:09:46]

As discussed! Please have a look.
Thanks!
Pierre

[manzagop (departed), 2016-11-22 17:11:08]

Description was changed from

==========
Delete the stability file on clean exit.

The stability debugging experiment has Chrome store internal state to a
memory mapped file. This CL ensures the file gets deleted on clean exit.

Testing:
- Launch chrome --enable-features=StabilityDebugging --user-data-dir=foo
- Tested cases:
  - exiting by clicking the 'x'
  - the quick exit path, by sending WM_ENDSESSION to the top window

BUG=620813
==========

to

==========
Delete the stability file on clean exit.

The stability debugging experiment has Chrome store internal state to a
memory mapped file. This CL ensures the file gets deleted on clean exit.

Testing:
- Launch chrome --enable-features=StabilityDebugging --user-data-dir=foo
- Tested cases:
  - exiting by clicking the 'x'
  - browser rdv
  - the quick exit path, by sending WM_ENDSESSION to the top window

BUG=620813
==========

[manzagop (departed), 2016-11-22 17:14:10]

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

[commit-bot: I haz the power, 2016-11-22 17:14: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-11-22 18:04:34]

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

[commit-bot: I haz the power, 2016-11-22 18:04:35]

Dry run: This issue passed the CQ dry run.

[bcwhite, 2016-11-22 20:32:15]

lgtm

https://codereview.chromium.org/2522593008/diff/20001/chrome/browser/metrics/...
File chrome/browser/metrics/chrome_metrics_service_client.cc (right):

https://codereview.chromium.org/2522593008/diff/20001/chrome/browser/metrics/...
chrome/browser/metrics/chrome_metrics_service_client.cc:264: DCHECK_NE(path,
nullptr);
Put the expected value first.  But for pointers you can just do:
DCHECK(path);

[manzagop (departed), 2016-11-22 21:06:08]

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

[manzagop (departed), 2016-11-22 21:06:37]

Thanks!

https://codereview.chromium.org/2522593008/diff/20001/chrome/browser/metrics/...
File chrome/browser/metrics/chrome_metrics_service_client.cc (right):

https://codereview.chromium.org/2522593008/diff/20001/chrome/browser/metrics/...
chrome/browser/metrics/chrome_metrics_service_client.cc:264: DCHECK_NE(path,
nullptr);
On 2016/11/22 20:32:15, bcwhite wrote:
> Put the expected value first.  But for pointers you can just do:
> DCHECK(path);

Kept the _NE version because I prefer the more verbose error message.

Done.

[commit-bot: I haz the power, 2016-11-22 21:06:41]

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

[manzagop (departed), 2016-11-22 21:07:36]

manzagop@chromium.org changed reviewers:
+ asvitkine@chromium.org

[manzagop (departed), 2016-11-22 21:07:37]

Hi Alexei!
This is to ensure clean exits don't leave the stability file behind. Could you
have an OWNERS' look?
Thanks!
Pierre

[Alexei Svitkine (slow), 2016-11-22 21:29:37]

https://codereview.chromium.org/2522593008/diff/40001/chrome/browser/metrics/...
File chrome/browser/metrics/chrome_metrics_service_client.cc (right):

https://codereview.chromium.org/2522593008/diff/40001/chrome/browser/metrics/...
chrome/browser/metrics/chrome_metrics_service_client.cc:429: #endif  // OS_WIN
Why not have all this logic live in browser_watcher:: ?

Seems unfortunate to leak all the details such as the enabled feature and file
path into ChromeMetricsServiceClient when they could live in a more specific
place.

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

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

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

Dry run: This issue passed the CQ dry run.

[manzagop (departed), 2016-11-22 22:16:09]

Thanks! Hopefully better now.

https://codereview.chromium.org/2522593008/diff/40001/chrome/browser/metrics/...
File chrome/browser/metrics/chrome_metrics_service_client.cc (right):

https://codereview.chromium.org/2522593008/diff/40001/chrome/browser/metrics/...
chrome/browser/metrics/chrome_metrics_service_client.cc:429: #endif  // OS_WIN
On 2016/11/22 21:29:37, Alexei Svitkine (slow) wrote:
> Why not have all this logic live in browser_watcher:: ?
> 
> Seems unfortunate to leak all the details such as the enabled feature and file
> path into ChromeMetricsServiceClient when they could live in a more specific
> place.

Done.

[manzagop (departed), 2016-11-22 22:16:22]

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

[commit-bot: I haz the power, 2016-11-22 22:16:55]

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

[Alexei Svitkine (slow), 2016-11-22 22:22:19]

lgtm

https://codereview.chromium.org/2522593008/diff/60001/chrome/browser/metrics/...
File chrome/browser/metrics/chrome_metrics_service_client.cc (right):

https://codereview.chromium.org/2522593008/diff/60001/chrome/browser/metrics/...
chrome/browser/metrics/chrome_metrics_service_client.cc:261: 
Nit: Remove from the diff

[commit-bot: I haz the power, 2016-11-22 23:26:55]

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

[commit-bot: I haz the power, 2016-11-22 23:26:56]

Dry run: This issue passed the CQ dry run.

[manzagop (departed), 2016-11-23 16:35:49]

Thanks for the reviews!

https://codereview.chromium.org/2522593008/diff/60001/chrome/browser/metrics/...
File chrome/browser/metrics/chrome_metrics_service_client.cc (right):

https://codereview.chromium.org/2522593008/diff/60001/chrome/browser/metrics/...
chrome/browser/metrics/chrome_metrics_service_client.cc:261: 
On 2016/11/22 22:22:19, Alexei Svitkine (slow) wrote:
> Nit: Remove from the diff

Done.

[manzagop (departed), 2016-11-23 16:35:54]

The CQ bit was checked by manzagop@chromium.org

[manzagop (departed), 2016-11-23 16:35:54]

The patchset sent to the CQ was uploaded after l-g-t-m from
bcwhite@chromium.org, asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/2522593008/#ps70001
(title: "Strip extra whitespace")

[commit-bot: I haz the power, 2016-11-23 16:36: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, 2016-11-23 17:27:03]

CQ is committing da patch.

Bot data: {"patchset_id": 70001, "attempt_start_ts": 1479918954400920,
"parent_rev": "c6d479715fdfd066d365848f6cbcb27b6412e949", "commit_rev":
"da662a0cac47366b678faf57692e13129aa8e976"}

[commit-bot: I haz the power, 2016-11-23 17:27:26]

Description was changed from

==========
Delete the stability file on clean exit.

The stability debugging experiment has Chrome store internal state to a
memory mapped file. This CL ensures the file gets deleted on clean exit.

Testing:
- Launch chrome --enable-features=StabilityDebugging --user-data-dir=foo
- Tested cases:
  - exiting by clicking the 'x'
  - browser rdv
  - the quick exit path, by sending WM_ENDSESSION to the top window

BUG=620813
==========

to

==========
Delete the stability file on clean exit.

The stability debugging experiment has Chrome store internal state to a
memory mapped file. This CL ensures the file gets deleted on clean exit.

Testing:
- Launch chrome --enable-features=StabilityDebugging --user-data-dir=foo
- Tested cases:
  - exiting by clicking the 'x'
  - browser rdv
  - the quick exit path, by sending WM_ENDSESSION to the top window

BUG=620813
==========

[commit-bot: I haz the power, 2016-11-23 17:27:28]

Committed patchset #4 (id:70001)

[commit-bot: I haz the power, 2016-11-23 17:31:02]

Description was changed from

==========
Delete the stability file on clean exit.

The stability debugging experiment has Chrome store internal state to a
memory mapped file. This CL ensures the file gets deleted on clean exit.

Testing:
- Launch chrome --enable-features=StabilityDebugging --user-data-dir=foo
- Tested cases:
  - exiting by clicking the 'x'
  - browser rdv
  - the quick exit path, by sending WM_ENDSESSION to the top window

BUG=620813
==========

to

==========
Delete the stability file on clean exit.

The stability debugging experiment has Chrome store internal state to a
memory mapped file. This CL ensures the file gets deleted on clean exit.

Testing:
- Launch chrome --enable-features=StabilityDebugging --user-data-dir=foo
- Tested cases:
  - exiting by clicking the 'x'
  - browser rdv
  - the quick exit path, by sending WM_ENDSESSION to the top window

BUG=620813

Committed: https://crrev.com/14aff5d238aea71a0ca9b1439d7d06d1e2794c09
Cr-Commit-Position: refs/heads/master@{#434197}
==========

[commit-bot: I haz the power, 2016-11-23 17:31:03]

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