Video tutorials : Several fixes

This CL
1 - Changes Tutorial not to be a subclass of ImageTile.
2 - Handled additional feature type enum types.
3 - Added some unittests
4 - Fixed a CSS bug

Change-Id: Iceca0057f7e0d9b549af5481a12c7f1565e21441
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2464855Reviewed-by: Calder Kitagawa <ckitagawa@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Shakti Sahu <shaktisahu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816693}

chrome/android/java/src/org/chromium/chrome/browser/app/video_tutorials/NewTabPageVideoIPHManager.java

@@ -60,10 +60,8 @@ public class NewTabPageVideoIPHManager {
 
         mContext = viewStub.getContext();
         mTracker = TrackerFactory.getTrackerForProfile(profile);
-        Callback<Tutorial> clickListener = this::onClickIPH;
-        Callback<Tutorial> dismissListener = this::onDismissIPH;
         mVideoIPHCoordinator = createVideoIPHCoordinator(
-                viewStub, createImageFetcher(profile), clickListener, dismissListener);
+                viewStub, createImageFetcher(profile), this::onClickIPH, this::onDismissIPH);
         mVideoTutorialService = VideoTutorialServiceFactory.getForProfile(profile);
         mVideoTutorialService.getTutorials(this::onFetchTutorials);
     }

chrome/android/java/src/org/chromium/chrome/browser/app/video_tutorials/VideoPlayerActivity.java

@@ -16,6 +16,7 @@ import org.chromium.chrome.browser.ChromeTabbedActivity;
 import org.chromium.chrome.browser.SynchronousInitializationActivity;
 import org.chromium.chrome.browser.WebContentsFactory;
 import org.chromium.chrome.browser.profiles.Profile;
+import org.chromium.chrome.browser.video_tutorials.FeatureType;
 import org.chromium.chrome.browser.video_tutorials.Tutorial;
 import org.chromium.chrome.browser.video_tutorials.VideoTutorialService;
 import org.chromium.chrome.browser.video_tutorials.VideoTutorialServiceFactory;
@@ -50,7 +51,8 @@ public class VideoPlayerActivity extends SynchronousInitializationActivity {
                 this, videoTutorialService, this::createWebContents, this::tryNow, this::finish);
         setContentView(mCoordinator.getView());
 
-        int featureType = IntentUtils.safeGetIntExtra(getIntent(), EXTRA_VIDEO_TUTORIAL, 0);
+        int featureType =
+                IntentUtils.safeGetIntExtra(getIntent(), EXTRA_VIDEO_TUTORIAL, FeatureType.INVALID);
         videoTutorialService.getTutorial(featureType, mCoordinator::playVideoTutorial);
     }
 

chrome/android/junit/src/org/chromium/chrome/browser/app/video_tutorials/NewTabPageVideoIPHManagerTest.java

@@ -30,8 +30,11 @@ import org.chromium.chrome.browser.video_tutorials.FeatureType;
 import org.chromium.chrome.browser.video_tutorials.Tutorial;
 import org.chromium.chrome.browser.video_tutorials.VideoTutorialServiceFactory;
 import org.chromium.chrome.browser.video_tutorials.iph.VideoIPHCoordinator;
+import org.chromium.chrome.browser.video_tutorials.iph.VideoTutorialIPHUtils;
 import org.chromium.chrome.browser.video_tutorials.test.TestVideoTutorialService;
 import org.chromium.chrome.test.util.browser.Features;
+import org.chromium.components.feature_engagement.EventConstants;
+import org.chromium.components.feature_engagement.FeatureConstants;
 import org.chromium.components.feature_engagement.Tracker;
 
 import java.util.HashMap;
