Moving Breakpad Windows specific logic to lower layer.

Moving Windows Breakpad restart logic to lower layer.

Currently this logic needs to be copied and called every time
chrome::AttemptRestart() is called. Moving it within chrome::AttemptRestart()
allows it to be reused. Moreover, it seems that calling
chrome::AttemptRestart() without the said logic (there are such occurrences),
is not correct.

This is in preparation of reducing the number of "Restart" codepaths in MD
Settings.

BUG=617830
Committed: https://crrev.com/615a9567426f289f43b9e3d614a646c59528ad93
Cr-Commit-Position: refs/heads/master@{#398195}

[dpapad, 2016-06-02 17:58:43]

Description was changed from

==========
Moving breakpad Windows specific logic to lower layer.

BUG=
==========

to

==========
Moving Windows breakpad restart logic to lower layer.

Currently this logic needs to be copied and called every time
chrome::AttemptRestart() is called. Moving it within chrome::AttemptRestart()
allows it to be reused.

This is in preparation of reducing "Restart" codepaths in MD Settings.
==========

[dpapad, 2016-06-02 18:06:55]

Description was changed from

==========
Moving Windows breakpad restart logic to lower layer.

Currently this logic needs to be copied and called every time
chrome::AttemptRestart() is called. Moving it within chrome::AttemptRestart()
allows it to be reused.

This is in preparation of reducing "Restart" codepaths in MD Settings.
==========

to

==========
Moving Windows Breakpad restart logic to lower layer.

Currently this logic needs to be copied and called every time
chrome::AttemptRestart() is called. Moving it within chrome::AttemptRestart()
allows it to be reused.

This is in preparation of reducing "Restart" codepaths in MD Settings.
==========

[dpapad, 2016-06-02 18:11:31]

Description was changed from

==========
Moving Windows Breakpad restart logic to lower layer.

Currently this logic needs to be copied and called every time
chrome::AttemptRestart() is called. Moving it within chrome::AttemptRestart()
allows it to be reused.

This is in preparation of reducing "Restart" codepaths in MD Settings.
==========

to

==========
Moving Windows Breakpad restart logic to lower layer.

Currently this logic needs to be copied and called every time
chrome::AttemptRestart() is called. Moving it within chrome::AttemptRestart()
allows it to be reused. Moreover, it seems that calling
chrome::AttemptRestart() without the said logic (there are such occurrences),
is not correct.

This is in preparation of reducing "Restart" codepaths in MD Settings.
==========

[dpapad, 2016-06-02 21:38:46]

Description was changed from

==========
Moving Windows Breakpad restart logic to lower layer.

Currently this logic needs to be copied and called every time
chrome::AttemptRestart() is called. Moving it within chrome::AttemptRestart()
allows it to be reused. Moreover, it seems that calling
chrome::AttemptRestart() without the said logic (there are such occurrences),
is not correct.

This is in preparation of reducing "Restart" codepaths in MD Settings.
==========

to

==========
Moving Windows Breakpad restart logic to lower layer.

Currently this logic needs to be copied and called every time
chrome::AttemptRestart() is called. Moving it within chrome::AttemptRestart()
allows it to be reused. Moreover, it seems that calling
chrome::AttemptRestart() without the said logic (there are such occurrences),
is not correct.

This is in preparation of reducing the number of"Restart" codepaths in MD
Settings.
==========

[dpapad, 2016-06-02 22:46:50]

dpapad@chromium.org changed reviewers:
+ siggi@chromium.org, thestig@chromium.org

[dpapad, 2016-06-02 22:48:01]

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

[dpapad, 2016-06-02 22:48:30]

Description was changed from

==========
Moving Windows Breakpad restart logic to lower layer.

Currently this logic needs to be copied and called every time
chrome::AttemptRestart() is called. Moving it within chrome::AttemptRestart()
allows it to be reused. Moreover, it seems that calling
chrome::AttemptRestart() without the said logic (there are such occurrences),
is not correct.

This is in preparation of reducing the number of"Restart" codepaths in MD
Settings.
==========

to

==========
Moving Windows Breakpad restart logic to lower layer.

Currently this logic needs to be copied and called every time
chrome::AttemptRestart() is called. Moving it within chrome::AttemptRestart()
allows it to be reused. Moreover, it seems that calling
chrome::AttemptRestart() without the said logic (there are such occurrences),
is not correct.

This is in preparation of reducing the number of "Restart" codepaths in MD
Settings.
==========

[Sigurður Ásgeirsson, 2016-06-06 13:40:01]

lgtm with a nit/request

https://codereview.chromium.org/2033173002/diff/60001/chrome/browser/lifetime...
File chrome/browser/lifetime/application_lifetime.cc (right):

https://codereview.chromium.org/2033173002/diff/60001/chrome/browser/lifetime...
chrome/browser/lifetime/application_lifetime.cc:205: typedef void (__cdecl
*ClearBreakpadPipeEnvVar)();
these in-place function type declarations irk me. I know you're just shuffling
code here, but perhaps you could pull these type declarations out to a common
header?
It's then possible to make the function type matching break at compile-time by
e.g. assigning the exported function to a variable of this type...

[dpapad, 2016-06-06 17:09:45]

https://codereview.chromium.org/2033173002/diff/60001/chrome/browser/lifetime...
File chrome/browser/lifetime/application_lifetime.cc (right):

https://codereview.chromium.org/2033173002/diff/60001/chrome/browser/lifetime...
chrome/browser/lifetime/application_lifetime.cc:205: typedef void (__cdecl
*ClearBreakpadPipeEnvVar)();
On 2016/06/06 at 13:40:01, Sigurður Ásgeirsson wrote:
> these in-place function type declarations irk me. I know you're just shuffling
code here, but perhaps you could pull these type declarations out to a common
header?
> It's then possible to make the function type matching break at compile-time by
e.g. assigning the exported function to a variable of this type...

I prefer to leave this cleanup to someone with a Windows workstation (and more
Windows dev experience). Since I don't have one, the development cycle is pretty
long (uploading my changes, triggering the trybots, waiting for the results).

[Lei Zhang, 2016-06-06 22:56:16]

lgtm

https://codereview.chromium.org/2033173002/diff/60001/chrome/browser/lifetime...
File chrome/browser/lifetime/application_lifetime.cc (right):

https://codereview.chromium.org/2033173002/diff/60001/chrome/browser/lifetime...
chrome/browser/lifetime/application_lifetime.cc:198: // On Windows Breakpad will
upload crash reports if the breakpad pipe name
there's a missing comma after "Windows"

[dpapad, 2016-06-06 23:14:37]

https://codereview.chromium.org/2033173002/diff/60001/chrome/browser/lifetime...
File chrome/browser/lifetime/application_lifetime.cc (right):

https://codereview.chromium.org/2033173002/diff/60001/chrome/browser/lifetime...
chrome/browser/lifetime/application_lifetime.cc:198: // On Windows Breakpad will
upload crash reports if the breakpad pipe name
On 2016/06/06 at 22:56:15, Lei Zhang (Slow) wrote:
> there's a missing comma after "Windows"

Done.

[Dan Beam, 2016-06-07 00:20:50]

lgtm but can you add a BUG= for this?

[dpapad, 2016-06-07 00:28:51]

Description was changed from

==========
Moving Windows Breakpad restart logic to lower layer.

Currently this logic needs to be copied and called every time
chrome::AttemptRestart() is called. Moving it within chrome::AttemptRestart()
allows it to be reused. Moreover, it seems that calling
chrome::AttemptRestart() without the said logic (there are such occurrences),
is not correct.

This is in preparation of reducing the number of "Restart" codepaths in MD
Settings.
==========

to

==========
Moving Windows Breakpad restart logic to lower layer.

Currently this logic needs to be copied and called every time
chrome::AttemptRestart() is called. Moving it within chrome::AttemptRestart()
allows it to be reused. Moreover, it seems that calling
chrome::AttemptRestart() without the said logic (there are such occurrences),
is not correct.

This is in preparation of reducing the number of "Restart" codepaths in MD
Settings.

BUG=617830
==========

[dpapad, 2016-06-07 00:29:17]

On 2016/06/07 at 00:20:50, dbeam wrote:
> lgtm but can you add a BUG= for this?

Done.

[dpapad, 2016-06-07 00:38:23]

The CQ bit was checked by dpapad@chromium.org

[dpapad, 2016-06-07 00:38:24]

The patchset sent to the CQ was uploaded after l-g-t-m from
thestig@chromium.org, siggi@chromium.org
Link to the patchset: https://codereview.chromium.org/2033173002/#ps80001
(title: "Address comment.")

[commit-bot: I haz the power, 2016-06-07 00:39:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2033173002/80001

[commit-bot: I haz the power, 2016-06-07 02:11:02]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-06-07 02:13:02]

Description was changed from

==========
Moving Windows Breakpad restart logic to lower layer.

Currently this logic needs to be copied and called every time
chrome::AttemptRestart() is called. Moving it within chrome::AttemptRestart()
allows it to be reused. Moreover, it seems that calling
chrome::AttemptRestart() without the said logic (there are such occurrences),
is not correct.

This is in preparation of reducing the number of "Restart" codepaths in MD
Settings.

BUG=617830
==========

to

==========
Moving Windows Breakpad restart logic to lower layer.

Currently this logic needs to be copied and called every time
chrome::AttemptRestart() is called. Moving it within chrome::AttemptRestart()
allows it to be reused. Moreover, it seems that calling
chrome::AttemptRestart() without the said logic (there are such occurrences),
is not correct.

This is in preparation of reducing the number of "Restart" codepaths in MD
Settings.

BUG=617830

Committed: https://crrev.com/615a9567426f289f43b9e3d614a646c59528ad93
Cr-Commit-Position: refs/heads/master@{#398195}
==========

[commit-bot: I haz the power, 2016-06-07 02:13:03]

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