Rename DesktopEngagement* to DesktopSessionDuration*.

Rename DesktopEngagement* to DesktopSessionDuration*.

The DesktopEngagementService measures session length times on desktop.
Its name is very similar to the SiteEngagementService, which measures
how much user interaction a site has in total. This CL renames
DesktopEngagement* to DesktopSessionDuration* to clarify its role and
make it more distinct from Site Engagement.

BUG=646216
Committed: https://crrev.com/aa83588da377026736b3f20c7d8cdcdda1f0a9b3
Cr-Commit-Position: refs/heads/master@{#418773}

[dominickn, 2016-09-13 01:26:43]

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

[commit-bot: I haz the power, 2016-09-13 01:26:56]

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

[dominickn, 2016-09-13 01:29:52]

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

[dominickn, 2016-09-13 01:29:53]

PTAL, thanks!

https://codereview.chromium.org/2333113002/diff/1/chrome/browser/metrics/desk...
File chrome/browser/metrics/desktop_session/chrome_visibility_observer.cc
(right):

https://codereview.chromium.org/2333113002/diff/1/chrome/browser/metrics/desk...
chrome/browser/metrics/desktop_session/chrome_visibility_observer.cc:63:
"DesktopSession", "visibility_gap_timeout");
I grepped the finch studies directory and found no instances of this string, so
I assume it's safe to rename. Please let me know if this assumption is
incorrect.

[commit-bot: I haz the power, 2016-09-13 02:45:39]

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

[commit-bot: I haz the power, 2016-09-13 02:45:39]

Dry run: This issue passed the CQ dry run.

[gayane -on leave until 09-2017, 2016-09-13 17:09:29]

gayane@chromium.org changed reviewers:
+ gayane@chromium.org

[gayane -on leave until 09-2017, 2016-09-13 17:09:29]

Thanks for the changes.

https://codereview.chromium.org/2333113002/diff/1/chrome/browser/metrics/desk...
File chrome/browser/metrics/desktop_session/chrome_visibility_observer.cc
(right):

https://codereview.chromium.org/2333113002/diff/1/chrome/browser/metrics/desk...
chrome/browser/metrics/desktop_session/chrome_visibility_observer.cc:63:
"DesktopSession", "visibility_gap_timeout");
On 2016/09/13 01:29:53, dominickn wrote:
> I grepped the finch studies directory and found no instances of this string,
so
> I assume it's safe to rename. Please let me know if this assumption is
> incorrect.

Thats correct the study doesn't exist at the moment. This code was added for
future in case we will need to play around with the default timeout intervals.
So its safe to rename.

[Alexei Svitkine (slow), 2016-09-13 17:20:11]

lgtm, thanks!

[dominickn, 2016-09-13 23:14:53]

dominickn@chromium.org changed reviewers:
+ sky@chromium.org

[dominickn, 2016-09-13 23:14:53]

Thanks!

+sky: RS for chrome/browser (this is a pure renaming CL).

[sky, 2016-09-14 00:00:12]

https://codereview.chromium.org/2333113002/diff/1/chrome/browser/metrics/desk...
File chrome/browser/metrics/desktop_session/desktop_session_service.h (right):

