Revert "[ThemeSync] Add pre-defined theme collection options." [chromium/src : main]

0 views

Skip to first unread message

luci-bisection@appspot.gserviceaccount.com (Gerrit)

unread,

2:12 AM (2 hours ago) 2:12 AM

to Chromium LUCI CQ, Xinyi Ji, Xi Han, android-bu...@system.gserviceaccount.com, chromium...@chromium.org

Attention needed from Xi Han and Xinyi Ji

luci-bi...@appspot.gserviceaccount.com has uploaded the change for review

luci-bi...@appspot.gserviceaccount.com would like Chromium LUCI CQ, Xinyi Ji and Xi Han to review this change.

Commit message

Revert "[ThemeSync] Add pre-defined theme collection options."

This reverts commit 24530ebe568ffde8e825df39a620af9feac5f7e5.

Reason for revert:
LUCI Bisection has identified this change as the cause of a test failure. See the analysis: https://ci.chromium.org/ui/p/chromium/bisection/test-analysis/b/5711130048593920

Sample build with failed test: https://ci.chromium.org/b/8675339005816383857
Affected test(s):
[://chrome/android\:chrome_junit_tests!junit:org.chromium.chrome.browser.ntp_customization:NtpCustomizationCoordinatorUnitTest#testShowBottomSheet_SyncEnabled[29]](https://ci.chromium.org/ui/test/chromium/:%2F%2Fchrome%2Fandroid%5C:chrome_junit_tests%21junit:org.chromium.chrome.browser.ntp_customization:NtpCustomizationCoordinatorUnitTest%23testShowBottomSheet_SyncEnabled%5B29%5D?q=VHash%3A4d5baf101df4bf09)
[://chrome/android\:chrome_junit_tests!junit:org.chromium.chrome.browser.ntp_customization:NtpCustomizationCoordinatorUnitTest#testShowBottomSheet_SyncEnabled[36]](https://ci.chromium.org/ui/test/chromium/:%2F%2Fchrome%2Fandroid%5C:chrome_junit_tests%21junit:org.chromium.chrome.browser.ntp_customization:NtpCustomizationCoordinatorUnitTest%23testShowBottomSheet_SyncEnabled%5B36%5D?q=VHash%3A4d5baf101df4bf09)

If this is a false positive, please report it at http://b.corp.google.com/createIssue?component=1199205&description=Analysis%3A+https%3A%2F%2Fci.chromium.org%2Fui%2Fp%2Fchromium%2Fbisection%2Ftest-analysis%2Fb%2F5711130048593920&format=PLAIN&priority=P3&title=Wrongly+blamed+https%3A%2F%2Fchromium-review.googlesource.com%2Fc%2Fchromium%2Fsrc%2F%2B%2F8126275&type=BUG

Original change's description:
> [ThemeSync] Add pre-defined theme collection options.
>
> In this CL, we allow to add at most 2 pre-defined theme collection
> images when the options in the history list is less than 6.
> 1. The history list will show: default + local history + remote
>    history + pre-defined options (2 chrome colors + 2 theme
>    collections);
> 2. In NtpThemeSyncHistoryCoordinator, we fetch URLs from the theme
>    collection id0 and filter out any URLs already exist in local
>    history to prevent duplicated entries;
> 3. For pre-defined theme collection images, we fetch its preview image
>    and show it in the recyclerview. The preview image is saved in
>    NtpBackgroundDataImageBase.
> 4. After the user the user clicks a pre-defined theme collection item,
>    fetch its background bitmap. After the bitmap is ready, we will pick
>    the primary color based on the preview bitmap, and save the data to
>    the local history of the shared preference. This is different from
>    a regular theme collection data, the saving data is done when the
>    bottom sheet is closed. If user clicks it again, we reused the
>    fetched bitmap.
> 5. Resolve an image missing issue when clicking multiple options from
>    the history list. This is because the newly selected items will be
>    persistent to the shared preference, and the bitmap of the removed
>    entry will be deleted as at most 3 items are kept in local history.
>    To fix it, any time when a item (originally from the local history)
>    is clicked, we save its bitmap to disk in case it has been deleted
>    before. We don't update the history list when it is showing.
> 6. Fix the issue that recyclerview's scroll position is reset after
>    clicking an item at the end by returning the history recyclerview
>    in NtpCustomizationBottomSheetContent.
> Demo: http://shortn/_AEeTlThxRE.
>
> Bug: 488439751
> Change-Id: If21e27a6a672886f0f7f6c8345e7266413e1252c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8126275
> Commit-Queue: Xi Han <ha...@chromium.org>
> Reviewed-by: Xinyi Ji <xin...@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1668241}
>

Bug: 488439751

No-Presubmit: true

No-Tree-Checks: true

No-Try: true

Change-Id: I28bbd700f5aca3bd9afbee1d5b87bdfb9f4709bc

Change diff

Change information

Files:

M chrome/browser/ntp_customization/java/src/org/chromium/chrome/browser/ntp_customization/NtpCustomizationBottomSheetContent.java
M chrome/browser/ntp_customization/java/src/org/chromium/chrome/browser/ntp_customization/NtpCustomizationCoordinator.java
M chrome/browser/ntp_customization/java/src/org/chromium/chrome/browser/ntp_customization/NtpCustomizationUtils.java
M chrome/browser/ntp_customization/java/src/org/chromium/chrome/browser/ntp_customization/theme/theme_collections/NtpThemeCollectionManager.java
M chrome/browser/ntp_customization/java/src/org/chromium/chrome/browser/ntp_customization/theme_sync/NtpThemeSyncHistoryCoordinator.java
M chrome/browser/ntp_customization/java/src/org/chromium/chrome/browser/ntp_customization/theme_sync/NtpThemeSyncHistoryRecyclerViewAdaptor.java
M chrome/browser/ntp_customization/java/src/org/chromium/chrome/browser/ntp_customization/theme_sync/data/NtpBackgroundDataImageBase.java
M chrome/browser/ntp_customization/java/src/org/chromium/chrome/browser/ntp_customization/theme_sync/data/NtpBackgroundDataManager.java
M chrome/browser/ntp_customization/java/src/org/chromium/chrome/browser/ntp_customization/theme_sync/data/NtpBackgroundDataThemeCollection.java
M chrome/browser/ntp_customization/junit/src/org/chromium/chrome/browser/ntp_customization/NtpCustomizationBottomSheetContentUnitTest.java
M chrome/browser/ntp_customization/junit/src/org/chromium/chrome/browser/ntp_customization/theme_sync/NtpThemeSyncHistoryCoordinatorUnitTest.java
M chrome/browser/ntp_customization/junit/src/org/chromium/chrome/browser/ntp_customization/theme_sync/NtpThemeSyncHistoryRecyclerViewAdaptorUnitTest.java

Change size: XL

Delta: 12 files changed, 119 insertions(+), 965 deletions(-)

Open in Gerrit

Related details

Attention is currently required from:

Xi Han
Xinyi Ji

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

luci-bisection@appspot.gserviceaccount.com (Gerrit)

unread,

2:13 AM (2 hours ago) 2:13 AM

to rubber-...@appspot.gserviceaccount.com, Chromium LUCI CQ, Xinyi Ji, Xi Han, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, hanxi...@chromium.org

Attention needed from Xi Han and Xinyi Ji

Message from luci-bi...@appspot.gserviceaccount.com

LUCI Bisection could not automatically submit this revert because LUCI Bisection has not yet support auto-commit of revert CL for test failure.

Open in Gerrit

Related details

Attention is currently required from:

Xi Han
Xinyi Ji

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

rubber-stamper@appspot.gserviceaccount.com (Gerrit)

unread,

2:14 AM (2 hours ago) 2:14 AM

to luci-bi...@appspot.gserviceaccount.com, Chromium LUCI CQ, Xinyi Ji, Xi Han, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, hanxi...@chromium.org

Attention needed from Xi Han and Xinyi Ji

rubber-...@appspot.gserviceaccount.com voted Bot-Commit+1

Bot-Commit

Open in Gerrit

Related details

Attention is currently required from:

Xi Han
Xinyi Ji

Submit Requirements:

Code-Coverage
Code-Owners

Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Reply all

Reply to author

Forward

0 new messages