@@ -116,15 +119,17 @@ public class NewTabPageVideoIPHManagerTest {
     @Test
     public void testShowFirstEnabledIPH() {
         // We have already watched the first tutorial, the second one should show up as IPH card.
+        Tutorial testTutorial = mTestVideoTutorialService.getTestTutorials().get(1);
         Mockito.when(mTracker.shouldTriggerHelpUI(Mockito.anyString())).thenReturn(false, true);
         mVideoIPHManager = new TestNewTabPageVideoIPHManager(mViewStub, mProfile);
-        Mockito.verify(mVideoIPHCoordinator)
-                .showVideoIPH(mTestVideoTutorialService.getTestTutorials().get(1));
+        Mockito.verify(mVideoIPHCoordinator).showVideoIPH(testTutorial);
 
-        // Click on the IPH. video player should be launched.
-        mIPHClickListener.onResult(mTestVideoTutorialService.getTestTutorials().get(1));
+        // Click on the IPH. The video player should be launched.
+        mIPHClickListener.onResult(testTutorial);
         assertThat(mVideoIPHManager.videoPlayerLaunched, equalTo(true));
         assertThat(mVideoIPHManager.videoListLaunched, equalTo(false));
+        Mockito.verify(mTracker, Mockito.times(1))
+                .notifyEvent(VideoTutorialIPHUtils.getClickEvent(testTutorial.featureType));
     }
 
     @Test
@@ -143,4 +148,63 @@ public class NewTabPageVideoIPHManagerTest {
         assertThat(mVideoIPHManager.videoPlayerLaunched, equalTo(false));
         assertThat(mVideoIPHManager.videoListLaunched, equalTo(true));
     }
+
+    @Test
+    public void testDismissIPH() {
+        // Show a tutorial IPH.
+        Tutorial testTutorial = mTestVideoTutorialService.getTestTutorials().get(0);
+        Mockito.when(mTracker.shouldTriggerHelpUI(Mockito.anyString())).thenReturn(true);
+        mVideoIPHManager = new TestNewTabPageVideoIPHManager(mViewStub, mProfile);
+        Mockito.verify(mVideoIPHCoordinator).showVideoIPH(testTutorial);
+
+        // Dismiss the IPH. The tracker should record this event.
+        mIPHDismissListener.onResult(testTutorial);
+        Mockito.verify(mTracker, Mockito.times(1))
+                .notifyEvent(VideoTutorialIPHUtils.getDismissEvent(testTutorial.featureType));
+        assertThat(mVideoIPHManager.videoPlayerLaunched, equalTo(false));
+        assertThat(mVideoIPHManager.videoListLaunched, equalTo(false));
+    }
+
+    @Test
+    public void testClickEventForFeatureTypes() {
+        assertThat(VideoTutorialIPHUtils.getClickEvent(FeatureType.SUMMARY),
+                equalTo(EventConstants.VIDEO_TUTORIAL_CLICKED_SUMMARY));
+        assertThat(VideoTutorialIPHUtils.getClickEvent(FeatureType.CHROME_INTRO),
+                equalTo(EventConstants.VIDEO_TUTORIAL_CLICKED_CHROME_INTRO));
+        assertThat(VideoTutorialIPHUtils.getClickEvent(FeatureType.DOWNLOAD),
+                equalTo(EventConstants.VIDEO_TUTORIAL_CLICKED_DOWNLOAD));
+        assertThat(VideoTutorialIPHUtils.getClickEvent(FeatureType.SEARCH),
+                equalTo(EventConstants.VIDEO_TUTORIAL_CLICKED_SEARCH));
+        assertThat(VideoTutorialIPHUtils.getClickEvent(FeatureType.VOICE_SEARCH),
+                equalTo(EventConstants.VIDEO_TUTORIAL_CLICKED_VOICE_SEARCH));
+    }
+
+    @Test
+    public void testDismissEventForFeatureTypes() {
+        assertThat(VideoTutorialIPHUtils.getDismissEvent(FeatureType.SUMMARY),
+                equalTo(EventConstants.VIDEO_TUTORIAL_DISMISSED_SUMMARY));
+        assertThat(VideoTutorialIPHUtils.getDismissEvent(FeatureType.CHROME_INTRO),
+                equalTo(EventConstants.VIDEO_TUTORIAL_DISMISSED_CHROME_INTRO));
+        assertThat(VideoTutorialIPHUtils.getDismissEvent(FeatureType.DOWNLOAD),
+                equalTo(EventConstants.VIDEO_TUTORIAL_DISMISSED_DOWNLOAD));
+        assertThat(VideoTutorialIPHUtils.getDismissEvent(FeatureType.SEARCH),
+                equalTo(EventConstants.VIDEO_TUTORIAL_DISMISSED_SEARCH));
+        assertThat(VideoTutorialIPHUtils.getDismissEvent(FeatureType.VOICE_SEARCH),
+                equalTo(EventConstants.VIDEO_TUTORIAL_DISMISSED_VOICE_SEARCH));
+    }
+
+    @Test
+    public void testNTPFeatureNameForFeatureTypes() {
+        assertThat(VideoTutorialIPHUtils.getFeatureNameForNTP(FeatureType.SUMMARY),
+                equalTo(FeatureConstants.VIDEO_TUTORIAL_NTP_SUMMARY_FEATURE));
+        assertThat(VideoTutorialIPHUtils.getFeatureNameForNTP(FeatureType.CHROME_INTRO),
+                equalTo(FeatureConstants.VIDEO_TUTORIAL_NTP_CHROME_INTRO_FEATURE));
+        assertThat(VideoTutorialIPHUtils.getFeatureNameForNTP(FeatureType.DOWNLOAD),
+                equalTo(FeatureConstants.VIDEO_TUTORIAL_NTP_DOWNLOAD_FEATURE));
+        assertThat(VideoTutorialIPHUtils.getFeatureNameForNTP(FeatureType.SEARCH),
+                equalTo(FeatureConstants.VIDEO_TUTORIAL_NTP_SEARCH_FEATURE));
+        assertThat(VideoTutorialIPHUtils.getFeatureNameForNTP(FeatureType.VOICE_SEARCH),
+                equalTo(FeatureConstants.VIDEO_TUTORIAL_NTP_VOICE_SEARCH_FEATURE));
+        assertThat(VideoTutorialIPHUtils.getFeatureNameForNTP(FeatureType.INVALID), equalTo(null));
+    }
 }

