Cleanup: Remove VersionUpdater::RelaunchBrowser interface method.

Cleanup: Remove VersionUpdater::RelaunchBrowser interface method.

Specifically:
 - Introducing a chrome::AttemptRelaunch() method.
 - Removing VersionUpdater::RelaunchBrowser() method, since the extra
   level of indirection seems unnecessary.

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

BUG=617830
Committed: https://crrev.com/374368fd0a260c0d13cab0447ba4b30c36d448b6
Cr-Commit-Position: refs/heads/master@{#398777}

[dpapad, 2016-06-06 19:21:05]

Description was changed from

==========
Cleanup: Remove VersionUpdater::RelaunchBrowser interface method.

The extra level of indirection seems unnecessary.

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

to

==========
Cleanup: Remove VersionUpdater::RelaunchBrowser interface method.

The extra level of indirection seems unnecessary, after introducing
a chrome::AttemptRelaunch() method.
==========

[dpapad, 2016-06-06 20:32:27]

Description was changed from

==========
Cleanup: Remove VersionUpdater::RelaunchBrowser interface method.

The extra level of indirection seems unnecessary, after introducing
a chrome::AttemptRelaunch() method.
==========

to

==========
Cleanup: Remove VersionUpdater::RelaunchBrowser interface method.

Specifically:
 - Introducing a chrome::AttemptRelaunch() method.
 - Removing VersionUpdater::RelaunchBrowser() method, since the extra
   level of indirection seems unnecessary.
==========

[dpapad, 2016-06-06 21:34:23]

Description was changed from

==========
Cleanup: Remove VersionUpdater::RelaunchBrowser interface method.

Specifically:
 - Introducing a chrome::AttemptRelaunch() method.
 - Removing VersionUpdater::RelaunchBrowser() method, since the extra
   level of indirection seems unnecessary.
==========

to

==========
Cleanup: Remove VersionUpdater::RelaunchBrowser interface method.

Specifically:
 - Introducing a chrome::AttemptRelaunch() method.
 - Removing VersionUpdater::RelaunchBrowser() method, since the extra
   level of indirection seems unnecessary.

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

[dpapad, 2016-06-06 22:07:14]

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

[dpapad, 2016-06-06 22:09:38]

Description was changed from

==========
Cleanup: Remove VersionUpdater::RelaunchBrowser interface method.

Specifically:
 - Introducing a chrome::AttemptRelaunch() method.
 - Removing VersionUpdater::RelaunchBrowser() method, since the extra
   level of indirection seems unnecessary.

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

to

==========
Cleanup: Remove VersionUpdater::RelaunchBrowser interface method.

Specifically:
 - Introducing a chrome::AttemptRelaunch() method.
 - Removing VersionUpdater::RelaunchBrowser() method, since the extra
   level of indirection seems unnecessary.

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

BUG=603625
==========

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

Description was changed from

==========
Cleanup: Remove VersionUpdater::RelaunchBrowser interface method.

Specifically:
 - Introducing a chrome::AttemptRelaunch() method.
 - Removing VersionUpdater::RelaunchBrowser() method, since the extra
   level of indirection seems unnecessary.

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

BUG=603625
==========

to

==========
Cleanup: Remove VersionUpdater::RelaunchBrowser interface method.

Specifically:
 - Introducing a chrome::AttemptRelaunch() method.
 - Removing VersionUpdater::RelaunchBrowser() method, since the extra
   level of indirection seems unnecessary.

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

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

[Lei Zhang, 2016-06-07 00:33:54]

In a previous code review involving application_lifetime.h, there was some
unhappiness about it being littered with a semi-related collection of functions.
Can we not add to it?

How about we make VersionUpdater::RelaunchBrowser() a static function instead,
and move the AttemptRelaunch() code into it?

[dpapad, 2016-06-07 01:27:12]

On 2016/06/07 at 00:33:54, thestig wrote:
> In a previous code review involving application_lifetime.h, there was some
unhappiness about it being littered with a semi-related collection of functions.
Can we not add to it?
> 
> How about we make VersionUpdater::RelaunchBrowser() a static function instead,
and move the AttemptRelaunch() code into it?

Looking through the repository history, version_updater.h was predates
application_lifetime.h by four months, see [1] and [2]. The fact that
VersionUpdater interface has a RelaunchBrowser method seems a remnant of the
state of the code at the time, and I can't fully justify keeping it there,
instead of putting it in where it seems it belongs semantically
(application_lifetime.h). Do you remember who had concerns about adding more
methods in application_lifetime.h? Perhaps is worth asking before preemptively
keeping it in VersionUpdater as a static method.

[1] https://chromiumcodereview.appspot.com/9320056
[2] https://chromiumcodereview.appspot.com/10409022

[dpapad, 2016-06-09 00:36:42]

On 2016/06/07 at 01:27:12, dpapad wrote:
> On 2016/06/07 at 00:33:54, thestig wrote:
> > In a previous code review involving application_lifetime.h, there was some
unhappiness about it being littered with a semi-related collection of functions.
Can we not add to it?
> > 
> > How about we make VersionUpdater::RelaunchBrowser() a static function
instead, and move the AttemptRelaunch() code into it?
> 
> Looking through the repository history, version_updater.h was predates
application_lifetime.h by four months, see [1] and [2]. The fact that
VersionUpdater interface has a RelaunchBrowser method seems a remnant of the
state of the code at the time, and I can't fully justify keeping it there,
instead of putting it in where it seems it belongs semantically
(application_lifetime.h). Do you remember who had concerns about adding more
methods in application_lifetime.h? Perhaps is worth asking before preemptively
keeping it in VersionUpdater as a static method.
> 
> [1] https://chromiumcodereview.appspot.com/9320056
> [2] https://chromiumcodereview.appspot.com/10409022

Friendly ping.

[Lei Zhang, 2016-06-09 00:49:09]

On 2016/06/07 01:27:12, dpapad wrote:
> On 2016/06/07 at 00:33:54, thestig wrote:
> > In a previous code review involving application_lifetime.h, there was some
> unhappiness about it being littered with a semi-related collection of
functions.
> Can we not add to it?
> > 
> > How about we make VersionUpdater::RelaunchBrowser() a static function
instead,
> and move the AttemptRelaunch() code into it?
> 
> Looking through the repository history, version_updater.h was predates
> application_lifetime.h by four months, see [1] and [2]. The fact that
> VersionUpdater interface has a RelaunchBrowser method seems a remnant of the
> state of the code at the time, and I can't fully justify keeping it there,
> instead of putting it in where it seems it belongs semantically
> (application_lifetime.h). Do you remember who had concerns about adding more
> methods in application_lifetime.h? Perhaps is worth asking before preemptively
> keeping it in VersionUpdater as a static method.

https://codereview.chromium.org/1382563002/#msg6

The file has been cleaned up a bit since then in separate CLs. I suppose
AttemptRelaunch() and AttemptRestart() closely related so they should stay near
each other, so ok.

[Lei Zhang, 2016-06-09 00:50:40]

https://codereview.chromium.org/2039273003/diff/1/chrome/browser/lifetime/app...
File chrome/browser/lifetime/application_lifetime.cc (right):

https://codereview.chromium.org/2039273003/diff/1/chrome/browser/lifetime/app...
chrome/browser/lifetime/application_lifetime.cc:216: #if defined(OS_CHROMEOS)
UpdateRecommendedMessageBox::Accept() is another place that calls these two
functions, but it differs. Maybe that version makes more sense?

[dpapad, 2016-06-09 01:14:27]

https://codereview.chromium.org/2039273003/diff/1/chrome/browser/lifetime/app...
File chrome/browser/lifetime/application_lifetime.cc (right):

https://codereview.chromium.org/2039273003/diff/1/chrome/browser/lifetime/app...
chrome/browser/lifetime/application_lifetime.cc:216: #if defined(OS_CHROMEOS)
On 2016/06/09 at 00:50:40, Lei Zhang (Slow) wrote:
> UpdateRecommendedMessageBox::Accept() is another place that calls these two
functions, but it differs. Maybe that version makes more sense?

Done, changed this to use UpdateRecommendedMessageBox's version. Also updated
UpdateRecommendedMessageBox to call chrome::AttemptRelaunch().

[Lei Zhang, 2016-06-09 01:18:57]

lgtm

[dpapad, 2016-06-09 01:22:59]

On 2016/06/09 at 01:18:57, thestig wrote:
> lgtm

Thank you!

[dpapad, 2016-06-09 01:32:24]

The CQ bit was checked by dpapad@chromium.org

[commit-bot: I haz the power, 2016-06-09 01:32:51]

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

[commit-bot: I haz the power, 2016-06-09 04:04:54]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-09 04:06:24]

Description was changed from

==========
Cleanup: Remove VersionUpdater::RelaunchBrowser interface method.

Specifically:
 - Introducing a chrome::AttemptRelaunch() method.
 - Removing VersionUpdater::RelaunchBrowser() method, since the extra
   level of indirection seems unnecessary.

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

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

to

==========
Cleanup: Remove VersionUpdater::RelaunchBrowser interface method.

Specifically:
 - Introducing a chrome::AttemptRelaunch() method.
 - Removing VersionUpdater::RelaunchBrowser() method, since the extra
   level of indirection seems unnecessary.

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

BUG=617830

Committed: https://crrev.com/374368fd0a260c0d13cab0447ba4b30c36d448b6
Cr-Commit-Position: refs/heads/master@{#398777}
==========

[commit-bot: I haz the power, 2016-06-09 04:06:25]

Patchset 2 (id:??) landed as
https://crrev.com/374368fd0a260c0d13cab0447ba4b30c36d448b6
Cr-Commit-Position: refs/heads/master@{#398777}