Stop calling CommitPendingWriter() in LogCleanShutdown().

Stop calling CommitPendingWriter() in LogCleanShutdown().

PrefService's dtor is responsible to call CommitPendingWrite() so there is no need to call it here.

BUG=470501
Committed: https://crrev.com/a5c00e28d104b06b952aa2f1d9b4c21178cc4b71
Cr-Commit-Position: refs/heads/master@{#322535}

[hashimoto, 2015-03-26 10:35:49]

Patchset #1 (id:1) has been deleted

[hashimoto, 2015-03-26 10:42:08]

hashimoto@chromium.org changed reviewers:
+ holte@chromium.org

[hashimoto, 2015-03-26 10:42:08]

Steven, could you review this change?

The hack in LogCleanShutdown() was introduced by
https://codereview.chromium.org/7779036.
In the referred bug (crbug.com/95375), it is said that "... it also calls
RecordCurrentState().  That function, rather than perhaps doin a push of the
stat, proceeds to call RecordPluginChanges() ..."
However, MetricsService::RecordCurrentState() is not calling a function named
RecordPluginChanges() today.

I think it's OK to remove this hack now.

[Steven Holte, 2015-03-26 21:58:43]

On 2015/03/26 10:42:08, hashimoto wrote:
> Steven, could you review this change?
> 
> The hack in LogCleanShutdown() was introduced by
> https://codereview.chromium.org/7779036.
> In the referred bug (crbug.com/95375), it is said that "... it also calls
> RecordCurrentState().  That function, rather than perhaps doin a push of the
> stat, proceeds to call RecordPluginChanges() ..."
> However, MetricsService::RecordCurrentState() is not calling a function named
> RecordPluginChanges() today.
> 
> I think it's OK to remove this hack now.

Hm, I think the basic idea here is just to flush the pref change to disk.  Where
does the PrefService get destructed relative to here?  Are there any potential
delays?

[hashimoto, 2015-03-27 06:13:01]

On 2015/03/26 21:58:43, Steven Holte wrote:
> On 2015/03/26 10:42:08, hashimoto wrote:
> > Steven, could you review this change?
> > 
> > The hack in LogCleanShutdown() was introduced by
> > https://codereview.chromium.org/7779036.
> > In the referred bug (crbug.com/95375), it is said that "... it also calls
> > RecordCurrentState().  That function, rather than perhaps doin a push of the
> > stat, proceeds to call RecordPluginChanges() ..."
> > However, MetricsService::RecordCurrentState() is not calling a function
named
> > RecordPluginChanges() today.
> > 
> > I think it's OK to remove this hack now.
> 
> Hm, I think the basic idea here is just to flush the pref change to disk. 
Where
> does the PrefService get destructed relative to here?  Are there any potential
> delays?

I think there is no delay without CommitPendingWrite() in LogCleanShutdown().

LogCleanShutdown() is called by two functions:
MetricsService::RecordStartOfSessionEnd() and
MetricsService::RecordCompletedSessionEnd().

MetricsService::RecordStartOfSessionEnd() is called by
BrowserProcessImpl::EndSession(),
and it's immediately followed by local_state()->CommitPendingWrite().
(https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/bro...)

MetricsService::RecordCompletedSessionEnd() is called by
ShutdownPreThreadsStop() in browser_shutdown.cc,
and it's also followed by prefs->CommitPendingWrite().
(https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/bro...)

AFAIK in all code paths where LogCleanShutdown() gets called, we can expect the
pref change is flushed to disk with almost no delay.

[Steven Holte, 2015-03-27 06:17:56]

On 2015/03/27 06:13:01, hashimoto wrote:
> On 2015/03/26 21:58:43, Steven Holte wrote:
> > On 2015/03/26 10:42:08, hashimoto wrote:
> > > Steven, could you review this change?
> > > 
> > > The hack in LogCleanShutdown() was introduced by
> > > https://codereview.chromium.org/7779036.
> > > In the referred bug (crbug.com/95375), it is said that "... it also calls
> > > RecordCurrentState().  That function, rather than perhaps doin a push of
the
> > > stat, proceeds to call RecordPluginChanges() ..."
> > > However, MetricsService::RecordCurrentState() is not calling a function
> named
> > > RecordPluginChanges() today.
> > > 
> > > I think it's OK to remove this hack now.
> > 
> > Hm, I think the basic idea here is just to flush the pref change to disk. 
> Where
> > does the PrefService get destructed relative to here?  Are there any
potential
> > delays?
> 
> I think there is no delay without CommitPendingWrite() in LogCleanShutdown().
> 
> LogCleanShutdown() is called by two functions:
> MetricsService::RecordStartOfSessionEnd() and
> MetricsService::RecordCompletedSessionEnd().
> 
> MetricsService::RecordStartOfSessionEnd() is called by
> BrowserProcessImpl::EndSession(),
> and it's immediately followed by local_state()->CommitPendingWrite().
>
(https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/bro...)
> 
> MetricsService::RecordCompletedSessionEnd() is called by
> ShutdownPreThreadsStop() in browser_shutdown.cc,
> and it's also followed by prefs->CommitPendingWrite().
>
(https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/bro...)
> 
> AFAIK in all code paths where LogCleanShutdown() gets called, we can expect
the
> pref change is flushed to disk with almost no delay.

OK lgtm

[hashimoto, 2015-03-27 06:25:17]

The CQ bit was checked by hashimoto@chromium.org

[commit-bot: I haz the power, 2015-03-27 06:25:32]

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

[commit-bot: I haz the power, 2015-03-27 06:28:47]

Committed patchset #1 (id:20001)

[commit-bot: I haz the power, 2015-03-27 06:29:44]

Patchset 1 (id:??) landed as
https://crrev.com/a5c00e28d104b06b952aa2f1d9b4c21178cc4b71
Cr-Commit-Position: refs/heads/master@{#322535}