[DesktopSessionDurationTracker] Remove visibility-switch timeout from session length.

[DesktopSessionDurationTracker] Remove visibility-switch timeout from session length.

This aims to remove the artificial spike at 3 seconds in the Session.TotalDuration metric.

BUG=652253
Committed: https://crrev.com/8f546cb788ad68d0f2de3ea047a0166a35884b33
Cr-Commit-Position: refs/heads/master@{#422501}

[chrisha, 2016-10-03 15:31:35]

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

[commit-bot: I haz the power, 2016-10-03 15:31:56]

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-10-03 16:37:58]

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

[commit-bot: I haz the power, 2016-10-03 16:37:59]

Dry run: This issue passed the CQ dry run.

[chrisha, 2016-10-03 16:39:38]

chrisha@chromium.org changed reviewers:
+ asvitkine@chromium.org, rkaplow@chromium.org

[chrisha, 2016-10-03 16:39:38]

PTAL?

[Alexei Svitkine (slow), 2016-10-03 16:50:33]

https://codereview.chromium.org/2385103003/diff/40001/chrome/browser/metrics/...
File
chrome/browser/metrics/desktop_session_duration/chrome_visibility_observer.cc
(right):

https://codereview.chromium.org/2385103003/diff/40001/chrome/browser/metrics/...
chrome/browser/metrics/desktop_session_duration/chrome_visibility_observer.cc:33:
bool active, base::TimeDelta time_ago) {
Nit: I think convention is 1 param per line if the first param is on its own
line, but not 100% sure - can you check what git cl format does?

https://codereview.chromium.org/2385103003/diff/40001/chrome/browser/metrics/...
File
chrome/browser/metrics/desktop_session_duration/desktop_session_duration_tracker_unittest.cc
(right):

https://codereview.chromium.org/2385103003/diff/40001/chrome/browser/metrics/...
chrome/browser/metrics/desktop_session_duration/desktop_session_duration_tracker_unittest.cc:185:
}
I'm having trouble seeing how this test is different from the one below...

Also, if the two must share some of the same helper code - can you maybe
refactor the code to use some helper functions?

[chrisha, 2016-10-03 16:58:40]

https://codereview.chromium.org/2385103003/diff/40001/chrome/browser/metrics/...
File
chrome/browser/metrics/desktop_session_duration/desktop_session_duration_tracker_unittest.cc
(right):

https://codereview.chromium.org/2385103003/diff/40001/chrome/browser/metrics/...
chrome/browser/metrics/desktop_session_duration/desktop_session_duration_tracker_unittest.cc:185:
}
On 2016/10/03 16:50:33, Alexei Svitkine (slow) wrote:
> I'm having trouble seeing how this test is different from the one below...
> 
> Also, if the two must share some of the same helper code - can you maybe
> refactor the code to use some helper functions?

Oops, was in the middle of writing this and completely forgot about it :)

[chrisha, 2016-10-03 18:06:09]

PTAL?

https://codereview.chromium.org/2385103003/diff/40001/chrome/browser/metrics/...
File
chrome/browser/metrics/desktop_session_duration/chrome_visibility_observer.cc
(right):

https://codereview.chromium.org/2385103003/diff/40001/chrome/browser/metrics/...
chrome/browser/metrics/desktop_session_duration/chrome_visibility_observer.cc:33:
bool active, base::TimeDelta time_ago) {
On 2016/10/03 16:50:33, Alexei Svitkine (slow) wrote:
> Nit: I think convention is 1 param per line if the first param is on its own
> line, but not 100% sure - can you check what git cl format does?

Done.

[Alexei Svitkine (slow), 2016-10-03 18:38:49]

lgtm

https://codereview.chromium.org/2385103003/diff/60001/chrome/browser/metrics/...
File
chrome/browser/metrics/desktop_session_duration/desktop_session_duration_tracker_unittest.cc
(right):

https://codereview.chromium.org/2385103003/diff/60001/chrome/browser/metrics/...
chrome/browser/metrics/desktop_session_duration/desktop_session_duration_tracker_unittest.cc:56:
base::MessageLoop loop_;
Nit: For things not used directly in the tests, put in private: section and add
DISALLOW_COPY_AND_ASSIGN().

[chrisha, 2016-10-03 19:15:41]

On 2016/10/03 18:38:49, Alexei Svitkine (slow) wrote:
> lgtm
> 
>
https://codereview.chromium.org/2385103003/diff/60001/chrome/browser/metrics/...
> File
>
chrome/browser/metrics/desktop_session_duration/desktop_session_duration_tracker_unittest.cc
> (right):
> 
>
https://codereview.chromium.org/2385103003/diff/60001/chrome/browser/metrics/...
>
chrome/browser/metrics/desktop_session_duration/desktop_session_duration_tracker_unittest.cc:56:
> base::MessageLoop loop_;
> Nit: For things not used directly in the tests, put in private: section and
add
> DISALLOW_COPY_AND_ASSIGN().

Done.

[chrisha, 2016-10-03 19:19:11]

The CQ bit was checked by chrisha@chromium.org

[chrisha, 2016-10-03 19:19:11]

The patchset sent to the CQ was uploaded after l-g-t-m from
asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/2385103003/#ps80001
(title: "Address nits.")

[commit-bot: I haz the power, 2016-10-03 19:19:48]

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-10-03 19:59:15]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-10-03 20:00:50]

Description was changed from

==========
[DesktopSessionDurationTracker] Remove visibility-switch timeout from session
length.

This aims to remove the artificial spike at 3 seconds in the
Session.TotalDuration metric.

BUG=652253
==========

to

==========
[DesktopSessionDurationTracker] Remove visibility-switch timeout from session
length.

This aims to remove the artificial spike at 3 seconds in the
Session.TotalDuration metric.

BUG=652253

Committed: https://crrev.com/8f546cb788ad68d0f2de3ea047a0166a35884b33
Cr-Commit-Position: refs/heads/master@{#422501}
==========

[commit-bot: I haz the power, 2016-10-03 20:00:51]

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