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

Issue 876943 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 12
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

Re-enable testPeekWithPageScrollPercentage for ContextualSuggestionTest

Project Member Reported by ericrk@chromium.org, Aug 23

Issue description

The SurfaceSynchronization/VizDisplayCompositor features on Android mean that the browser is no longer notified of every frame update the renderer makes. Instead we only notify the browser if we think that the frame update will impact the browser's rendering.

This means that we no longer consistently deliver page scroll percentage updates to the browser, breaking the  ContextualSuggestionsMediator's page scroll percentage handling and ContextualSuggestionTest's testPeekWithPageScrollPercentage case.

Page-scroll percentage based contextual suggestions are currently disabled, but we may want to re-enable in the future.

While we could deliver every scroll update to the browser, this requires an extra IPC per frame and seems pretty inefficient, especially if not being used. Instead, it feels like we might be able to take one of the following approaches:

 - If we don't plan to use this info in the near future and are just interested in
   page scroll percentage for UKM, we could probably log the same UKM from the
   renderer process.

 - If we do want to use this info in the future, we could:
   - Send the renderer a target page scroll percentage we're interested in and
     have the renderer only notify when we cross that threshold.
   - Have the renderer update the browser at a lower granularity (every 10%
     change in page scroll percentage)
   - Only send page scroll percentages in cases where we have a contextual
     suggestion and need the data (still inefficient, but a bit more scoped,
     may be OK if contextual suggestions are somewhat rare).

 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 23

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

commit 7d458b26825f8910e306cae1dc6e5c4f83da8839
Author: Eric Karl <ericrk@chromium.org>
Date: Thu Aug 23 17:25:51 2018

Disable testPeekWithPageScrollPercentage from ContextualSuggestionTest

The SurfaceSynchronization feature breaks this test, but after
discussion it turns out this test is verifying behavior that is no
longer used (or planned to be used in the near future).

Disabling the test for now and filing a bug so we can discuss ways
to work around this if we want to re-enable in the future.

R=twellington@chromium.org

Bug:  876943 ,  871772 
Change-Id: I4949b17b20e6376b9b659d9af6cb5d727f348f51
Reviewed-on: https://chromium-review.googlesource.com/1186111
Commit-Queue: Eric Karl <ericrk@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585531}
[modify] https://crrev.com/7d458b26825f8910e306cae1dc6e5c4f83da8839/chrome/android/javatests/src/org/chromium/chrome/browser/contextual_suggestions/ContextualSuggestionsTest.java

Project Member

Comment 2 by bugdroid1@chromium.org, Sep 22

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

commit ba455a15dbfb3cb4491f5b6b120b2afc72b46110
Author: Theresa <twellington@chromium.org>
Date: Sat Sep 22 00:09:42 2018

[EoC] Remove support for reverse scroll triggering

We're no longer experimenting with reverse scroll triggering, so remove
support for:
 - reverse scroll triggering
 - slim peek
 - peek conditions (apart from delay, which we may use later)

BUG= 859292 , 876943 ,885331

