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

Issue 776168 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature



Sign in to add a comment

Add support for giving out handles to ensure no IPH will be shown

Project Member Reported by nyquist@chromium.org, Oct 18 2017

Issue description

What steps will reproduce the problem?
(1) Try to ensure that IPH can not be shown while something important is happening.

What is the expected result?
There is an API call I can do to lock and unlock whether IPH can be shown.

What happens instead?
There are no direct API calls that can be made to accomplish this.

Notes:
We should provide an API to take a lock of all IPH. When asked, the Tracker will give out a handle, which ensures that nothing can be shown while it's active. When it's closed, IPH can again be shown. This should be an additional failure-case for the statistics tracking. In the Java-version of the API, we probably want to use a Runnable or Callback<T> or something like that to be invoked when the handle is finished (or a named API).

We should probably also change the API to instead of returning a boolean for ShouldTriggerHelpUI(...), where we also should be returning a handle, which when closed, would conceptually invoke the Dismiss(...) call for the given feature. We should be able to then remove the Dismiss(...) method all together. The handle itself should then of course know the name of the feature it was invoked for (if it's still necessary).

We might have to roll these two parts out over time, depending on how easy it is for users of IPH to hold on to a handle.


This is a different way for fixing issue 775097, but also improves the API.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 17 2017

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

commit c05089f66f28c6e7e07dd3000395ae68b513dd18
Author: Tommy Nyquist <nyquist@chromium.org>
Date: Fri Nov 17 22:31:48 2017

Add support for locking display of in-product help.

Up until now, there has been no way to ensure that no in-product help is
displayed during important user journeys. It can conceptually pop up at
any time.

This CL therefore adds a way to acquire a display lock that locks all
display of in-product help. It does not remove currently displaying
in-product help, but it does ensure nothing new is displayed.

When the protected user journey has finished, the lock must be
released to ensure that other in-product help can be displayed.

On most platforms, the lock is released simply by deleting the
DisplayLockHandle, which is given out as an std::unique_ptr. On Android,
there are two APIs, one in C++ (shared with the other platforms), and
one in Java. Since Java does not support destructors, a particular
release() method must be invoked on the DisplayLockHandle instead to
ensure that the C++ object is deleted.

These locks are tracked separately in UMA for why in-product help has
not been displayed: TriggerHelpUIResult.FAILURE_DISPLAY_LOCK.

BUG= 776168 

Change-Id: I388fe8d626a59c5128481d5758b3bb66821e0aa3
Reviewed-on: https://chromium-review.googlesource.com/758631
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517591}
[modify] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/chrome/android/javatests/src/org/chromium/chrome/browser/appmenu/ChromeHomeAppMenuTest.java
[modify] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/components_unittests.filter
[modify] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/internal/BUILD.gn
[modify] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/internal/android/java/src/org/chromium/components/feature_engagement/internal/TrackerImpl.java
[modify] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/internal/android/tracker_impl_android.cc
[modify] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/internal/android/tracker_impl_android.h
[modify] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/internal/condition_validator.cc
[modify] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/internal/condition_validator.h
[modify] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/internal/condition_validator_unittest.cc
[add] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/internal/display_lock_controller.h
[add] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/internal/display_lock_controller_impl.cc
[add] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/internal/display_lock_controller_impl.h
[add] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/internal/display_lock_controller_impl_unittest.cc
[modify] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/internal/feature_config_condition_validator.cc
[modify] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/internal/feature_config_condition_validator.h
[modify] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/internal/feature_config_condition_validator_unittest.cc
[modify] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/internal/never_condition_validator.cc
[modify] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/internal/never_condition_validator.h
[modify] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/internal/never_condition_validator_unittest.cc
[add] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/internal/noop_display_lock_controller.cc
[add] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/internal/noop_display_lock_controller.h
[add] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/internal/noop_display_lock_controller_unittest.cc
[modify] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/internal/once_condition_validator.cc
[modify] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/internal/once_condition_validator.h
[modify] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/internal/once_condition_validator_unittest.cc
[modify] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/internal/stats.cc
[modify] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/internal/stats.h
[modify] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/internal/tracker_impl.cc
[modify] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/internal/tracker_impl.h
[modify] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/internal/tracker_impl_unittest.cc
[modify] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/public/BUILD.gn
[modify] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/public/android/java/src/org/chromium/components/feature_engagement/Tracker.java
[add] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/public/tracker.cc
[modify] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/public/tracker.h
[modify] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/test/mock_tracker.cc
[modify] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/test/mock_tracker.h
[modify] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/components/feature_engagement/test/test_tracker.cc
[modify] https://crrev.com/c05089f66f28c6e7e07dd3000395ae68b513dd18/tools/metrics/histograms/enums.xml

Cc: -twelling...@chromium.org nyquist@chromium.org
Owner: twelling...@chromium.org
twellington: Is this something you're planning on using? If so, is there a bug for the follow-up work? Or would you want to use this one?
Status: Fixed (was: Assigned)
I filed a new crbug to use this for promo dialogs (issue 788873).

Sign in to add a comment