New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 706309 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Traveling - Back 2/6
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Feature

Blocking:
issue 704228


Show other hotlists

Hotlists containing this issue:
In-Product-Help


Sign in to add a comment

In-Product Help Backend

Project Member Reported by nyquist@chromium.org, Mar 29 2017

Issue description

Tracking bug for implementation of the In-Product Help client-side backend.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9bf0cdf694d4267208da03b25ab45959606d1208

commit 9bf0cdf694d4267208da03b25ab45959606d1208
Author: nyquist <nyquist@chromium.org>
Date: Fri Mar 31 20:45:07 2017

Add component for feature engagement tracking.

The Feature Engagement Tracker provides a client-side backend for
displaying feature enlightenment or in-product help with a clean and
easy to use API to be consumed by the UI frontend. The backend behaves
as a black box and takes input about user behavior. Whenever the
frontend gives a trigger signal that in-product help could be displayed,
the backend will provide an answer to whether it is appropriate to show
it or not.

This CL adds a new component for this, with both a public C++ and a
public Java API. It also sets up the KeyedService factory in //chrome,
and hooks up the bridge between the C++ and Java implementations of the
backend. The Java version is a pass through, and all business logic is
implemented in C++. This means that callers do not have to care whether
they use the Java or C++ version.

It has been implemented as a component to ensure that other components
can use it directly, and to ensure that it would be easy to use the
same system across different platforms.

BUG= 706309 