Change-Id: Ibe962d6ece4340eb7b57784bbe078cc0847de728
Reviewed-on: https://chromium-review.googlesource.com/1236453
Reviewed-by: Becky Zhou <huayinz@chromium.org>
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Commit-Queue: Theresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593386}
[modify] https://crrev.com/ba455a15dbfb3cb4491f5b6b120b2afc72b46110/chrome/android/java/res/layout/contextual_suggestions_toolbar.xml
[modify] https://crrev.com/ba455a15dbfb3cb4491f5b6b120b2afc72b46110/chrome/android/java/res/values/dimens.xml
[modify] https://crrev.com/ba455a15dbfb3cb4491f5b6b120b2afc72b46110/chrome/android/java/src/org/chromium/chrome/browser/ChromeFeatureList.java
[modify] https://crrev.com/ba455a15dbfb3cb4491f5b6b120b2afc72b46110/chrome/android/java/src/org/chromium/chrome/browser/contextual_suggestions/ContentCoordinator.java
[modify] https://crrev.com/ba455a15dbfb3cb4491f5b6b120b2afc72b46110/chrome/android/java/src/org/chromium/chrome/browser/contextual_suggestions/ContextualSuggestionsBottomSheetContent.java
[modify] https://crrev.com/ba455a15dbfb3cb4491f5b6b120b2afc72b46110/chrome/android/java/src/org/chromium/chrome/browser/contextual_suggestions/ContextualSuggestionsCoordinator.java
[modify] https://crrev.com/ba455a15dbfb3cb4491f5b6b120b2afc72b46110/chrome/android/java/src/org/chromium/chrome/browser/contextual_suggestions/ContextualSuggestionsMediator.java
[modify] https://crrev.com/ba455a15dbfb3cb4491f5b6b120b2afc72b46110/chrome/android/java/src/org/chromium/chrome/browser/contextual_suggestions/ContextualSuggestionsModel.java
[modify] https://crrev.com/ba455a15dbfb3cb4491f5b6b120b2afc72b46110/chrome/android/java/src/org/chromium/chrome/browser/contextual_suggestions/FetchHelper.java
[modify] https://crrev.com/ba455a15dbfb3cb4491f5b6b120b2afc72b46110/chrome/android/java/src/org/chromium/chrome/browser/contextual_suggestions/ToolbarView.java
[modify] https://crrev.com/ba455a15dbfb3cb4491f5b6b120b2afc72b46110/chrome/android/java/src/org/chromium/chrome/browser/contextual_suggestions/ToolbarViewBinder.java
[modify] https://crrev.com/ba455a15dbfb3cb4491f5b6b120b2afc72b46110/chrome/android/java/src/org/chromium/chrome/browser/util/FeatureUtilities.java
[modify] https://crrev.com/ba455a15dbfb3cb4491f5b6b120b2afc72b46110/chrome/android/javatests/src/org/chromium/chrome/browser/contextual_suggestions/ContextualSuggestionsTest.java
[modify] https://crrev.com/ba455a15dbfb3cb4491f5b6b120b2afc72b46110/chrome/android/javatests/src/org/chromium/chrome/browser/contextual_suggestions/EnabledStateMonitorTest.java
[modify] https://crrev.com/ba455a15dbfb3cb4491f5b6b120b2afc72b46110/chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManagerTest.java
[modify] https://crrev.com/ba455a15dbfb3cb4491f5b6b120b2afc72b46110/chrome/android/javatests/src/org/chromium/chrome/browser/fullscreen/FullscreenManagerTest.java
[modify] https://crrev.com/ba455a15dbfb3cb4491f5b6b120b2afc72b46110/chrome/android/javatests/src/org/chromium/chrome/browser/infobar/InfoBarContainerTest.java
[modify] https://crrev.com/ba455a15dbfb3cb4491f5b6b120b2afc72b46110/chrome/android/junit/src/org/chromium/chrome/browser/contextual_suggestions/FetchHelperTest.java
[modify] https://crrev.com/ba455a15dbfb3cb4491f5b6b120b2afc72b46110/chrome/browser/about_flags.cc
[modify] https://crrev.com/ba455a15dbfb3cb4491f5b6b120b2afc72b46110/chrome/browser/android/chrome_feature_list.cc
[modify] https://crrev.com/ba455a15dbfb3cb4491f5b6b120b2afc72b46110/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/ba455a15dbfb3cb4491f5b6b120b2afc72b46110/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/ba455a15dbfb3cb4491f5b6b120b2afc72b46110/chrome/browser/ntp_snippets/contextual_content_suggestions_service_factory.cc
[modify] https://crrev.com/ba455a15dbfb3cb4491f5b6b120b2afc72b46110/chrome/browser/ui/webui/eoc_internals/eoc_internals_page_handler.cc
[modify] https://crrev.com/ba455a15dbfb3cb4491f5b6b120b2afc72b46110/components/ntp_snippets/contextual/contextual_suggestions_features.cc
[modify] https://crrev.com/ba455a15dbfb3cb4491f5b6b120b2afc72b46110/components/ntp_snippets/contextual/contextual_suggestions_features.h
[modify] https://crrev.com/ba455a15dbfb3cb4491f5b6b120b2afc72b46110/components/ntp_snippets/contextual/contextual_suggestions_fetch.cc
[modify] https://crrev.com/ba455a15dbfb3cb4491f5b6b120b2afc72b46110/components/ntp_snippets/contextual/contextual_suggestions_fetch_unittest.cc
[modify] https://crrev.com/ba455a15dbfb3cb4491f5b6b120b2afc72b46110/testing/variations/fieldtrial_testing_config.json

Components: UI>Browser>ContentSuggestions>Explore
Labels: Hotlist-EoC-MVP
Status: Fixed (was: Available)
Fixed by removing the test.

Sign in to add a comment