https://codereview.chromium.org/2333113002/diff/1/chrome/browser/metrics/desk...
chrome/browser/metrics/desktop_session/desktop_session_service.h:19: class
DesktopSessionService : public AudibleContentsTracker::Observer {
This name is easily confused with session restore. Can you use something more
metrics/observer specific?

[dominickn, 2016-09-14 03:37:39]

On 2016/09/14 00:00:12, sky wrote:
>
https://codereview.chromium.org/2333113002/diff/1/chrome/browser/metrics/desk...
> File chrome/browser/metrics/desktop_session/desktop_session_service.h (right):
> 
>
https://codereview.chromium.org/2333113002/diff/1/chrome/browser/metrics/desk...
> chrome/browser/metrics/desktop_session/desktop_session_service.h:19: class
> DesktopSessionService : public AudibleContentsTracker::Observer {
> This name is easily confused with session restore. Can you use something more
> metrics/observer specific?

asvitkine: WDYT about DesktopSessionRecorder in place of DesktopSessionService?

[Alexei Svitkine (slow), 2016-09-14 15:05:06]

DesktopSessionRecorder sounds like something else than what it is, so I
don't like that proposed name.

We could make the name very explicit about what it's doing - i.e.
DesktopSessionDurationTracker. I'd be OK with it. (It does mean that if we
expand its scope at some point, we might need to rename it, but I'm OK with
not worrying about that at this point.)

On Tue, Sep 13, 2016 at 11:37 PM, <dominickn@chromium.org> wrote:

> On 2016/09/14 00:00:12, sky wrote:
> >
> https://codereview.chromium.org/2333113002/diff/1/chrome/
> browser/metrics/desktop_session/desktop_session_service.h
> > File chrome/browser/metrics/desktop_session/desktop_session_service.h
> (right):
> >
> >
> https://codereview.chromium.org/2333113002/diff/1/chrome/
> browser/metrics/desktop_session/desktop_session_service.h#newcode19
> > chrome/browser/metrics/desktop_session/desktop_session_service.h:19:
> class
> > DesktopSessionService : public AudibleContentsTracker::Observer {
> > This name is easily confused with session restore. Can you use something
> more
> > metrics/observer specific?
>
> asvitkine: WDYT about DesktopSessionRecorder in place of
> DesktopSessionService?
>
> https://codereview.chromium.org/2333113002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[sky, 2016-09-14 17:47:44]

+1 to DesktopSessionDurationTracker

On Wed, Sep 14, 2016 at 8:04 AM, Alexei Svitkine <asvitkine@chromium.org>
wrote:
> DesktopSessionRecorder sounds like something else than what it is, so I
> don't like that proposed name.
>
> We could make the name very explicit about what it's doing - i.e.
> DesktopSessionDurationTracker. I'd be OK with it. (It does mean that if we
> expand its scope at some point, we might need to rename it, but I'm OK with
> not worrying about that at this point.)
>
> On Tue, Sep 13, 2016 at 11:37 PM, <dominickn@chromium.org> wrote:
>>
>> On 2016/09/14 00:00:12, sky wrote:
>> >
>>
>>
https://codereview.chromium.org/2333113002/diff/1/chrome/browser/metrics/desk...
>> > File chrome/browser/metrics/desktop_session/desktop_session_service.h
>> > (right):
>> >
>> >
>>
>>
https://codereview.chromium.org/2333113002/diff/1/chrome/browser/metrics/desk...
>> > chrome/browser/metrics/desktop_session/desktop_session_service.h:19:
>> > class
>> > DesktopSessionService : public AudibleContentsTracker::Observer {
>> > This name is easily confused with session restore. Can you use something
>> > more
>> > metrics/observer specific?
>>
>> asvitkine: WDYT about DesktopSessionRecorder in place of
>> DesktopSessionService?
>>
>> https://codereview.chromium.org/2333113002/
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[dominickn, 2016-09-15 00:41:47]

Description was changed from

==========
Rename DesktopEngagement* to DesktopSession*.

The DesktopEngagementService measures session length times on desktop.
Its name is very similar to the SiteEngagementService, which measures
how much user interaction a site has in total. This CL renames
DesktopEngagement* to DesktopSession* to clarify its role and make it
more distinct from Site Engagement.

BUG=646216
==========

to

==========
Rename DesktopEngagement* to DesktopSessionDuration*.

The DesktopEngagementService measures session length times on desktop.
Its name is very similar to the SiteEngagementService, which measures
how much user interaction a site has in total. This CL renames
DesktopEngagement* to DesktopSessionDuration* to clarify its role and
make it more distinct from Site Engagement.

BUG=646216
==========

[dominickn, 2016-09-15 00:41:55]

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

[commit-bot: I haz the power, 2016-09-15 00:42:35]

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

[dominickn, 2016-09-15 00:43:22]

Renamed to DestopSessionDuration*. PTAL.

[commit-bot: I haz the power, 2016-09-15 01:30:28]

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

[commit-bot: I haz the power, 2016-09-15 01:30:29]

Dry run: This issue passed the CQ dry run.

[sky, 2016-09-15 03:04:05]

https://codereview.chromium.org/2333113002/diff/20001/chrome/browser/metrics/...
File chrome/browser/metrics/desktop_session_duration/audible_contents_tracker.h
(right):

https://codereview.chromium.org/2333113002/diff/20001/chrome/browser/metrics/...
chrome/browser/metrics/desktop_session_duration/audible_contents_tracker.h:23:
class Observer {
for a later patch: Style guide says observers should be in their own header.

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

https://codereview.chromium.org/2333113002/diff/20001/chrome/browser/metrics/...
chrome/browser/metrics/desktop_session_duration/chrome_visibility_observer.cc:63:
"DesktopSessionDuration", "visibility_gap_timeout");
Who keys off this string and does it need to be updated?

[dominickn, 2016-09-15 03:13:46]

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

https://codereview.chromium.org/2333113002/diff/20001/chrome/browser/metrics/...
chrome/browser/metrics/desktop_session_duration/chrome_visibility_observer.cc:63:
"DesktopSessionDuration", "visibility_gap_timeout");
On 2016/09/15 03:04:05, sky wrote:
> Who keys off this string and does it need to be updated?

Already asked and clarified this question in ps1: nothing uses it at the moment.

[sky, 2016-09-15 03:52:18]

Sorry, missed that, LGTM

[sky, 2016-09-15 03:52:27]

And thanks for updating the name!

[dominickn, 2016-09-15 03:56:25]

The CQ bit was checked by dominickn@chromium.org

[dominickn, 2016-09-15 03:56:25]

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/2333113002/#ps20001
(title: "Change name")

[commit-bot: I haz the power, 2016-09-15 03:56:39]

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-09-15 04:03:40]

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