chrome/android/junit/src/org/chromium/chrome/browser/app/video_tutorials/OWNERS

+file://chrome/browser/video_tutorials/OWNERS
+
+# TEAM: chrome-upboarding@chromium.org
+# COMPONENT: Upboarding>VideoTutorials
+# OS: Android

chrome/browser/resources/video_tutorials/video_player.js

@@ -15,7 +15,7 @@ function onDocumentLoaded() {
 
 function onVideoEnded() {
   // Resize the poster.
-  video.style.classList.add('video-ended');
+  video.classList.add('video-ended');
   video.controls = false;
 }
 

chrome/browser/video_tutorials/android/java/src/org/chromium/chrome/browser/video_tutorials/Tutorial.java

@@ -4,13 +4,13 @@
 
 package org.chromium.chrome.browser.video_tutorials;
 
-import org.chromium.components.browser_ui.widget.image_tiles.ImageTile;
-
 /**
  * Class encapsulating data needed to show a video tutorial on the UI.
  */
-public class Tutorial extends ImageTile {
+public class Tutorial {
     public final @FeatureType int featureType;
+    public final String title;
+    public final String accessibilityText;
     public final String videoUrl;
     public final String posterUrl;
     public final String captionUrl;
@@ -20,16 +20,13 @@ public class Tutorial extends ImageTile {
     /** Constructor */
     public Tutorial(@FeatureType int featureType, String title, String videoUrl, String posterUrl,
             String captionUrl, String shareUrl, int videoLength) {
-        super(createIdFromFeatureType(featureType), title, title);
         this.featureType = featureType;
+        this.title = title;
+        this.accessibilityText = title;
         this.videoUrl = videoUrl;
         this.posterUrl = posterUrl;
         this.captionUrl = captionUrl;
         this.shareUrl = shareUrl;
         this.videoLength = videoLength;
     }
-
-    private static String createIdFromFeatureType(int featureType) {
-        return String.valueOf(featureType);
-    }
 }

chrome/browser/video_tutorials/internal/android/java/src/org/chromium/chrome/browser/video_tutorials/iph/VideoIPHCoordinatorImpl.java

@@ -50,7 +50,7 @@ public class VideoIPHCoordinatorImpl implements VideoIPHCoordinator {
     @Override
     public void showVideoIPH(Tutorial tutorial) {
         mModel.set(VideoIPHProperties.VISIBILITY, true);
-        mModel.set(VideoIPHProperties.DISPLAY_TITLE, tutorial.displayTitle);
+        mModel.set(VideoIPHProperties.DISPLAY_TITLE, tutorial.title);
         mModel.set(VideoIPHProperties.VIDEO_LENGTH,
                 VideoTutorialUtils.getVideoLengthString(tutorial.videoLength));
         mModel.set(VideoIPHProperties.CLICK_LISTENER, () -> mOnClickListener.onResult(tutorial));

chrome/browser/video_tutorials/internal/android/java/src/org/chromium/chrome/browser/video_tutorials/iph/VideoIPHTest.java

@@ -78,9 +78,9 @@ public class VideoIPHTest {
     public void testShowIPH() {
         final Tutorial tutorial = createDummyTutorial();
         TestThreadUtils.runOnUiThreadBlocking(() -> { mCoordinator.showVideoIPH(tutorial); });
-        onView(withText(tutorial.displayTitle)).check(matches(isDisplayed()));
+        onView(withText(tutorial.title)).check(matches(isDisplayed()));
         onView(withText("5:35")).check(matches(isDisplayed()));
-        onView(withText(tutorial.displayTitle)).perform(ViewActions.click());
+        onView(withText(tutorial.title)).perform(ViewActions.click());
         Mockito.verify(mOnClickListener).onResult(Mockito.any());
         onView(withId(R.id.close_button)).perform(ViewActions.click());
         Mockito.verify(mOnDismissListener).onResult(Mockito.any());

chrome/browser/video_tutorials/internal/android/java/src/org/chromium/chrome/browser/video_tutorials/list/TutorialListCoordinatorTest.java

@@ -81,10 +81,10 @@ public class TutorialListCoordinatorTest {
     @SmallTest
     public void testShowList() {
         Tutorial tutorial = mTestVideoTutorialService.getTestTutorials().get(0);
-        onView(withText(tutorial.displayTitle)).check(matches(isDisplayed()));
+        onView(withText(tutorial.title)).check(matches(isDisplayed()));
         onView(withText(VideoTutorialUtils.getVideoLengthString(tutorial.videoLength)))
                 .check(matches(isDisplayed()));
-        onView(withText(tutorial.displayTitle)).perform(ViewActions.click());
+        onView(withText(tutorial.title)).perform(ViewActions.click());
         Mockito.verify(mClickCallback).onResult(tutorial);
     }
 }

chrome/browser/video_tutorials/internal/android/java/src/org/chromium/chrome/browser/video_tutorials/list/TutorialListMediator.java

@@ -57,7 +57,7 @@ public class TutorialListMediator {
     private PropertyModel buildModelFromTutorial(Tutorial tutorial) {
         PropertyModel.Builder builder =
                 new PropertyModel.Builder(TutorialCardProperties.ALL_KEYS)
-                        .with(TutorialCardProperties.TITLE, tutorial.displayTitle)
+                        .with(TutorialCardProperties.TITLE, tutorial.title)
                         .with(TutorialCardProperties.VIDEO_LENGTH,
                                 VideoTutorialUtils.getVideoLengthString(tutorial.videoLength))
                         .with(TutorialCardProperties.CLICK_CALLBACK,

chrome/browser/video_tutorials/internal/android/java/src/org/chromium/chrome/browser/video_tutorials/metrics/VideoTutorialMetrics.java

@@ -65,10 +65,16 @@ public class VideoTutorialMetrics {
 
     private static String histogramSuffixFromFeatureType(@FeatureType int feature) {
         switch (feature) {
+            case FeatureType.CHROME_INTRO:
+                return "ChromeIntro";
             case FeatureType.DOWNLOAD:
                 return "Download";
             case FeatureType.SEARCH:
                 return "Search";
+            case FeatureType.VOICE_SEARCH:
+                return "VoiceSearch";
+            case FeatureType.SUMMARY:
+                return "Summary";
             default:
                 return "Unknown";
         }

tools/metrics/histograms/histograms_xml/video_tutorials/histograms.xml

@@ -22,9 +22,12 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
 <histograms>
 
 <variants name="VideoTutorialFeatureTopic">
+  <variant name="ChromeIntro"/>
   <variant name="Download"/>
   <variant name="Search"/>
+  <variant name="Summary"/>
   <variant name="Unknown"/>
+  <variant name="VoiceSearch"/>
 </variants>
 
 <histogram name="VideoTutorials.Player.LoadTimeLatency" units="ms"