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

Issue 715185 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug


Participants' hotlists:
In-Product-Help


Sign in to add a comment

[IPH] UI - Add triggers for download video

Project Member Reported by dtrainor@chromium.org, Apr 25 2017

Issue description

- See "Feature Configurations: Download video" 
 in https://docs.google.com/document/d/1PhvySbeYOuI6KN3GaJ24ZtZ6vgwJ56Fqmmup0C8NCfc/edit#heading=h.27zujojwkolc
- See "Download Video" (slide 15) in https://docs.google.com/a/google.com/presentation/d/1p9G33j1FNmoeI5jwcqFjBRC1Vm1TbpsJZktwJCXgim0/edit?usp=sharing

This involves:
(1) Adding support for piping the video download button coordinates through to the browser process.
(2) Showing the popup bubble at those coordinates when appropriate (see design doc and spec for details).
 
Cc: mlamouri@chromium.org
Labels: -M-60 M-61
The current CL in review ( https://codereview.chromium.org/2943983003/ ) does the following things:
- Checks whether we should show IPH through a call to Tracker::ShouldShowHelpUI().
- Calls from Blink renderer process through the browser process all the way up to Tab.java with the rects of where the button should be (showMediaDownloadInProductHelp(int x, int y, int width, int height)), and shows the TextBubble.
- Calls Tracker::NotifyEvent("download_button_shown") when the IPH is displayed.
- Calls Dismissed() whenever the popup is dismissed.


However, even after that CL lands, there are a couple of things we need to implement:

1) Add pulsating dot in UI.
We already know where the rectangle is for the button, so hopefully this would only have to be implemented on the Java UI side.

2) Tell the tracker whenever a video is in fact downloaded.
This basically means calling Tracker::NotifyEvent("...event_used...") in the browser process. The question here is mostly how to plumb the fact that the user initiated a video download to the browser process.
Hopefully, there's already a trigger we can re-use for this "user-initiated download of video". If not, we might have to add some new IPC. If we have to add new IPC, hopefully we can re-use the mojo-service MediaDownloadInProductHelp that is already being added in the CL mentioned above.
Regarding #2, we do not notify anyone about the download starting or finishing at the moment. It might be tricky to figure out when the download is finished as the download from the controls is no different from a download initiated by a web page today. It is actually implemented as <a download> pointing to the source file.
Cc: khushals...@chromium.org
Owner: nyquist@chromium.org
https://codereview.chromium.org/2943983003/ should land soon now. Assigning the bug to nyquist@ for the follow-up work.
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 22 2017

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

commit c5447db2943848192c0452c7b8e766a943335f07
Author: khushalsagar <khushalsagar@chromium.org>
Date: Tue Aug 22 17:53:01 2017

chrome/blink: Add functionality for in-product help for media elements.

IPH (In-product help) UI is added to highlight and assist in the
discovery of features in the browser. This change adds this
functionality for the download feature on media elements in blink. This
involves the following additions:

1) A public WebMediaIPH interface in blink that can be used to ask the
embedder to display IPH widgets for an element.

2) A mojo MediaIPHService to bridge these requests to the browser where
the UI can be shown using android pop-ups overlayed on the content.

3) An addition to the features supported by the FeatureEngagementTracker
for media download IPH.

The feature is currently gated behind a finch flag which is disabled by
default.

BUG=715185

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

[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/chrome/android/java/strings/android_chrome_strings.grd
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/chrome/browser/android/DEPS
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/chrome/browser/android/tab_android.cc
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/chrome/browser/android/tab_android.h
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/chrome/browser/chrome_content_browser_manifest_overlay.json
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/components/feature_engagement/public/android/java/src/org/chromium/components/feature_engagement/EventConstants.java
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/components/feature_engagement/public/android/java/src/org/chromium/components/feature_engagement/FeatureConstants.java
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/components/feature_engagement/public/feature_constants.cc
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/components/feature_engagement/public/feature_constants.h
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/components/feature_engagement/public/feature_list.cc
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/components/feature_engagement/public/feature_list.h
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/content/public/common/common_param_traits_macros.h
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/content/public/common/web_preferences.cc
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/content/public/common/web_preferences.h
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/content/renderer/render_view_impl.cc
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/third_party/WebKit/Source/core/exported/WebSettingsImpl.cpp
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/third_party/WebKit/Source/core/exported/WebSettingsImpl.h
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/third_party/WebKit/Source/core/frame/Settings.json5
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/third_party/WebKit/Source/modules/media_controls/BUILD.gn
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/third_party/WebKit/Source/modules/media_controls/DEPS
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/third_party/WebKit/Source/modules/media_controls/MediaControlsImpl.cpp
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/third_party/WebKit/Source/modules/media_controls/MediaControlsImpl.h
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/third_party/WebKit/Source/modules/media_controls/MediaControlsImplTest.cpp
[add] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/third_party/WebKit/Source/modules/media_controls/MediaDownloadInProductHelpManager.cpp
[add] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/third_party/WebKit/Source/modules/media_controls/MediaDownloadInProductHelpManager.h
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/third_party/WebKit/Source/modules/media_controls/elements/MediaControlDownloadButtonElement.cpp
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/third_party/WebKit/Source/modules/media_controls/elements/MediaControlDownloadButtonElement.h
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/third_party/WebKit/Source/modules/media_controls/elements/MediaControlInputElement.h
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/third_party/WebKit/Source/platform/mojo/Geometry.typemap
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/third_party/WebKit/Source/platform/mojo/GeometryStructTraits.cpp
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/third_party/WebKit/Source/platform/mojo/GeometryStructTraits.h
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/third_party/WebKit/Source/platform/mojo/GeometryStructTraitsTest.cpp
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/third_party/WebKit/public/BUILD.gn
[add] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/third_party/WebKit/public/platform/media_download_in_product_help.mojom
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/third_party/WebKit/public/web/WebSettings.h
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/tools/metrics/actions/actions.xml
[modify] https://crrev.com/c5447db2943848192c0452c7b8e766a943335f07/tools/metrics/histograms/histograms.xml

Labels: -M-61 M-64
Owner: shaktisahu@chromium.org
Assigning to shaktisahu@ for implemmeting part 2 of comment #3.

The thought would be to add a new method to MediaDownloadInProductHelp that would be invoked from the renderer every time the user clicks the download-video-button, regardless of whether IPH was displayed or not.
Labels: -M-64 M-66
Also, we would need to write a test plan for this. Probably we can use the download test server.

For manual testing, we probably want to use this guide to create the command line arguments:
https://chromium.googlesource.com/chromium/src/+/master/components/feature_engagement/README.md#Using-Chrome-Variations-at-runtime

More from an offline discussion about sending video-button-click event to server :
The current issue is that we cannot use the existing mojo pipe to send this event, because the pipe is created only when we have to show the IPH and reset after the IPH is dismissed.

There are a few options we can take :

1- Send the event along with the download source attribution data. However, currently we don't have a way to differentiate media downloads from <a> downloads and this will be worked on soon as part of collecting download source attribution data. 
2- Create a generic mojo pipe for FeatureEngagement which will always be kept alive. This can be reused in future when there are more similar IPH needs to communicate between renderer and browser.
3- Third alternative is to keep the existing pipe MediaDownloadInProductHelp always alive and use this for sending the button click events.

Sign in to add a comment