Review-Url: https://codereview.chromium.org/2782113002
Cr-Commit-Position: refs/heads/master@{#461215}

[modify] https://crrev.com/9bf0cdf694d4267208da03b25ab45959606d1208/chrome/android/BUILD.gn
[modify] https://crrev.com/9bf0cdf694d4267208da03b25ab45959606d1208/chrome/android/java/DEPS
[add] https://crrev.com/9bf0cdf694d4267208da03b25ab45959606d1208/chrome/android/java/src/org/chromium/chrome/browser/feature_engagement_tracker/FeatureEngagementTrackerFactory.java
[add] https://crrev.com/9bf0cdf694d4267208da03b25ab45959606d1208/chrome/android/java/src/org/chromium/chrome/browser/feature_engagement_tracker/OWNERS
[modify] https://crrev.com/9bf0cdf694d4267208da03b25ab45959606d1208/chrome/android/java_sources.gni
[modify] https://crrev.com/9bf0cdf694d4267208da03b25ab45959606d1208/chrome/browser/BUILD.gn
[modify] https://crrev.com/9bf0cdf694d4267208da03b25ab45959606d1208/chrome/browser/android/chrome_jni_registrar.cc
[add] https://crrev.com/9bf0cdf694d4267208da03b25ab45959606d1208/chrome/browser/android/feature_engagement_tracker/OWNERS
[add] https://crrev.com/9bf0cdf694d4267208da03b25ab45959606d1208/chrome/browser/android/feature_engagement_tracker/feature_engagement_tracker_factory_android.cc
[add] https://crrev.com/9bf0cdf694d4267208da03b25ab45959606d1208/chrome/browser/android/feature_engagement_tracker/feature_engagement_tracker_factory_android.h
[add] https://crrev.com/9bf0cdf694d4267208da03b25ab45959606d1208/chrome/browser/feature_engagement_tracker/DEPS
[add] https://crrev.com/9bf0cdf694d4267208da03b25ab45959606d1208/chrome/browser/feature_engagement_tracker/OWNERS
[add] https://crrev.com/9bf0cdf694d4267208da03b25ab45959606d1208/chrome/browser/feature_engagement_tracker/feature_engagement_tracker_factory.cc
[add] https://crrev.com/9bf0cdf694d4267208da03b25ab45959606d1208/chrome/browser/feature_engagement_tracker/feature_engagement_tracker_factory.h
[modify] https://crrev.com/9bf0cdf694d4267208da03b25ab45959606d1208/chrome/common/chrome_constants.cc
[modify] https://crrev.com/9bf0cdf694d4267208da03b25ab45959606d1208/chrome/common/chrome_constants.h
[modify] https://crrev.com/9bf0cdf694d4267208da03b25ab45959606d1208/components/BUILD.gn
[add] https://crrev.com/9bf0cdf694d4267208da03b25ab45959606d1208/components/feature_engagement_tracker/BUILD.gn
[add] https://crrev.com/9bf0cdf694d4267208da03b25ab45959606d1208/components/feature_engagement_tracker/DEPS
[add] https://crrev.com/9bf0cdf694d4267208da03b25ab45959606d1208/components/feature_engagement_tracker/OWNERS
[add] https://crrev.com/9bf0cdf694d4267208da03b25ab45959606d1208/components/feature_engagement_tracker/README.md
[add] https://crrev.com/9bf0cdf694d4267208da03b25ab45959606d1208/components/feature_engagement_tracker/internal/BUILD.gn
[add] https://crrev.com/9bf0cdf694d4267208da03b25ab45959606d1208/components/feature_engagement_tracker/internal/android/feature_engagement_tracker_impl_android.cc
[add] https://crrev.com/9bf0cdf694d4267208da03b25ab45959606d1208/components/feature_engagement_tracker/internal/android/feature_engagement_tracker_impl_android.h
[add] https://crrev.com/9bf0cdf694d4267208da03b25ab45959606d1208/components/feature_engagement_tracker/internal/android/feature_engagement_tracker_jni_registrar.cc
[add] https://crrev.com/9bf0cdf694d4267208da03b25ab45959606d1208/components/feature_engagement_tracker/internal/android/java/src/org/chromium/components/feature_engagement_tracker/internal/FeatureEngagementTrackerImpl.java
[add] https://crrev.com/9bf0cdf694d4267208da03b25ab45959606d1208/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl.cc
[add] https://crrev.com/9bf0cdf694d4267208da03b25ab45959606d1208/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl.h
[add] https://crrev.com/9bf0cdf694d4267208da03b25ab45959606d1208/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl_unittest.cc
[add] https://crrev.com/9bf0cdf694d4267208da03b25ab45959606d1208/components/feature_engagement_tracker/public/BUILD.gn
[add] https://crrev.com/9bf0cdf694d4267208da03b25ab45959606d1208/components/feature_engagement_tracker/public/android/feature_engagement_tracker_jni_registrar.h
[add] https://crrev.com/9bf0cdf694d4267208da03b25ab45959606d1208/components/feature_engagement_tracker/public/android/java/src/org/chromium/components/feature_engagement_tracker/FeatureEngagementTracker.java
[add] https://crrev.com/9bf0cdf694d4267208da03b25ab45959606d1208/components/feature_engagement_tracker/public/feature_engagement_tracker.h

Project Member

Comment 2 by bugdroid1@chromium.org, Apr 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ce897e8ed500323ddc5544a945287b9dae98831b

commit ce897e8ed500323ddc5544a945287b9dae98831b
Author: Tommy Nyquist <nyquist@chromium.org>
Date: Mon Apr 03 22:49:54 2017

Add simple implementation of in-product help backend.

This version of the in-product help backend ensures nothing will be
triggered unless the base::Feature has been enabled. It also ensures
that only one feature can be triggered per session.

It also changes the public API to now use base::Feature instead of the
string name. The Java API for this is still using strings, so a helper
class with all the relevant constants has been added, that matches the
native base::Feature instances.

A feature can now be enabled on the command line using:
--enable-features=IPH_DummyFeature
This dummy feature will be removed once there are real features using
the backend.

BUG= 706309 

Change-Id: I1996d9c50d73a93acb26a675c14ebf8770cf0571
Reviewed-on: https://chromium-review.googlesource.com/465352
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#461566}
[modify] https://crrev.com/ce897e8ed500323ddc5544a945287b9dae98831b/components/feature_engagement_tracker/internal/BUILD.gn
[modify] https://crrev.com/ce897e8ed500323ddc5544a945287b9dae98831b/components/feature_engagement_tracker/internal/android/feature_engagement_tracker_impl_android.cc
[modify] https://crrev.com/ce897e8ed500323ddc5544a945287b9dae98831b/components/feature_engagement_tracker/internal/android/feature_engagement_tracker_impl_android.h
[add] https://crrev.com/ce897e8ed500323ddc5544a945287b9dae98831b/components/feature_engagement_tracker/internal/feature_constants.cc
[modify] https://crrev.com/ce897e8ed500323ddc5544a945287b9dae98831b/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl.cc
[modify] https://crrev.com/ce897e8ed500323ddc5544a945287b9dae98831b/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl.h
[modify] https://crrev.com/ce897e8ed500323ddc5544a945287b9dae98831b/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl_unittest.cc
[add] https://crrev.com/ce897e8ed500323ddc5544a945287b9dae98831b/components/feature_engagement_tracker/internal/feature_list.cc
[add] https://crrev.com/ce897e8ed500323ddc5544a945287b9dae98831b/components/feature_engagement_tracker/internal/feature_list.h
[modify] https://crrev.com/ce897e8ed500323ddc5544a945287b9dae98831b/components/feature_engagement_tracker/public/BUILD.gn
[add] https://crrev.com/ce897e8ed500323ddc5544a945287b9dae98831b/components/feature_engagement_tracker/public/android/java/src/org/chromium/components/feature_engagement_tracker/FeatureConstants.java
[add] https://crrev.com/ce897e8ed500323ddc5544a945287b9dae98831b/components/feature_engagement_tracker/public/feature_constants.h
[modify] https://crrev.com/ce897e8ed500323ddc5544a945287b9dae98831b/components/feature_engagement_tracker/public/feature_engagement_tracker.h

Project Member

Comment 3 by bugdroid1@chromium.org, Apr 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fb5c907d801f5fb0d6542c14f29c5749ae78abc7

commit fb5c907d801f5fb0d6542c14f29c5749ae78abc7
Author: Tommy Nyquist <nyquist@chromium.org>
Date: Tue Apr 04 00:31:04 2017

Simplify feature engagement tracker API.

Instead of splitting out preconditions, trigger-events and whether
a feature has been used, they are now all just events. This
simplifies the API surface quite a bit.

Whether a feature has been used will now happen by marking a
particular event as the one that signifies that a feature has been
used. This will be configured per in-product help feature.

The check for whether the in-product help should be trigger is
still a similar method that returns a boolean.

BUG= 706309 

Change-Id: I309f95ebc417d078e9e671b271b2d50e7e8f0fb2
Reviewed-on: https://chromium-review.googlesource.com/466586
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#461587}
[modify] https://crrev.com/fb5c907d801f5fb0d6542c14f29c5749ae78abc7/components/feature_engagement_tracker/internal/android/feature_engagement_tracker_impl_android.cc
[modify] https://crrev.com/fb5c907d801f5fb0d6542c14f29c5749ae78abc7/components/feature_engagement_tracker/internal/android/feature_engagement_tracker_impl_android.h
[modify] https://crrev.com/fb5c907d801f5fb0d6542c14f29c5749ae78abc7/components/feature_engagement_tracker/internal/android/java/src/org/chromium/components/feature_engagement_tracker/internal/FeatureEngagementTrackerImpl.java
[modify] https://crrev.com/fb5c907d801f5fb0d6542c14f29c5749ae78abc7/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl.cc
[modify] https://crrev.com/fb5c907d801f5fb0d6542c14f29c5749ae78abc7/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl.h
[modify] https://crrev.com/fb5c907d801f5fb0d6542c14f29c5749ae78abc7/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl_unittest.cc
[modify] https://crrev.com/fb5c907d801f5fb0d6542c14f29c5749ae78abc7/components/feature_engagement_tracker/public/android/java/src/org/chromium/components/feature_engagement_tracker/FeatureEngagementTracker.java
[modify] https://crrev.com/fb5c907d801f5fb0d6542c14f29c5749ae78abc7/components/feature_engagement_tracker/public/feature_engagement_tracker.h

Project Member

Comment 4 by bugdroid1@chromium.org, Apr 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/51794fd73b390795889b5553bcbf8244d24cb2fd

commit 51794fd73b390795889b5553bcbf8244d24cb2fd
Author: Tommy Nyquist <nyquist@chromium.org>
Date: Tue Apr 11 06:16:47 2017

Add internal framework for FeatureEngagementTracker

Previously, the feature_engagement_tracker component mostly contained a
public API, with hardly any business logic.

This CL adds a framework for the internal implementation, such as
Configuration, Model, Store and ConditionValidator, and simple implementations
and tests for each of these:
- The Model contains the full state of the tracker.
- The Store is the backing storage of the model.
- The Configuration provides the configuration for each of the features.
- The ConditionValidator is the decider of whether in-product help should be
  shown.

It also updates the FeatureEngagementTrackerImpl to use this new framework, and
adds support for tracking whether in-product help is currently being shown in
the model.

It also adds a particular ConditionValidator for demoing the feature, which
enables each feature to show in-product help exactly once, regardless of
whether any events have happened, as long as the current state is valid. This
will later be used for manual testing through a switch in chrome://flags.

This CL does not add support for asynchronous loading of any state.

BUG= 706309 

Change-Id: I16fa9a09e5c5378be8d3cd48e8e37c0ad8bc124d
Reviewed-on: https://chromium-review.googlesource.com/472534
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#463549}
[modify] https://crrev.com/51794fd73b390795889b5553bcbf8244d24cb2fd/components/feature_engagement_tracker/internal/BUILD.gn
[modify] https://crrev.com/51794fd73b390795889b5553bcbf8244d24cb2fd/components/feature_engagement_tracker/internal/android/feature_engagement_tracker_impl_android.cc
[modify] https://crrev.com/51794fd73b390795889b5553bcbf8244d24cb2fd/components/feature_engagement_tracker/internal/android/feature_engagement_tracker_impl_android.h
[add] https://crrev.com/51794fd73b390795889b5553bcbf8244d24cb2fd/components/feature_engagement_tracker/internal/condition_validator.h
[add] https://crrev.com/51794fd73b390795889b5553bcbf8244d24cb2fd/components/feature_engagement_tracker/internal/configuration.cc
[add] https://crrev.com/51794fd73b390795889b5553bcbf8244d24cb2fd/components/feature_engagement_tracker/internal/configuration.h
[add] https://crrev.com/51794fd73b390795889b5553bcbf8244d24cb2fd/components/feature_engagement_tracker/internal/editable_configuration.cc
[add] https://crrev.com/51794fd73b390795889b5553bcbf8244d24cb2fd/components/feature_engagement_tracker/internal/editable_configuration.h
[add] https://crrev.com/51794fd73b390795889b5553bcbf8244d24cb2fd/components/feature_engagement_tracker/internal/editable_configuration_unittest.cc
[modify] https://crrev.com/51794fd73b390795889b5553bcbf8244d24cb2fd/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl.cc
[modify] https://crrev.com/51794fd73b390795889b5553bcbf8244d24cb2fd/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl.h
[modify] https://crrev.com/51794fd73b390795889b5553bcbf8244d24cb2fd/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl_unittest.cc
[modify] https://crrev.com/51794fd73b390795889b5553bcbf8244d24cb2fd/components/feature_engagement_tracker/internal/feature_list.h
[add] https://crrev.com/51794fd73b390795889b5553bcbf8244d24cb2fd/components/feature_engagement_tracker/internal/in_memory_store.cc
[add] https://crrev.com/51794fd73b390795889b5553bcbf8244d24cb2fd/components/feature_engagement_tracker/internal/in_memory_store.h
[add] https://crrev.com/51794fd73b390795889b5553bcbf8244d24cb2fd/components/feature_engagement_tracker/internal/in_memory_store_unittest.cc
[add] https://crrev.com/51794fd73b390795889b5553bcbf8244d24cb2fd/components/feature_engagement_tracker/internal/model.h
[add] https://crrev.com/51794fd73b390795889b5553bcbf8244d24cb2fd/components/feature_engagement_tracker/internal/model_impl.cc
[add] https://crrev.com/51794fd73b390795889b5553bcbf8244d24cb2fd/components/feature_engagement_tracker/internal/model_impl.h
[add] https://crrev.com/51794fd73b390795889b5553bcbf8244d24cb2fd/components/feature_engagement_tracker/internal/model_impl_unittest.cc
[add] https://crrev.com/51794fd73b390795889b5553bcbf8244d24cb2fd/components/feature_engagement_tracker/internal/once_condition_validator.cc
[add] https://crrev.com/51794fd73b390795889b5553bcbf8244d24cb2fd/components/feature_engagement_tracker/internal/once_condition_validator.h
[add] https://crrev.com/51794fd73b390795889b5553bcbf8244d24cb2fd/components/feature_engagement_tracker/internal/once_condition_validator_unittest.cc
[add] https://crrev.com/51794fd73b390795889b5553bcbf8244d24cb2fd/components/feature_engagement_tracker/internal/store.h

Project Member

Comment 5 by bugdroid1@chromium.org, Apr 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4fad7069e05d919a504e8db88f6d0a1019aa129b

commit 4fad7069e05d919a504e8db88f6d0a1019aa129b
Author: Tommy Nyquist <nyquist@chromium.org>
Date: Tue Apr 11 21:09:19 2017

Add demo-mode for In-Product Help.

This adds a new flag to chrome://flags for controlling a demo mode for
in-product help. By enabling the flag, each in-product help feature
will trigger a single time per session. This is to enable manual testing
of all in-product help UI.

Since enabling this feature can not enable the other in-product help
features, the OnceConditionValidator is changed to not require a feature
to be enabled.

BUG= 706309 

Change-Id: Id01bc34fb1d297645c115e2f2b13114eb05b2e24
Reviewed-on: https://chromium-review.googlesource.com/473706
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#463770}
[modify] https://crrev.com/4fad7069e05d919a504e8db88f6d0a1019aa129b/chrome/browser/about_flags.cc
[modify] https://crrev.com/4fad7069e05d919a504e8db88f6d0a1019aa129b/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/4fad7069e05d919a504e8db88f6d0a1019aa129b/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/4fad7069e05d919a504e8db88f6d0a1019aa129b/components/feature_engagement_tracker/internal/BUILD.gn
[modify] https://crrev.com/4fad7069e05d919a504e8db88f6d0a1019aa129b/components/feature_engagement_tracker/internal/feature_constants.cc
[modify] https://crrev.com/4fad7069e05d919a504e8db88f6d0a1019aa129b/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl.cc
[add] https://crrev.com/4fad7069e05d919a504e8db88f6d0a1019aa129b/components/feature_engagement_tracker/internal/never_condition_validator.cc
[add] https://crrev.com/4fad7069e05d919a504e8db88f6d0a1019aa129b/components/feature_engagement_tracker/internal/never_condition_validator.h
[add] https://crrev.com/4fad7069e05d919a504e8db88f6d0a1019aa129b/components/feature_engagement_tracker/internal/never_condition_validator_unittest.cc
[modify] https://crrev.com/4fad7069e05d919a504e8db88f6d0a1019aa129b/components/feature_engagement_tracker/internal/once_condition_validator.cc
[modify] https://crrev.com/4fad7069e05d919a504e8db88f6d0a1019aa129b/components/feature_engagement_tracker/internal/once_condition_validator.h
[modify] https://crrev.com/4fad7069e05d919a504e8db88f6d0a1019aa129b/components/feature_engagement_tracker/internal/once_condition_validator_unittest.cc
[add] https://crrev.com/4fad7069e05d919a504e8db88f6d0a1019aa129b/components/feature_engagement_tracker/internal/single_invalid_configuration.cc
[add] https://crrev.com/4fad7069e05d919a504e8db88f6d0a1019aa129b/components/feature_engagement_tracker/internal/single_invalid_configuration.h
[add] https://crrev.com/4fad7069e05d919a504e8db88f6d0a1019aa129b/components/feature_engagement_tracker/internal/single_invalid_configuration_unittest.cc
[modify] https://crrev.com/4fad7069e05d919a504e8db88f6d0a1019aa129b/components/feature_engagement_tracker/public/feature_constants.h
[modify] https://crrev.com/4fad7069e05d919a504e8db88f6d0a1019aa129b/tools/metrics/histograms/histograms.xml

Project Member

Comment 6 by bugdroid1@chromium.org, Apr 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/da01210bc087bff3f3e9b582598e3d0bb68bd9e6

commit da01210bc087bff3f3e9b582598e3d0bb68bd9e6
Author: Tommy Nyquist <nyquist@chromium.org>
Date: Wed Apr 12 23:27:47 2017

Add basic implementation of feature engagement tracker model.

This CL adds the Event protocol buffer definition, which is used both
at runtime and will also be used for storing data to disk.

This required updating the InMemoryStore to support injecting a set of
events at construction time, and be able to provide them to the callback
when the Store is loaded. It also expands the Store API to provide a
way of writing data back.

In addition, it expanded the Model implementation to support storing,
and retrieving events and also incrementing them. New events will be
created on the fly as they do not have to be registered up front.

The new initialization code requires a MessageLoop, which meant that
some existing tests also needed to be updated.

BUG= 706309 

Change-Id: I11f613e565945bfd8783e61b4f0bbb71996369e9
Reviewed-on: https://chromium-review.googlesource.com/475492
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#464206}
[modify] https://crrev.com/da01210bc087bff3f3e9b582598e3d0bb68bd9e6/components/feature_engagement_tracker/internal/BUILD.gn
[modify] https://crrev.com/da01210bc087bff3f3e9b582598e3d0bb68bd9e6/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl_unittest.cc
[modify] https://crrev.com/da01210bc087bff3f3e9b582598e3d0bb68bd9e6/components/feature_engagement_tracker/internal/in_memory_store.cc
[modify] https://crrev.com/da01210bc087bff3f3e9b582598e3d0bb68bd9e6/components/feature_engagement_tracker/internal/in_memory_store.h
[modify] https://crrev.com/da01210bc087bff3f3e9b582598e3d0bb68bd9e6/components/feature_engagement_tracker/internal/in_memory_store_unittest.cc
[modify] https://crrev.com/da01210bc087bff3f3e9b582598e3d0bb68bd9e6/components/feature_engagement_tracker/internal/model.h
[modify] https://crrev.com/da01210bc087bff3f3e9b582598e3d0bb68bd9e6/components/feature_engagement_tracker/internal/model_impl.cc
[modify] https://crrev.com/da01210bc087bff3f3e9b582598e3d0bb68bd9e6/components/feature_engagement_tracker/internal/model_impl.h
[modify] https://crrev.com/da01210bc087bff3f3e9b582598e3d0bb68bd9e6/components/feature_engagement_tracker/internal/model_impl_unittest.cc
[modify] https://crrev.com/da01210bc087bff3f3e9b582598e3d0bb68bd9e6/components/feature_engagement_tracker/internal/never_condition_validator_unittest.cc
[modify] https://crrev.com/da01210bc087bff3f3e9b582598e3d0bb68bd9e6/components/feature_engagement_tracker/internal/once_condition_validator_unittest.cc
[add] https://crrev.com/da01210bc087bff3f3e9b582598e3d0bb68bd9e6/components/feature_engagement_tracker/internal/proto/BUILD.gn
[add] https://crrev.com/da01210bc087bff3f3e9b582598e3d0bb68bd9e6/components/feature_engagement_tracker/internal/proto/event.proto
[modify] https://crrev.com/da01210bc087bff3f3e9b582598e3d0bb68bd9e6/components/feature_engagement_tracker/internal/store.h

Project Member

Comment 7 by bugdroid1@chromium.org, Apr 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/74a524acc53882844cfb50d156e673b42ea2809d

commit 74a524acc53882844cfb50d156e673b42ea2809d
Author: Tommy Nyquist <nyquist@chromium.org>
Date: Wed Apr 19 22:41:37 2017

Add support for parsing Chrome Variations field trial config.

This CL expands the FeatureConfig to its full definition, and adds
support for parsing it from field trial params.

It also adds helper comparators for all the new FeatureConfig data.

BUG= 706309 

Change-Id: I87dcd506422c3d93707b46eb9c4a6ae7df39dc34
Reviewed-on: https://chromium-review.googlesource.com/477532
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: Shakti Sahu <shaktisahu@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#465790}
[modify] https://crrev.com/74a524acc53882844cfb50d156e673b42ea2809d/components/feature_engagement_tracker/internal/BUILD.gn
[add] https://crrev.com/74a524acc53882844cfb50d156e673b42ea2809d/components/feature_engagement_tracker/internal/chrome_variations_configuration.cc
[add] https://crrev.com/74a524acc53882844cfb50d156e673b42ea2809d/components/feature_engagement_tracker/internal/chrome_variations_configuration.h
[add] https://crrev.com/74a524acc53882844cfb50d156e673b42ea2809d/components/feature_engagement_tracker/internal/chrome_variations_configuration_unittest.cc
[modify] https://crrev.com/74a524acc53882844cfb50d156e673b42ea2809d/components/feature_engagement_tracker/internal/configuration.cc
[modify] https://crrev.com/74a524acc53882844cfb50d156e673b42ea2809d/components/feature_engagement_tracker/internal/configuration.h
[modify] https://crrev.com/74a524acc53882844cfb50d156e673b42ea2809d/components/feature_engagement_tracker/internal/editable_configuration_unittest.cc
[modify] https://crrev.com/74a524acc53882844cfb50d156e673b42ea2809d/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl_unittest.cc
[modify] https://crrev.com/74a524acc53882844cfb50d156e673b42ea2809d/components/feature_engagement_tracker/internal/model_impl_unittest.cc
[modify] https://crrev.com/74a524acc53882844cfb50d156e673b42ea2809d/components/feature_engagement_tracker/internal/never_condition_validator_unittest.cc
[modify] https://crrev.com/74a524acc53882844cfb50d156e673b42ea2809d/components/feature_engagement_tracker/internal/single_invalid_configuration.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Apr 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/74a524acc53882844cfb50d156e673b42ea2809d

commit 74a524acc53882844cfb50d156e673b42ea2809d
Author: Tommy Nyquist <nyquist@chromium.org>
Date: Wed Apr 19 22:41:37 2017

Add support for parsing Chrome Variations field trial config.

This CL expands the FeatureConfig to its full definition, and adds
support for parsing it from field trial params.

It also adds helper comparators for all the new FeatureConfig data.

BUG= 706309 

Change-Id: I87dcd506422c3d93707b46eb9c4a6ae7df39dc34
Reviewed-on: https://chromium-review.googlesource.com/477532
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: Shakti Sahu <shaktisahu@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#465790}
[modify] https://crrev.com/74a524acc53882844cfb50d156e673b42ea2809d/components/feature_engagement_tracker/internal/BUILD.gn
[add] https://crrev.com/74a524acc53882844cfb50d156e673b42ea2809d/components/feature_engagement_tracker/internal/chrome_variations_configuration.cc
[add] https://crrev.com/74a524acc53882844cfb50d156e673b42ea2809d/components/feature_engagement_tracker/internal/chrome_variations_configuration.h
[add] https://crrev.com/74a524acc53882844cfb50d156e673b42ea2809d/components/feature_engagement_tracker/internal/chrome_variations_configuration_unittest.cc
[modify] https://crrev.com/74a524acc53882844cfb50d156e673b42ea2809d/components/feature_engagement_tracker/internal/configuration.cc
[modify] https://crrev.com/74a524acc53882844cfb50d156e673b42ea2809d/components/feature_engagement_tracker/internal/configuration.h
[modify] https://crrev.com/74a524acc53882844cfb50d156e673b42ea2809d/components/feature_engagement_tracker/internal/editable_configuration_unittest.cc
[modify] https://crrev.com/74a524acc53882844cfb50d156e673b42ea2809d/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl_unittest.cc
[modify] https://crrev.com/74a524acc53882844cfb50d156e673b42ea2809d/components/feature_engagement_tracker/internal/model_impl_unittest.cc
[modify] https://crrev.com/74a524acc53882844cfb50d156e673b42ea2809d/components/feature_engagement_tracker/internal/never_condition_validator_unittest.cc
[modify] https://crrev.com/74a524acc53882844cfb50d156e673b42ea2809d/components/feature_engagement_tracker/internal/single_invalid_configuration.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Apr 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0d89c8a2635c58c952c33c22a71d9a7f3ea564b5

commit 0d89c8a2635c58c952c33c22a71d9a7f3ea564b5
Author: Tommy Nyquist <nyquist@chromium.org>
Date: Tue Apr 25 05:15:14 2017

Add new validator for whether In-Product Help events should be stored.

This CL adds a new interface that will be used to validate whether new
events should be stored, and during DB load on startup, it is also used
to check if old event should still be kept around.

The production implementation of this will be
FeatureConfigStorageValidator, which uses the FeatureConfig to check
what should be stored, but for now, the NeverStorageValidator will be
used for all things to ensure that nothing is ever stored for now.

This CL sets up the ownership of the StorageValidator, but does not
update the ModelImpl to use the new framework, and leaves that as
TODOs.

BUG= 706309 

Change-Id: Ic581edc73f8f059c18fb1dcb5a549c4e62f490f8
Reviewed-on: https://chromium-review.googlesource.com/484682
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#466875}
[modify] https://crrev.com/0d89c8a2635c58c952c33c22a71d9a7f3ea564b5/components/feature_engagement_tracker/internal/BUILD.gn
[modify] https://crrev.com/0d89c8a2635c58c952c33c22a71d9a7f3ea564b5/components/feature_engagement_tracker/internal/condition_validator.h
[add] https://crrev.com/0d89c8a2635c58c952c33c22a71d9a7f3ea564b5/components/feature_engagement_tracker/internal/feature_config_storage_validator.cc
[add] https://crrev.com/0d89c8a2635c58c952c33c22a71d9a7f3ea564b5/components/feature_engagement_tracker/internal/feature_config_storage_validator.h
[add] https://crrev.com/0d89c8a2635c58c952c33c22a71d9a7f3ea564b5/components/feature_engagement_tracker/internal/feature_config_storage_validator_unittest.cc
[modify] https://crrev.com/0d89c8a2635c58c952c33c22a71d9a7f3ea564b5/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl.cc
[modify] https://crrev.com/0d89c8a2635c58c952c33c22a71d9a7f3ea564b5/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl.h
[modify] https://crrev.com/0d89c8a2635c58c952c33c22a71d9a7f3ea564b5/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl_unittest.cc
[modify] https://crrev.com/0d89c8a2635c58c952c33c22a71d9a7f3ea564b5/components/feature_engagement_tracker/internal/model.h
[modify] https://crrev.com/0d89c8a2635c58c952c33c22a71d9a7f3ea564b5/components/feature_engagement_tracker/internal/model_impl.cc
[modify] https://crrev.com/0d89c8a2635c58c952c33c22a71d9a7f3ea564b5/components/feature_engagement_tracker/internal/model_impl.h
[modify] https://crrev.com/0d89c8a2635c58c952c33c22a71d9a7f3ea564b5/components/feature_engagement_tracker/internal/model_impl_unittest.cc
[modify] https://crrev.com/0d89c8a2635c58c952c33c22a71d9a7f3ea564b5/components/feature_engagement_tracker/internal/never_condition_validator.cc
[modify] https://crrev.com/0d89c8a2635c58c952c33c22a71d9a7f3ea564b5/components/feature_engagement_tracker/internal/never_condition_validator.h
[modify] https://crrev.com/0d89c8a2635c58c952c33c22a71d9a7f3ea564b5/components/feature_engagement_tracker/internal/never_condition_validator_unittest.cc
[add] https://crrev.com/0d89c8a2635c58c952c33c22a71d9a7f3ea564b5/components/feature_engagement_tracker/internal/never_storage_validator.cc
[add] https://crrev.com/0d89c8a2635c58c952c33c22a71d9a7f3ea564b5/components/feature_engagement_tracker/internal/never_storage_validator.h
[add] https://crrev.com/0d89c8a2635c58c952c33c22a71d9a7f3ea564b5/components/feature_engagement_tracker/internal/never_storage_validator_unittest.cc
[modify] https://crrev.com/0d89c8a2635c58c952c33c22a71d9a7f3ea564b5/components/feature_engagement_tracker/internal/once_condition_validator.cc
[modify] https://crrev.com/0d89c8a2635c58c952c33c22a71d9a7f3ea564b5/components/feature_engagement_tracker/internal/once_condition_validator.h
[modify] https://crrev.com/0d89c8a2635c58c952c33c22a71d9a7f3ea564b5/components/feature_engagement_tracker/internal/once_condition_validator_unittest.cc
[add] https://crrev.com/0d89c8a2635c58c952c33c22a71d9a7f3ea564b5/components/feature_engagement_tracker/internal/storage_validator.h

Project Member

Comment 10 by bugdroid1@chromium.org, Apr 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e880fc81c7cda2ce18d8b95c58fbbd6d0c163661

commit e880fc81c7cda2ce18d8b95c58fbbd6d0c163661
Author: Tommy Nyquist <nyquist@chromium.org>
Date: Fri Apr 28 02:21:47 2017

Add support for initialization callback for In-Product Help

This CL adds support for invoking a callback when the underlying model
has finished loading. In addition, it exposes the restul of the
initialization as a boolean method.

The result of the initialization is always posted back, even if the
result is already known.

The CL implements both the C++ and the Java bridge implementation of
this.

BUG= 706309 

Change-Id: I09310d658ed0d0b9a9ef78dfd401d3ef49dd339e
Reviewed-on: https://chromium-review.googlesource.com/486969
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#467851}
[modify] https://crrev.com/e880fc81c7cda2ce18d8b95c58fbbd6d0c163661/components/feature_engagement_tracker/internal/android/feature_engagement_tracker_impl_android.cc
[modify] https://crrev.com/e880fc81c7cda2ce18d8b95c58fbbd6d0c163661/components/feature_engagement_tracker/internal/android/feature_engagement_tracker_impl_android.h
[modify] https://crrev.com/e880fc81c7cda2ce18d8b95c58fbbd6d0c163661/components/feature_engagement_tracker/internal/android/java/src/org/chromium/components/feature_engagement_tracker/internal/FeatureEngagementTrackerImpl.java
[modify] https://crrev.com/e880fc81c7cda2ce18d8b95c58fbbd6d0c163661/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl.cc
[modify] https://crrev.com/e880fc81c7cda2ce18d8b95c58fbbd6d0c163661/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl.h
[modify] https://crrev.com/e880fc81c7cda2ce18d8b95c58fbbd6d0c163661/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl_unittest.cc
[modify] https://crrev.com/e880fc81c7cda2ce18d8b95c58fbbd6d0c163661/components/feature_engagement_tracker/internal/model.h
[modify] https://crrev.com/e880fc81c7cda2ce18d8b95c58fbbd6d0c163661/components/feature_engagement_tracker/public/android/java/src/org/chromium/components/feature_engagement_tracker/FeatureEngagementTracker.java
[modify] https://crrev.com/e880fc81c7cda2ce18d8b95c58fbbd6d0c163661/components/feature_engagement_tracker/public/feature_engagement_tracker.h

Project Member

Comment 11 by bugdroid1@chromium.org, May 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d1f67520a79d83e0158662a3c70ab9c9a25fbed2

commit d1f67520a79d83e0158662a3c70ab9c9a25fbed2
Author: Tommy Nyquist <nyquist@chromium.org>
Date: Tue May 09 21:32:30 2017

Fix comment typos in FeatureConfigStorageValidator

The initial implementation of FeatureConfigStorageValidator was based
on being a part of the ConditionValidator, but the CL ended up creating
a totally separate API for validating storage conditions, the
StorageValidator. However, some of the comments were not updated as
part of that change, so they still refer to the ConditionValidator.

Also the class comment for the FeatureConfigStorageValidator ended
too abruptly, so that comment has been briefly expanded.

BUG= 706309 

Change-Id: I1fc93c48b66bb8b2a2555c1670cfd7a6e09f622e
Reviewed-on: https://chromium-review.googlesource.com/497248
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#470420}
[modify] https://crrev.com/d1f67520a79d83e0158662a3c70ab9c9a25fbed2/components/feature_engagement_tracker/internal/feature_config_storage_validator.h

Project Member

Comment 12 by bugdroid1@chromium.org, May 10 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0b53efdb492a6633f713cc5a337b87d931da672b

commit 0b53efdb492a6633f713cc5a337b87d931da672b
Author: Tommy Nyquist <nyquist@chromium.org>
Date: Wed May 10 07:08:21 2017

Clarify public API for In-Product Help

This CL moves the feature list and the feature constant definitions
to the public directory so developers do not have to edit the
internal parts for adding new features or events. This makes the
C++ code behave in a similar manner as the Java API.

In addition, this CL adds a requirement to now pass the feature
in when dismissing the current In-Product Help. This is to ensure
that we can safely handle multiple In-Product Help features in
the future without involving callers. The current features have been
updated to pass in the feature.

BUG= 706309 
TBR=miguelg@chromium.org

Change-Id: I8fbd2ef19afae1e754cd3b3fd5da2c75cb4e38e5
Reviewed-on: https://chromium-review.googlesource.com/497008
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#470510}
[modify] https://crrev.com/0b53efdb492a6633f713cc5a337b87d931da672b/chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java
[modify] https://crrev.com/0b53efdb492a6633f713cc5a337b87d931da672b/chrome/android/java/src/org/chromium/chrome/browser/infobar/IPHInfoBarSupport.java
[modify] https://crrev.com/0b53efdb492a6633f713cc5a337b87d931da672b/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java
[modify] https://crrev.com/0b53efdb492a6633f713cc5a337b87d931da672b/components/feature_engagement_tracker/internal/BUILD.gn
[modify] https://crrev.com/0b53efdb492a6633f713cc5a337b87d931da672b/components/feature_engagement_tracker/internal/android/feature_engagement_tracker_impl_android.cc
[modify] https://crrev.com/0b53efdb492a6633f713cc5a337b87d931da672b/components/feature_engagement_tracker/internal/android/feature_engagement_tracker_impl_android.h
[modify] https://crrev.com/0b53efdb492a6633f713cc5a337b87d931da672b/components/feature_engagement_tracker/internal/android/java/src/org/chromium/components/feature_engagement_tracker/internal/FeatureEngagementTrackerImpl.java
[modify] https://crrev.com/0b53efdb492a6633f713cc5a337b87d931da672b/components/feature_engagement_tracker/internal/chrome_variations_configuration.cc
[modify] https://crrev.com/0b53efdb492a6633f713cc5a337b87d931da672b/components/feature_engagement_tracker/internal/chrome_variations_configuration.h
[modify] https://crrev.com/0b53efdb492a6633f713cc5a337b87d931da672b/components/feature_engagement_tracker/internal/condition_validator.h
[modify] https://crrev.com/0b53efdb492a6633f713cc5a337b87d931da672b/components/feature_engagement_tracker/internal/feature_config_storage_validator.cc
[modify] https://crrev.com/0b53efdb492a6633f713cc5a337b87d931da672b/components/feature_engagement_tracker/internal/feature_config_storage_validator.h
[modify] https://crrev.com/0b53efdb492a6633f713cc5a337b87d931da672b/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl.cc
[modify] https://crrev.com/0b53efdb492a6633f713cc5a337b87d931da672b/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl.h
[modify] https://crrev.com/0b53efdb492a6633f713cc5a337b87d931da672b/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl_unittest.cc
[modify] https://crrev.com/0b53efdb492a6633f713cc5a337b87d931da672b/components/feature_engagement_tracker/internal/never_condition_validator.h
[modify] https://crrev.com/0b53efdb492a6633f713cc5a337b87d931da672b/components/feature_engagement_tracker/internal/once_condition_validator.h
[modify] https://crrev.com/0b53efdb492a6633f713cc5a337b87d931da672b/components/feature_engagement_tracker/public/BUILD.gn
[modify] https://crrev.com/0b53efdb492a6633f713cc5a337b87d931da672b/components/feature_engagement_tracker/public/android/java/src/org/chromium/components/feature_engagement_tracker/FeatureEngagementTracker.java
[rename] https://crrev.com/0b53efdb492a6633f713cc5a337b87d931da672b/components/feature_engagement_tracker/public/feature_constants.cc
[modify] https://crrev.com/0b53efdb492a6633f713cc5a337b87d931da672b/components/feature_engagement_tracker/public/feature_engagement_tracker.h
[rename] https://crrev.com/0b53efdb492a6633f713cc5a337b87d931da672b/components/feature_engagement_tracker/public/feature_list.cc
[rename] https://crrev.com/0b53efdb492a6633f713cc5a337b87d931da672b/components/feature_engagement_tracker/public/feature_list.h

Project Member

Comment 13 by bugdroid1@chromium.org, May 10 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6867b355536313ae31b33d248179167aedf03351

commit 6867b355536313ae31b33d248179167aedf03351
Author: Tommy Nyquist <nyquist@chromium.org>
Date: Wed May 10 16:13:17 2017

Add helper filter for In-Product Help.

The feature_engagement_tracker components uses its own namespace, and
the classes within the namespace does not have any particular prefix. This
leads to manually listing all the test suites whenever tests should be run
locally and one does not want to run all the tests in components_unittests.

This CL adds a helper filter-file to be used with the gtests with the arg:
--test-launcher-filter-file=components/feature_engagement_tracker/components_unittests.filter

In addition, this CL adds a helpful section for how to compile the code
on other platforms than Android.

BUG= 706309 

Change-Id: If8e9e73346dc0228d8327f96d81424ccf9ff7905
Reviewed-on: https://chromium-review.googlesource.com/496967
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#470596}
[modify] https://crrev.com/6867b355536313ae31b33d248179167aedf03351/components/feature_engagement_tracker/BUILD.gn
[modify] https://crrev.com/6867b355536313ae31b33d248179167aedf03351/components/feature_engagement_tracker/README.md
[add] https://crrev.com/6867b355536313ae31b33d248179167aedf03351/components/feature_engagement_tracker/components_unittests.filter
[modify] https://crrev.com/6867b355536313ae31b33d248179167aedf03351/components/feature_engagement_tracker/internal/BUILD.gn

Project Member

Comment 14 by bugdroid1@chromium.org, May 10 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/43ae6a4ea6c789b9ce52f502a3bb9707b849c2a5

commit 43ae6a4ea6c789b9ce52f502a3bb9707b849c2a5
Author: Tommy Nyquist <nyquist@chromium.org>
Date: Wed May 10 19:18:06 2017

Change the internal API for the In-Product Help Model

The current Model for the Feature Engagement Tracker has a method
GetEvent(...). This model currently returns a const Event&, which
provides a clean an simple API for callers, knowing that the returned
value can never be nullptr. However, this also forces the method to
not be const, since it inserts an empty Event into the internal
map of all events.

This makes it impossible to pass around a const& to the Model whenever
someone needs to look up events, which does not feel right.

This CL therefore changes that method declaration to be:
const Event* GetEvent(const std::string& event_name) const;

It then of course updates all the relevant code to now possibly expect
nullptr values.

BUG= 706309 

Change-Id: I54a091fd22c8a3e02aed5f290ff83645b72e1b67
Reviewed-on: https://chromium-review.googlesource.com/501447
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: Vladimir Levin <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#470665}
[modify] https://crrev.com/43ae6a4ea6c789b9ce52f502a3bb9707b849c2a5/components/feature_engagement_tracker/internal/model.h
[modify] https://crrev.com/43ae6a4ea6c789b9ce52f502a3bb9707b849c2a5/components/feature_engagement_tracker/internal/model_impl.cc
[modify] https://crrev.com/43ae6a4ea6c789b9ce52f502a3bb9707b849c2a5/components/feature_engagement_tracker/internal/model_impl.h
[modify] https://crrev.com/43ae6a4ea6c789b9ce52f502a3bb9707b849c2a5/components/feature_engagement_tracker/internal/model_impl_unittest.cc
[modify] https://crrev.com/43ae6a4ea6c789b9ce52f502a3bb9707b849c2a5/components/feature_engagement_tracker/internal/never_condition_validator_unittest.cc
[modify] https://crrev.com/43ae6a4ea6c789b9ce52f502a3bb9707b849c2a5/components/feature_engagement_tracker/internal/once_condition_validator_unittest.cc

Project Member

Comment 15 by bugdroid1@chromium.org, May 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e28c0c1a5392c74ca50269bcc6066815ecdaa062

commit e28c0c1a5392c74ca50269bcc6066815ecdaa062
Author: Tommy Nyquist <nyquist@chromium.org>
Date: Thu May 11 03:06:20 2017

Make Comparator able to compare against criterias.

For the feature_engagement_tracker component, the Comparator struct
contains what to compare something against, but was just a dumb
struct, with no logic attached to it.

This CL adds support for checking whether a particular value meets
the expectations for a comparator. It makes sense doing this as a part
of the comparator, because it can then be re-used across the component.

This will be used later when comparing the FeatureConfig against real
Event instances.

BUG= 706309 

Change-Id: Ie06a85454ad39a3c5e854856be11a3f774843568
Reviewed-on: https://chromium-review.googlesource.com/500990
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#470788}
[modify] https://crrev.com/e28c0c1a5392c74ca50269bcc6066815ecdaa062/components/feature_engagement_tracker/components_unittests.filter
[modify] https://crrev.com/e28c0c1a5392c74ca50269bcc6066815ecdaa062/components/feature_engagement_tracker/internal/BUILD.gn
[modify] https://crrev.com/e28c0c1a5392c74ca50269bcc6066815ecdaa062/components/feature_engagement_tracker/internal/configuration.cc
[modify] https://crrev.com/e28c0c1a5392c74ca50269bcc6066815ecdaa062/components/feature_engagement_tracker/internal/configuration.h
[add] https://crrev.com/e28c0c1a5392c74ca50269bcc6066815ecdaa062/components/feature_engagement_tracker/internal/configuration_unittest.cc

Project Member

Comment 16 by bugdroid1@chromium.org, May 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8afbdf7d2bfa9779d548d9d131ae0415813453fb

commit 8afbdf7d2bfa9779d548d9d131ae0415813453fb
Author: Tommy Nyquist <nyquist@chromium.org>
Date: Thu May 11 07:37:47 2017

Create TimeProvider for Feature Engagement Tracker

The feature_engagement_tracker Model was supposed to return the number
of days since UNIX epoch, but up until now it has been hard coded to
return 0u.

This CL moves that method out of the Model interface and instead creates
a new TimeProvider interface. It also adds an implementation of this
that uses base::Time to figure out the current number of days since
epoch and adds a test to ensure that it behaves like expected.

In addition, the new interface makes the GetCurrentDay() method const,
as it should never change any internal state.

Lastly, it changes the Model interface to now take in the current
day as a parameter to IncrementEvent(...), which simplifies testing
quite a bit, and makes it clear that it is only for that method the
current day is required. This also made it possible for the
FeatureEngagementTrackerImpl to own the TimeProvider, which it will
use both for calls to the Model and the ConditionValidator.

BUG= 706309 

Change-Id: I74b066a6b3d59786fed2e2f70498605d9f9753db
Reviewed-on: https://chromium-review.googlesource.com/501469
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#470862}
[modify] https://crrev.com/8afbdf7d2bfa9779d548d9d131ae0415813453fb/components/feature_engagement_tracker/components_unittests.filter
[modify] https://crrev.com/8afbdf7d2bfa9779d548d9d131ae0415813453fb/components/feature_engagement_tracker/internal/BUILD.gn
[modify] https://crrev.com/8afbdf7d2bfa9779d548d9d131ae0415813453fb/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl.cc
[modify] https://crrev.com/8afbdf7d2bfa9779d548d9d131ae0415813453fb/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl.h
[modify] https://crrev.com/8afbdf7d2bfa9779d548d9d131ae0415813453fb/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl_unittest.cc
[modify] https://crrev.com/8afbdf7d2bfa9779d548d9d131ae0415813453fb/components/feature_engagement_tracker/internal/model.h
[modify] https://crrev.com/8afbdf7d2bfa9779d548d9d131ae0415813453fb/components/feature_engagement_tracker/internal/model_impl.cc
[modify] https://crrev.com/8afbdf7d2bfa9779d548d9d131ae0415813453fb/components/feature_engagement_tracker/internal/model_impl.h
[modify] https://crrev.com/8afbdf7d2bfa9779d548d9d131ae0415813453fb/components/feature_engagement_tracker/internal/model_impl_unittest.cc
[modify] https://crrev.com/8afbdf7d2bfa9779d548d9d131ae0415813453fb/components/feature_engagement_tracker/internal/never_condition_validator_unittest.cc
[modify] https://crrev.com/8afbdf7d2bfa9779d548d9d131ae0415813453fb/components/feature_engagement_tracker/internal/once_condition_validator_unittest.cc
[add] https://crrev.com/8afbdf7d2bfa9779d548d9d131ae0415813453fb/components/feature_engagement_tracker/internal/system_time_provider.cc
[add] https://crrev.com/8afbdf7d2bfa9779d548d9d131ae0415813453fb/components/feature_engagement_tracker/internal/system_time_provider.h
[add] https://crrev.com/8afbdf7d2bfa9779d548d9d131ae0415813453fb/components/feature_engagement_tracker/internal/system_time_provider_unittest.cc
[add] https://crrev.com/8afbdf7d2bfa9779d548d9d131ae0415813453fb/components/feature_engagement_tracker/internal/time_provider.h

Project Member

Comment 17 by bugdroid1@chromium.org, May 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6a960f89971032ab3809d50a4116c0f40c877fd3

commit 6a960f89971032ab3809d50a4116c0f40c877fd3
Author: Tommy Nyquist <nyquist@chromium.org>
Date: Thu May 11 21:29:35 2017

Add Result struct for ConditionValidator API.

Before this CL, the ConditionValidator::MeetsConditions(...) method only
returned a bool value whether all conditions were met. This made it
harder to track what went wrong in a generic way across all validators,
so this CL changes the return value to be a struct that contains
information about all conditions, and whether they were met.

In addition, to prepare for an upcoming CL that will need to use the
current day to figure out whether conditions are met, that parameter
is added in the current CL.

BUG= 706309 

Change-Id: I73815d88facfc1fbee9e6a0058790cd4b73dc714
Reviewed-on: https://chromium-review.googlesource.com/502731
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#471078}
[modify] https://crrev.com/6a960f89971032ab3809d50a4116c0f40c877fd3/components/feature_engagement_tracker/components_unittests.filter
[modify] https://crrev.com/6a960f89971032ab3809d50a4116c0f40c877fd3/components/feature_engagement_tracker/internal/BUILD.gn
[add] https://crrev.com/6a960f89971032ab3809d50a4116c0f40c877fd3/components/feature_engagement_tracker/internal/condition_validator.cc
[modify] https://crrev.com/6a960f89971032ab3809d50a4116c0f40c877fd3/components/feature_engagement_tracker/internal/condition_validator.h
[add] https://crrev.com/6a960f89971032ab3809d50a4116c0f40c877fd3/components/feature_engagement_tracker/internal/condition_validator_unittest.cc
[modify] https://crrev.com/6a960f89971032ab3809d50a4116c0f40c877fd3/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl.cc
[modify] https://crrev.com/6a960f89971032ab3809d50a4116c0f40c877fd3/components/feature_engagement_tracker/internal/never_condition_validator.cc
[modify] https://crrev.com/6a960f89971032ab3809d50a4116c0f40c877fd3/components/feature_engagement_tracker/internal/never_condition_validator.h
[modify] https://crrev.com/6a960f89971032ab3809d50a4116c0f40c877fd3/components/feature_engagement_tracker/internal/never_condition_validator_unittest.cc
[modify] https://crrev.com/6a960f89971032ab3809d50a4116c0f40c877fd3/components/feature_engagement_tracker/internal/once_condition_validator.cc
[modify] https://crrev.com/6a960f89971032ab3809d50a4116c0f40c877fd3/components/feature_engagement_tracker/internal/once_condition_validator.h
[modify] https://crrev.com/6a960f89971032ab3809d50a4116c0f40c877fd3/components/feature_engagement_tracker/internal/once_condition_validator_unittest.cc

Project Member

Comment 18 by bugdroid1@chromium.org, May 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/23b616383acde9c16e0892084498620ecc46856f

commit 23b616383acde9c16e0892084498620ecc46856f
Author: dtrainor <dtrainor@chromium.org>
Date: Fri May 12 00:37:43 2017

Add a PersistentStore to FeatureEngagementTracker

Currently we use an InMemoryStore for the FeatureEngagementTracker,
which doesn't persist feature Events beyond the scope of the application
lifetime.  However once we move beyond demo mode, the feature engagement
conditions will span months, so we need to track the data for longer
than a single session.

This CL implements the Store interface and backs it with a
leveldb_proto::ProtoDatabase.

BUG= 706309 

Review-Url: https://codereview.chromium.org/2876633002
Cr-Commit-Position: refs/heads/master@{#471149}

[modify] https://crrev.com/23b616383acde9c16e0892084498620ecc46856f/components/feature_engagement_tracker/DEPS
[modify] https://crrev.com/23b616383acde9c16e0892084498620ecc46856f/components/feature_engagement_tracker/components_unittests.filter
[modify] https://crrev.com/23b616383acde9c16e0892084498620ecc46856f/components/feature_engagement_tracker/internal/BUILD.gn
[modify] https://crrev.com/23b616383acde9c16e0892084498620ecc46856f/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl.cc
[modify] https://crrev.com/23b616383acde9c16e0892084498620ecc46856f/components/feature_engagement_tracker/internal/model_impl_unittest.cc
[add] https://crrev.com/23b616383acde9c16e0892084498620ecc46856f/components/feature_engagement_tracker/internal/persistent_store.cc
[add] https://crrev.com/23b616383acde9c16e0892084498620ecc46856f/components/feature_engagement_tracker/internal/persistent_store.h
[add] https://crrev.com/23b616383acde9c16e0892084498620ecc46856f/components/feature_engagement_tracker/internal/persistent_store_unittest.cc
[add] https://crrev.com/23b616383acde9c16e0892084498620ecc46856f/components/feature_engagement_tracker/internal/test/BUILD.gn
[add] https://crrev.com/23b616383acde9c16e0892084498620ecc46856f/components/feature_engagement_tracker/internal/test/event_util.cc
[add] https://crrev.com/23b616383acde9c16e0892084498620ecc46856f/components/feature_engagement_tracker/internal/test/event_util.h
[modify] https://crrev.com/23b616383acde9c16e0892084498620ecc46856f/tools/metrics/histograms/histograms.xml

Project Member

Comment 19 by bugdroid1@chromium.org, May 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6f323a5f1fd9ad313745573f945739541dd40161

commit 6f323a5f1fd9ad313745573f945739541dd40161
Author: Tommy Nyquist <nyquist@chromium.org>
Date: Fri May 12 16:52:14 2017

Move tracking of showing in-product help to ConditionValidator.

Before this CL, the Model was tracking whether any in-product help was
currently being displayed. However, this was never used by the model
itself.

This CL instead moves the tracking of that to the ConditionValidator
interface, which is the code that will eventually need to inspect the
state anyway. This simplifies the Model interface, and makes it
possible to make ConditionValidator::MeetsConditions(...) const since
the OnceConditionValidator now does not edit internal state anymore.

This also fits much better with the upcoming ConditionValidator
which uses FeatureConfig to validate conditions.

BUG= 706309 

Change-Id: Id97c5275c7c0fc5665afcb5a47c5bdd181b8eff1
Reviewed-on: https://chromium-review.googlesource.com/503441
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#471340}
[modify] https://crrev.com/6f323a5f1fd9ad313745573f945739541dd40161/components/feature_engagement_tracker/internal/condition_validator.h
[modify] https://crrev.com/6f323a5f1fd9ad313745573f945739541dd40161/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl.cc
[modify] https://crrev.com/6f323a5f1fd9ad313745573f945739541dd40161/components/feature_engagement_tracker/internal/model.h
[modify] https://crrev.com/6f323a5f1fd9ad313745573f945739541dd40161/components/feature_engagement_tracker/internal/model_impl.cc
[modify] https://crrev.com/6f323a5f1fd9ad313745573f945739541dd40161/components/feature_engagement_tracker/internal/model_impl.h
[modify] https://crrev.com/6f323a5f1fd9ad313745573f945739541dd40161/components/feature_engagement_tracker/internal/model_impl_unittest.cc
[modify] https://crrev.com/6f323a5f1fd9ad313745573f945739541dd40161/components/feature_engagement_tracker/internal/never_condition_validator.cc
[modify] https://crrev.com/6f323a5f1fd9ad313745573f945739541dd40161/components/feature_engagement_tracker/internal/never_condition_validator.h
[modify] https://crrev.com/6f323a5f1fd9ad313745573f945739541dd40161/components/feature_engagement_tracker/internal/never_condition_validator_unittest.cc
[modify] https://crrev.com/6f323a5f1fd9ad313745573f945739541dd40161/components/feature_engagement_tracker/internal/once_condition_validator.cc
[modify] https://crrev.com/6f323a5f1fd9ad313745573f945739541dd40161/components/feature_engagement_tracker/internal/once_condition_validator.h
[modify] https://crrev.com/6f323a5f1fd9ad313745573f945739541dd40161/components/feature_engagement_tracker/internal/once_condition_validator_unittest.cc

Project Member

Comment 20 by bugdroid1@chromium.org, May 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a845da1dc48a9d9cccabe3f93890ffa4f439df8d

commit a845da1dc48a9d9cccabe3f93890ffa4f439df8d
Author: Tommy Nyquist <nyquist@chromium.org>
Date: Fri May 12 17:17:29 2017

Move ownership of Configuration to FeatureEngagementTrackerImpl

Up until now, the Model owned the configuration, and provided a wrapper
method to retrieve configuration for particular features. This was
totally unnecessary, and it is better to just pass in the correct
FeatureConfig where it is needed.

In this CL we therefore start passing FeatureConfig as a parameter to
the ConditionValidator::MeetsConditions(...) method, which is exactly
where it is needed. Since the FeatureEngagementTrackerImpl is the one
invoking the ConditionValidator, it is also now the one that will own
the Configuration. Doing that also simplified the testing quite a bit,
since there was no longer any need for registering FeatureConfigs up
front.

Overall, this simplifies the Model to now only be dealing with the
events themselves.

Lastly, through this work it was found that it was unnecessary to
initialize the ScopedFeatureList in most cases, except for Chrome
Variations code, so this has also been removed.

BUG= 706309 

Change-Id: Ibf01faaa7009e17553bbc6c20e4f55907a5d4634
Reviewed-on: https://chromium-review.googlesource.com/503540
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#471348}
[modify] https://crrev.com/a845da1dc48a9d9cccabe3f93890ffa4f439df8d/components/feature_engagement_tracker/internal/condition_validator.h
[modify] https://crrev.com/a845da1dc48a9d9cccabe3f93890ffa4f439df8d/components/feature_engagement_tracker/internal/editable_configuration_unittest.cc
[modify] https://crrev.com/a845da1dc48a9d9cccabe3f93890ffa4f439df8d/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl.cc
[modify] https://crrev.com/a845da1dc48a9d9cccabe3f93890ffa4f439df8d/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl.h
[modify] https://crrev.com/a845da1dc48a9d9cccabe3f93890ffa4f439df8d/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl_unittest.cc
[modify] https://crrev.com/a845da1dc48a9d9cccabe3f93890ffa4f439df8d/components/feature_engagement_tracker/internal/model.h
[modify] https://crrev.com/a845da1dc48a9d9cccabe3f93890ffa4f439df8d/components/feature_engagement_tracker/internal/model_impl.cc
[modify] https://crrev.com/a845da1dc48a9d9cccabe3f93890ffa4f439df8d/components/feature_engagement_tracker/internal/model_impl.h
[modify] https://crrev.com/a845da1dc48a9d9cccabe3f93890ffa4f439df8d/components/feature_engagement_tracker/internal/model_impl_unittest.cc
[modify] https://crrev.com/a845da1dc48a9d9cccabe3f93890ffa4f439df8d/components/feature_engagement_tracker/internal/never_condition_validator.cc
[modify] https://crrev.com/a845da1dc48a9d9cccabe3f93890ffa4f439df8d/components/feature_engagement_tracker/internal/never_condition_validator.h
[modify] https://crrev.com/a845da1dc48a9d9cccabe3f93890ffa4f439df8d/components/feature_engagement_tracker/internal/never_condition_validator_unittest.cc
[modify] https://crrev.com/a845da1dc48a9d9cccabe3f93890ffa4f439df8d/components/feature_engagement_tracker/internal/once_condition_validator.cc
[modify] https://crrev.com/a845da1dc48a9d9cccabe3f93890ffa4f439df8d/components/feature_engagement_tracker/internal/once_condition_validator.h
[modify] https://crrev.com/a845da1dc48a9d9cccabe3f93890ffa4f439df8d/components/feature_engagement_tracker/internal/once_condition_validator_unittest.cc
[modify] https://crrev.com/a845da1dc48a9d9cccabe3f93890ffa4f439df8d/components/feature_engagement_tracker/internal/single_invalid_configuration_unittest.cc

Project Member

Comment 21 by bugdroid1@chromium.org, May 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bc268040daf0c3e94df0a97e1d9e2eb9c39a2e16

commit bc268040daf0c3e94df0a97e1d9e2eb9c39a2e16
Author: Tommy Nyquist <nyquist@chromium.org>
Date: Fri May 12 17:46:11 2017

Ensure that FeatureConfigStorageValidator verifies features are enabled.

Up until now, the FeatureConfigStorageValidator ignored the fact whether
a feature was enabled or not, which was incorrect. We should never store
events for features that are enabled.

This CL changes the code to only use the config from enabled features,
and adds a test to ensure that it behaves as expected.

BUG= 706309 

Change-Id: I5d24b7bfc08942dba9e5e0f0a83bb056b5a73acf
Reviewed-on: https://chromium-review.googlesource.com/503624
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#471354}
[modify] https://crrev.com/bc268040daf0c3e94df0a97e1d9e2eb9c39a2e16/components/feature_engagement_tracker/internal/feature_config_storage_validator.cc
[modify] https://crrev.com/bc268040daf0c3e94df0a97e1d9e2eb9c39a2e16/components/feature_engagement_tracker/internal/feature_config_storage_validator_unittest.cc

Project Member

Comment 22 by bugdroid1@chromium.org, May 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e1663d2c4db343e0285d6f2cc834654d00383c82

commit e1663d2c4db343e0285d6f2cc834654d00383c82
Author: Tommy Nyquist <nyquist@chromium.org>
Date: Fri May 12 19:22:08 2017

Add ConditionValidator that uses FeatureConfig as source of truth.

This CL adds a ConditionValidator that uses the parsed FeatureConfig
as the ruleset for validating whether all conditions are met. This is
the ConditionValidator that is intended to be used in production.

For now, the availability precondition is ignored, as we do not have a
way yet to know when a particular in-product help was made available.

One extra ConditionValidator::Result has also been added to ensure that
only enabled features will ever be shown.

BUG= 706309 

Change-Id: I5ecbb93baac4137bb5bb9ef9177697faafbc1003
Reviewed-on: https://chromium-review.googlesource.com/504087
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#471403}
[modify] https://crrev.com/e1663d2c4db343e0285d6f2cc834654d00383c82/components/feature_engagement_tracker/components_unittests.filter
[modify] https://crrev.com/e1663d2c4db343e0285d6f2cc834654d00383c82/components/feature_engagement_tracker/internal/BUILD.gn
[modify] https://crrev.com/e1663d2c4db343e0285d6f2cc834654d00383c82/components/feature_engagement_tracker/internal/condition_validator.cc
[modify] https://crrev.com/e1663d2c4db343e0285d6f2cc834654d00383c82/components/feature_engagement_tracker/internal/condition_validator.h
[modify] https://crrev.com/e1663d2c4db343e0285d6f2cc834654d00383c82/components/feature_engagement_tracker/internal/condition_validator_unittest.cc
[add] https://crrev.com/e1663d2c4db343e0285d6f2cc834654d00383c82/components/feature_engagement_tracker/internal/feature_config_condition_validator.cc
[add] https://crrev.com/e1663d2c4db343e0285d6f2cc834654d00383c82/components/feature_engagement_tracker/internal/feature_config_condition_validator.h
[add] https://crrev.com/e1663d2c4db343e0285d6f2cc834654d00383c82/components/feature_engagement_tracker/internal/feature_config_condition_validator_unittest.cc

Project Member

Comment 23 by bugdroid1@chromium.org, May 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cfc46a6fe55792b4e5ed1255991355953131d39e

commit cfc46a6fe55792b4e5ed1255991355953131d39e
Author: Tommy Nyquist <nyquist@chromium.org>
Date: Fri May 12 19:41:44 2017

Start using StorageValidator in ModelImpl

Up until now, the ModelImpl for the FeatureEngagementTracker has not
used the StorageValidator at all. However, this is necessary to ensure
the Store interactions are behaving as expected.

This CL starts dropping new events on the floor if the StorageValidator
says new events should not be stored for a particular event.

It also filters all old events during load to ensure it only keeps
events that meet the StorageValidator restrictions:
- Events where all activity is kept is not written back to the Store.
- Events where some activity is removed is written back to the Store.
- Events with no more activity is fully deleted from the Store.
The in-memory representation matches what is stored in the database.

This required a few changes such as passing through the current day
during initialization, but more imporantly required adding support
for deleting events from the store.

BUG= 706309 

Change-Id: I28b4711a553401b5b801be19f3d673eb61706cc8
Reviewed-on: https://chromium-review.googlesource.com/504153
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#471411}
[modify] https://crrev.com/cfc46a6fe55792b4e5ed1255991355953131d39e/components/feature_engagement_tracker/internal/feature_config_condition_validator_unittest.cc
[modify] https://crrev.com/cfc46a6fe55792b4e5ed1255991355953131d39e/components/feature_engagement_tracker/internal/feature_config_storage_validator.cc
[modify] https://crrev.com/cfc46a6fe55792b4e5ed1255991355953131d39e/components/feature_engagement_tracker/internal/feature_config_storage_validator.h
[modify] https://crrev.com/cfc46a6fe55792b4e5ed1255991355953131d39e/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl.cc
[modify] https://crrev.com/cfc46a6fe55792b4e5ed1255991355953131d39e/components/feature_engagement_tracker/internal/in_memory_store.cc
[modify] https://crrev.com/cfc46a6fe55792b4e5ed1255991355953131d39e/components/feature_engagement_tracker/internal/in_memory_store.h
[modify] https://crrev.com/cfc46a6fe55792b4e5ed1255991355953131d39e/components/feature_engagement_tracker/internal/model.h
[modify] https://crrev.com/cfc46a6fe55792b4e5ed1255991355953131d39e/components/feature_engagement_tracker/internal/model_impl.cc
[modify] https://crrev.com/cfc46a6fe55792b4e5ed1255991355953131d39e/components/feature_engagement_tracker/internal/model_impl.h
[modify] https://crrev.com/cfc46a6fe55792b4e5ed1255991355953131d39e/components/feature_engagement_tracker/internal/model_impl_unittest.cc
[modify] https://crrev.com/cfc46a6fe55792b4e5ed1255991355953131d39e/components/feature_engagement_tracker/internal/never_condition_validator_unittest.cc
[modify] https://crrev.com/cfc46a6fe55792b4e5ed1255991355953131d39e/components/feature_engagement_tracker/internal/never_storage_validator.cc
[modify] https://crrev.com/cfc46a6fe55792b4e5ed1255991355953131d39e/components/feature_engagement_tracker/internal/never_storage_validator.h
[modify] https://crrev.com/cfc46a6fe55792b4e5ed1255991355953131d39e/components/feature_engagement_tracker/internal/once_condition_validator_unittest.cc
[modify] https://crrev.com/cfc46a6fe55792b4e5ed1255991355953131d39e/components/feature_engagement_tracker/internal/persistent_store.cc
[modify] https://crrev.com/cfc46a6fe55792b4e5ed1255991355953131d39e/components/feature_engagement_tracker/internal/persistent_store.h
[modify] https://crrev.com/cfc46a6fe55792b4e5ed1255991355953131d39e/components/feature_engagement_tracker/internal/persistent_store_unittest.cc
[modify] https://crrev.com/cfc46a6fe55792b4e5ed1255991355953131d39e/components/feature_engagement_tracker/internal/storage_validator.h
[modify] https://crrev.com/cfc46a6fe55792b4e5ed1255991355953131d39e/components/feature_engagement_tracker/internal/store.h

Project Member

Comment 24 by bugdroid1@chromium.org, May 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f5bb53683cec36b37334b069b35b7c42b76f59a0

commit f5bb53683cec36b37334b069b35b7c42b76f59a0
Author: dtrainor <dtrainor@chromium.org>
Date: Sat May 13 01:20:38 2017

Support IPH NotifyEvent calls before Initialize

Previously any attempt to call FeatureEngagementTracker::NotifyEvent()
would crash if the Model and Store had not finished initializing yet.
This adds support for queuing up calls to NotifyEvent() so that once the
underlying Model and Store are initialized they can properly receive the
calls.

This is done by building a Model implementation that wraps another Model
implementation and queues up calls if the underlying Model is not ready.

BUG= 706309 

Review-Url: https://codereview.chromium.org/2877243002
Cr-Commit-Position: refs/heads/master@{#471528}

[modify] https://crrev.com/f5bb53683cec36b37334b069b35b7c42b76f59a0/components/feature_engagement_tracker/components_unittests.filter
[modify] https://crrev.com/f5bb53683cec36b37334b069b35b7c42b76f59a0/components/feature_engagement_tracker/internal/BUILD.gn
[modify] https://crrev.com/f5bb53683cec36b37334b069b35b7c42b76f59a0/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl.cc
[modify] https://crrev.com/f5bb53683cec36b37334b069b35b7c42b76f59a0/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl.h
[modify] https://crrev.com/f5bb53683cec36b37334b069b35b7c42b76f59a0/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl_unittest.cc
[add] https://crrev.com/f5bb53683cec36b37334b069b35b7c42b76f59a0/components/feature_engagement_tracker/internal/init_aware_model.cc
[add] https://crrev.com/f5bb53683cec36b37334b069b35b7c42b76f59a0/components/feature_engagement_tracker/internal/init_aware_model.h
[add] https://crrev.com/f5bb53683cec36b37334b069b35b7c42b76f59a0/components/feature_engagement_tracker/internal/init_aware_model_unittest.cc
[modify] https://crrev.com/f5bb53683cec36b37334b069b35b7c42b76f59a0/components/feature_engagement_tracker/internal/model_impl.cc
[modify] https://crrev.com/f5bb53683cec36b37334b069b35b7c42b76f59a0/components/feature_engagement_tracker/internal/model_impl_unittest.cc

Project Member

Comment 25 by bugdroid1@chromium.org, May 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0ef1f75d2dd2dab69f9510c2a57707d7b879e87c

commit 0ef1f75d2dd2dab69f9510c2a57707d7b879e87c
Author: Tommy Nyquist <nyquist@chromium.org>
Date: Sat May 13 02:31:46 2017

Add support for incrementing events from FeatureEngagementTrackerImpl

The FeatureEngagementTrackerImpl has until now never informed the
underlying Model about any new events.

This CL hooks up this piece of the framework, and adds tests to ensure
that the events wind up all the way to the store.

BUG= 706309 

Change-Id: I37723c3880d7586504ba13eeba0b532b10910aa2
Reviewed-on: https://chromium-review.googlesource.com/504109
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#471541}
[modify] https://crrev.com/0ef1f75d2dd2dab69f9510c2a57707d7b879e87c/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl.cc
[modify] https://crrev.com/0ef1f75d2dd2dab69f9510c2a57707d7b879e87c/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl_unittest.cc

Project Member

Comment 26 by bugdroid1@chromium.org, May 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/44a4480c96442d8d466e958b7260775a4cf30a5b

commit 44a4480c96442d8d466e958b7260775a4cf30a5b
Author: Tommy Nyquist <nyquist@chromium.org>
Date: Sat May 13 05:37:30 2017

Hook up production version of all FeatureEngagementTracker services.

Before this CL, only helper classes that never validated anything and
had invalid configurations were in use to ensure nothing was ever
written or shown.

This CL hooks up all the planned production services, and starts
reading the Chrome Variations configuration.

BUG= 706309 

Change-Id: I6ea9116ac3379d47f5bc4f81a674ce2f50dc1388
Reviewed-on: https://chromium-review.googlesource.com/504155
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#471566}
[modify] https://crrev.com/44a4480c96442d8d466e958b7260775a4cf30a5b/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl.cc

Project Member

Comment 27 by bugdroid1@chromium.org, May 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5130094870ab01d68c4f93b6dd2989e7abfdcd79

commit 5130094870ab01d68c4f93b6dd2989e7abfdcd79
Author: Tommy Nyquist <nyquist@chromium.org>
Date: Sat May 13 05:48:21 2017

Always increment the trigger event when In-Product Help is triggered.

The FeatureEngagementTracker is responsible for informing the Model
that a particular in-product help has been triggered, so that it is
possible to use that as a condition for when to display in-product
help later.

This CL ensures that the Model is updated with this information
whenever the feature meets the particular conditions in the
call to ShouldTriggerHelpUI(...).

BUG= 706309 

Change-Id: I29f11339e49869948db6dbb7b95f356a3c96d19c
Reviewed-on: https://chromium-review.googlesource.com/505097
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#471568}
[modify] https://crrev.com/5130094870ab01d68c4f93b6dd2989e7abfdcd79/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl.cc
[modify] https://crrev.com/5130094870ab01d68c4f93b6dd2989e7abfdcd79/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl_unittest.cc

The following revision should have referred to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/44ba9092273b822880e06cef6c59a24d40c9df05

commit 44ba9092273b822880e06cef6c59a24d40c9df05
Author: Tommy Nyquist <nyquist@chromium.org>
Date: Wed May 17 18:25:25 2017

Fix issue with In-Product Help demo mode.

In demo-mode (enabled through chrome://flags), the configuration for
In-Product Help was just something that was created at runtime, and the
only FeatureConfig field that was set was whether it was valid or not.

However, as part of being able to track that a feature triggering has
happened, it is now required to also have a trigger event name so the
configuration can decide what that field name can be. This was only set
in the Chrome Variations configuration and in tests, and not in the demo
mode.

This CL therefore just appends "_trigger" to the feature name and uses
that as the trigger event name when in demo-mode.

BUG=  723111  , 723009

Change-Id: I62b9507c2bd12c296184bedfb98bbe218923b374
Reviewed-on: https://chromium-review.googlesource.com/507787
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#472505}
[modify] https://crrev.com/44ba9092273b822880e06cef6c59a24d40c9df05/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl.cc

Project Member

Comment 29 by bugdroid1@chromium.org, May 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ddffa7d52640d5a9d1ac7a6d6180dabdab75ff05

commit ddffa7d52640d5a9d1ac7a6d6180dabdab75ff05
Author: Tommy Nyquist <nyquist@chromium.org>
Date: Wed May 24 15:54:45 2017

Change LevelDB Protocol Buffer DB to use base::OnceCallback.

To ensure that other parts of the codebase can use base::OnceCallback
when using the LevelDB Protocol Buffer database, this CL changes
all parts of the leveldb_proto database to now use base::OnceCallback
and base::BindOnce.

BUG= 723007 ,  706309 

Change-Id: I95e262fbcc62cce7189903803797c854ea7f881f
Reviewed-on: https://chromium-review.googlesource.com/513493
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#474306}
[modify] https://crrev.com/ddffa7d52640d5a9d1ac7a6d6180dabdab75ff05/components/leveldb_proto/proto_database.h
[modify] https://crrev.com/ddffa7d52640d5a9d1ac7a6d6180dabdab75ff05/components/leveldb_proto/proto_database_impl.h
[modify] https://crrev.com/ddffa7d52640d5a9d1ac7a6d6180dabdab75ff05/components/leveldb_proto/testing/fake_db.h

Project Member

Comment 30 by bugdroid1@chromium.org, May 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/038589d2a436b7a9f40a5ec4db0fe16b0a3d46a9

commit 038589d2a436b7a9f40a5ec4db0fe16b0a3d46a9
Author: Tommy Nyquist <nyquist@chromium.org>
Date: Thu May 25 19:17:48 2017

Add AvailabilityStore for feature engagement tracker.

The FeatureEngagementTracker is missing functionality to track when a
particular feature has been made available to the end user.

This CL adds the store to track that, using LevelDB as a backend by
using the templated //components/leveldb_proto.

The load of data has a single entry point, and it uses
base::OnceCallback to pass along ownership along the way, including to
the underlying DB.

The store reads from DB and uses a base::Feature filter to figure out
which features should be stored. Each enabled feature should have their
earliest day (in days since UNIX epoch) stored as their avilability
day. Feature that are no longer enabled are deleted. The end output
is provided to the initial callback as a map from feature to the
availability day.

Since the LevelDB for the events has not been used yet, this CL also
changes the suffix for the LevelDB UMA tracking for that DB to ensure
both databases for the feature engagement tracker has the same naming
scheme.

BUG= 723007 ,  706309 

Change-Id: Ic35e83efe75ac20e1c0fb261aee1d7591615d68d
Reviewed-on: https://chromium-review.googlesource.com/515122
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#474745}
[modify] https://crrev.com/038589d2a436b7a9f40a5ec4db0fe16b0a3d46a9/components/feature_engagement_tracker/components_unittests.filter
[modify] https://crrev.com/038589d2a436b7a9f40a5ec4db0fe16b0a3d46a9/components/feature_engagement_tracker/internal/BUILD.gn
[add] https://crrev.com/038589d2a436b7a9f40a5ec4db0fe16b0a3d46a9/components/feature_engagement_tracker/internal/availability_store.cc
[add] https://crrev.com/038589d2a436b7a9f40a5ec4db0fe16b0a3d46a9/components/feature_engagement_tracker/internal/availability_store.h
[add] https://crrev.com/038589d2a436b7a9f40a5ec4db0fe16b0a3d46a9/components/feature_engagement_tracker/internal/availability_store_unittest.cc
[modify] https://crrev.com/038589d2a436b7a9f40a5ec4db0fe16b0a3d46a9/components/feature_engagement_tracker/internal/persistent_store.cc
[modify] https://crrev.com/038589d2a436b7a9f40a5ec4db0fe16b0a3d46a9/components/feature_engagement_tracker/internal/proto/BUILD.gn
[add] https://crrev.com/038589d2a436b7a9f40a5ec4db0fe16b0a3d46a9/components/feature_engagement_tracker/internal/proto/availability.proto
[modify] https://crrev.com/038589d2a436b7a9f40a5ec4db0fe16b0a3d46a9/tools/metrics/histograms/histograms.xml

Project Member

Comment 31 by bugdroid1@chromium.org, May 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3040162868fcdfd2d3bf53471a51e764ecfeb18f

commit 3040162868fcdfd2d3bf53471a51e764ecfeb18f
Author: Tommy Nyquist <nyquist@chromium.org>
Date: Thu May 25 19:32:58 2017

Add AvailabilityModel for feature engagement tracker.

The FeatureEngagementTracker is missing functionality to track when a
particular feature has been made available to the end user.

This CL adds a model to track that, which uses the underlying
AvailabilityStore for a one-time load during initialization.

To help with testing and demo mode, this CL also adds a
NeverAvailabililtyModel, which can be used whenever a real model is
unnecessary.

BUG= 723007 ,  706309 

Change-Id: I0f39acf1979c88532e0321a252d1567f1cb77cc0
Reviewed-on: https://chromium-review.googlesource.com/515142
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#474754}
[modify] https://crrev.com/3040162868fcdfd2d3bf53471a51e764ecfeb18f/components/feature_engagement_tracker/components_unittests.filter
[modify] https://crrev.com/3040162868fcdfd2d3bf53471a51e764ecfeb18f/components/feature_engagement_tracker/internal/BUILD.gn
[add] https://crrev.com/3040162868fcdfd2d3bf53471a51e764ecfeb18f/components/feature_engagement_tracker/internal/availability_model.h
[add] https://crrev.com/3040162868fcdfd2d3bf53471a51e764ecfeb18f/components/feature_engagement_tracker/internal/availability_model_impl.cc
[add] https://crrev.com/3040162868fcdfd2d3bf53471a51e764ecfeb18f/components/feature_engagement_tracker/internal/availability_model_impl.h
[add] https://crrev.com/3040162868fcdfd2d3bf53471a51e764ecfeb18f/components/feature_engagement_tracker/internal/availability_model_impl_unittest.cc
[add] https://crrev.com/3040162868fcdfd2d3bf53471a51e764ecfeb18f/components/feature_engagement_tracker/internal/never_availability_model.cc
[add] https://crrev.com/3040162868fcdfd2d3bf53471a51e764ecfeb18f/components/feature_engagement_tracker/internal/never_availability_model.h
[add] https://crrev.com/3040162868fcdfd2d3bf53471a51e764ecfeb18f/components/feature_engagement_tracker/internal/never_availability_model_unittest.cc

Project Member

Comment 32 by bugdroid1@chromium.org, May 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/168444cf44dfed8b347a5fbaf4420a6733e11e2d

commit 168444cf44dfed8b347a5fbaf4420a6733e11e2d
Author: Tommy Nyquist <nyquist@chromium.org>
Date: Thu May 25 19:44:03 2017

Update ConditionValidator to require AvailabilityModel

The FeatureEngagementTracker is missing functionality to track when a
particular feature has been made available to the end user.

This CL updates the ConditionValidator API to now require an
AvailabilityModel whenever a check for whether a feature meets the
current conditions happens.

This also adds two new fields to the ConditionValidator::Result struct
that now includes relevant fields for availability checks.

Lastly, it updates some helper methods for tests to prepare for
upcoming tests that would have overlapping method names.

BUG= 723007 ,  706309 

Change-Id: Ia86847e7e84b9bded6d939afc46ad596a19c8517
Reviewed-on: https://chromium-review.googlesource.com/515202
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#474760}
[modify] https://crrev.com/168444cf44dfed8b347a5fbaf4420a6733e11e2d/components/feature_engagement_tracker/internal/condition_validator.cc
[modify] https://crrev.com/168444cf44dfed8b347a5fbaf4420a6733e11e2d/components/feature_engagement_tracker/internal/condition_validator.h
[modify] https://crrev.com/168444cf44dfed8b347a5fbaf4420a6733e11e2d/components/feature_engagement_tracker/internal/condition_validator_unittest.cc
[modify] https://crrev.com/168444cf44dfed8b347a5fbaf4420a6733e11e2d/components/feature_engagement_tracker/internal/feature_config_condition_validator.cc
[modify] https://crrev.com/168444cf44dfed8b347a5fbaf4420a6733e11e2d/components/feature_engagement_tracker/internal/feature_config_condition_validator.h
[modify] https://crrev.com/168444cf44dfed8b347a5fbaf4420a6733e11e2d/components/feature_engagement_tracker/internal/feature_config_condition_validator_unittest.cc
[modify] https://crrev.com/168444cf44dfed8b347a5fbaf4420a6733e11e2d/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl.cc
[modify] https://crrev.com/168444cf44dfed8b347a5fbaf4420a6733e11e2d/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl.h
[modify] https://crrev.com/168444cf44dfed8b347a5fbaf4420a6733e11e2d/components/feature_engagement_tracker/internal/feature_engagement_tracker_impl_unittest.cc
[modify] https://crrev.com/168444cf44dfed8b347a5fbaf4420a6733e11e2d/components/feature_engagement_tracker/internal/model.h
[modify] https://crrev.com/168444cf44dfed8b347a5fbaf4420a6733e11e2d/components/feature_engagement_tracker/internal/never_condition_validator.cc
[modify] https://crrev.com/168444cf44dfed8b347a5fbaf4420a6733e11e2d/components/feature_engagement_tracker/internal/never_condition_validator.h
[modify] https://crrev.com/168444cf44dfed8b347a5fbaf4420a6733e11e2d/components/feature_engagement_tracker/internal/never_condition_validator_unittest.cc
[modify] https://crrev.com/168444cf44dfed8b347a5fbaf4420a6733e11e2d/components/feature_engagement_tracker/internal/once_condition_validator.cc
[modify] https://crrev.com/168444cf44dfed8b347a5fbaf4420a6733e11e2d/components/feature_engagement_tracker/internal/once_condition_validator.h
[modify] https://crrev.com/168444cf44dfed8b347a5fbaf4420a6733e11e2d/components/feature_engagement_tracker/internal/once_condition_validator_unittest.cc

Status: Fixed (was: Started)
This was fixed with the commit in #32 ( 168444cf44dfed8b347a5fbaf4420a6733e11e2d )
Components: Internals>FeatureEngagementTracker

Sign in to add a comment