[commit-bot: I haz the power, 2016-09-15 04:03:42]

Failed to apply patch for
chrome/browser/metrics/desktop_session_duration/audible_contents_tracker.cc:
While running git apply --index -3 -p1;
  error: chrome/browser/metrics/desktop_engagement/audible_contents_tracker.cc:
does not exist in index

Patch:   NR 
chrome/browser/metrics/desktop_engagement/audible_contents_tracker.cc->chrome/browser/metrics/desktop_session_duration/audible_contents_tracker.cc
Index:
chrome/browser/metrics/desktop_session_duration/audible_contents_tracker.cc
diff --git
a/chrome/browser/metrics/desktop_engagement/audible_contents_tracker.cc
b/chrome/browser/metrics/desktop_session_duration/audible_contents_tracker.cc
similarity index 93%
rename from
chrome/browser/metrics/desktop_engagement/audible_contents_tracker.cc
rename to
chrome/browser/metrics/desktop_session_duration/audible_contents_tracker.cc
index
5979c610369dafc99acabde8a06f0ed1fef66549..16d8cbc4a95adfd249a2d3455362097ef87966a2
100644
--- a/chrome/browser/metrics/desktop_engagement/audible_contents_tracker.cc
+++
b/chrome/browser/metrics/desktop_session_duration/audible_contents_tracker.cc
@@ -2,9 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/metrics/desktop_engagement/audible_contents_tracker.h"
+#include
"chrome/browser/metrics/desktop_session_duration/audible_contents_tracker.h"
 
-#include
"chrome/browser/metrics/desktop_engagement/desktop_engagement_service.h"
+#include
"chrome/browser/metrics/desktop_session_duration/desktop_session_duration_tracker.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"

[commit-bot: I haz the power, 2016-09-15 04:04:28]

Description was changed from

==========
Rename DesktopEngagement* to DesktopSessionDuration*.

The DesktopEngagementService measures session length times on desktop.
Its name is very similar to the SiteEngagementService, which measures
how much user interaction a site has in total. This CL renames
DesktopEngagement* to DesktopSessionDuration* to clarify its role and
make it more distinct from Site Engagement.

BUG=646216
==========

to

==========
Rename DesktopEngagement* to DesktopSessionDuration*.

The DesktopEngagementService measures session length times on desktop.
Its name is very similar to the SiteEngagementService, which measures
how much user interaction a site has in total. This CL renames
DesktopEngagement* to DesktopSessionDuration* to clarify its role and
make it more distinct from Site Engagement.

BUG=646216

Committed: https://crrev.com/aa83588da377026736b3f20c7d8cdcdda1f0a9b3
Cr-Commit-Position: refs/heads/master@{#418773}
==========

[commit-bot: I haz the power, 2016-09-15 04:04:29]

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