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

Issue 895541 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

[EoS] catalog validation

Project Member Reported by petewil@chromium.org, Oct 15

Issue description

We should validate the Explore on Sites catalog.

This could include checking the URL, ensuring that the category index is in bounds of the enum, making sure the site has a title, and making sure the image renders properly.
 
Labels: OS-Android
Labels: android-fe-triaged
Status: Assigned (was: Untriaged)
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 25

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

commit ed67db073168bd0dedbbc9cdc5de4089b5abc997
Author: Pete Williamson <petewil@chromium.org>
Date: Thu Oct 25 20:54:41 2018

[EoS] Add catalog validation with metrics and unit tests.

Adds new validation methods to the catalog that Explore on Sites gets
from the network.  We now check:
Categories have a valid title and the category type is known.
Categories have at least one valid site in them.
Sites have a valid URL and title.

Bug: 895541
Change-Id: I437a5d116d5047739ca4e915589121589a05302d
Reviewed-on: https://chromium-review.googlesource.com/c/1290141
Reviewed-by: Justin DeWitt <dewittj@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Peter Williamson <petewil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602849}
[modify] https://crrev.com/ed67db073168bd0dedbbc9cdc5de4089b5abc997/chrome/browser/android/explore_sites/explore_sites_service_impl.cc
[modify] https://crrev.com/ed67db073168bd0dedbbc9cdc5de4089b5abc997/chrome/browser/android/explore_sites/explore_sites_service_impl.h
[modify] https://crrev.com/ed67db073168bd0dedbbc9cdc5de4089b5abc997/chrome/browser/android/explore_sites/explore_sites_service_impl_unittest.cc
[modify] https://crrev.com/ed67db073168bd0dedbbc9cdc5de4089b5abc997/chrome/browser/android/explore_sites/explore_sites_types.h
[modify] https://crrev.com/ed67db073168bd0dedbbc9cdc5de4089b5abc997/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/ed67db073168bd0dedbbc9cdc5de4089b5abc997/tools/metrics/histograms/histograms.xml

Labels: Merge-Request-71
This has been verified in Canary.
Project Member

Comment 5 by sheriffbot@chromium.org, Oct 26

Labels: -Merge-Request-71 Hotlist-Merge-Reject Merge-Reject-71
The bug is marked as P3 or Feature. It should not be merged as M71 is in beta. 
Please contact the approriate milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Pri-3 -Hotlist-Merge-Reject -Merge-Reject-71 Merge-Request-71 Pri-2
P3 was wrong for this bug, we really do want it for this milestone.
This is to support the Explore on Sites feature which is behind a feature flag.
Project Member

Comment 7 by sheriffbot@chromium.org, Oct 26

Labels: -Merge-Request-71 Hotlist-Merge-Review Merge-Review-71
This bug requires manual review: M71 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Merge-Review -Merge-Review-71 Merge-Approved-71
Merge approved to 71, branch 3578.
Cc: benmason@chromium.org
Project Member

Comment 10 by bugdroid1@chromium.org, Oct 30

Labels: -merge-approved-71 merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e937ebaa81066f71ad56d8ab053683448d21d03a

commit e937ebaa81066f71ad56d8ab053683448d21d03a
Author: Pete Williamson <petewil@chromium.org>
Date: Tue Oct 30 22:07:45 2018

[EoS] Add catalog validation with metrics and unit tests.

Adds new validation methods to the catalog that Explore on Sites gets
from the network.  We now check:
Categories have a valid title and the category type is known.
Categories have at least one valid site in them.
Sites have a valid URL and title.

TBR=petewil@chromium.org

(cherry picked from commit ed67db073168bd0dedbbc9cdc5de4089b5abc997)

Bug: 895541
Change-Id: I437a5d116d5047739ca4e915589121589a05302d
Reviewed-on: https://chromium-review.googlesource.com/c/1290141
Reviewed-by: Justin DeWitt <dewittj@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Peter Williamson <petewil@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#602849}
Reviewed-on: https://chromium-review.googlesource.com/c/1308817
Reviewed-by: Peter Williamson <petewil@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#422}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/e937ebaa81066f71ad56d8ab053683448d21d03a/chrome/browser/android/explore_sites/explore_sites_service_impl.cc
[modify] https://crrev.com/e937ebaa81066f71ad56d8ab053683448d21d03a/chrome/browser/android/explore_sites/explore_sites_service_impl.h
[modify] https://crrev.com/e937ebaa81066f71ad56d8ab053683448d21d03a/chrome/browser/android/explore_sites/explore_sites_service_impl_unittest.cc
[modify] https://crrev.com/e937ebaa81066f71ad56d8ab053683448d21d03a/chrome/browser/android/explore_sites/explore_sites_types.h
[modify] https://crrev.com/e937ebaa81066f71ad56d8ab053683448d21d03a/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/e937ebaa81066f71ad56d8ab053683448d21d03a/tools/metrics/histograms/histograms.xml

Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/e937ebaa81066f71ad56d8ab053683448d21d03a

Commit: e937ebaa81066f71ad56d8ab053683448d21d03a
Author: petewil@chromium.org
Commiter: petewil@chromium.org
Date: 2018-10-30 22:07:45 +0000 UTC

[EoS] Add catalog validation with metrics and unit tests.

Adds new validation methods to the catalog that Explore on Sites gets
from the network.  We now check:
Categories have a valid title and the category type is known.
Categories have at least one valid site in them.
Sites have a valid URL and title.

TBR=petewil@chromium.org

(cherry picked from commit ed67db073168bd0dedbbc9cdc5de4089b5abc997)

Bug: 895541
Change-Id: I437a5d116d5047739ca4e915589121589a05302d
Reviewed-on: https://chromium-review.googlesource.com/c/1290141
Reviewed-by: Justin DeWitt <dewittj@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Peter Williamson <petewil@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#602849}
Reviewed-on: https://chromium-review.googlesource.com/c/1308817
Reviewed-by: Peter Williamson <petewil@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#422}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/439ba6d76c7177712e206cf248c699f5ec55ff51

Commit: 439ba6d76c7177712e206cf248c699f5ec55ff51
Author: dewittj@chromium.org
Commiter: dewittj@chromium.org
Date: 2018-10-30 22:33:37 +0000 UTC

Revert "[EoS] Add catalog validation with metrics and unit tests."

This reverts commit e937ebaa81066f71ad56d8ab053683448d21d03a.

Reason for revert: Manula merge broke beta build

Original change's description:
> [EoS] Add catalog validation with metrics and unit tests.
> 
> Adds new validation methods to the catalog that Explore on Sites gets
> from the network.  We now check:
> Categories have a valid title and the category type is known.
> Categories have at least one valid site in them.
> Sites have a valid URL and title.
> 
> TBR=petewil@chromium.org
> 
> (cherry picked from commit ed67db073168bd0dedbbc9cdc5de4089b5abc997)
> 
> Bug: 895541
> Change-Id: I437a5d116d5047739ca4e915589121589a05302d
> Reviewed-on: https://chromium-review.googlesource.com/c/1290141
> Reviewed-by: Justin DeWitt <dewittj@chromium.org>
> Reviewed-by: Steven Holte <holte@chromium.org>
> Commit-Queue: Peter Williamson <petewil@chromium.org>
> Cr-Original-Commit-Position: refs/heads/master@{#602849}
> Reviewed-on: https://chromium-review.googlesource.com/c/1308817
> Reviewed-by: Peter Williamson <petewil@chromium.org>
> Cr-Commit-Position: refs/branch-heads/3578@{#422}
> Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}

TBR=petewil@chromium.org,dewittj@chromium.org,holte@chromium.org

Change-Id: I2f2be0c7b9512a5b95d3abf2eab20ccdbf83165a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 895541
Reviewed-on: https://chromium-review.googlesource.com/c/1308609
Reviewed-by: Justin DeWitt <dewittj@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#424}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
Project Member

Comment 13 by bugdroid1@chromium.org, Oct 30

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

commit 439ba6d76c7177712e206cf248c699f5ec55ff51
Author: Justin DeWitt <dewittj@chromium.org>
Date: Tue Oct 30 22:33:37 2018

Revert "[EoS] Add catalog validation with metrics and unit tests."

This reverts commit e937ebaa81066f71ad56d8ab053683448d21d03a.

Reason for revert: Manula merge broke beta build

Original change's description:
> [EoS] Add catalog validation with metrics and unit tests.
> 
> Adds new validation methods to the catalog that Explore on Sites gets
> from the network.  We now check:
> Categories have a valid title and the category type is known.
> Categories have at least one valid site in them.
> Sites have a valid URL and title.
> 
> TBR=petewil@chromium.org
> 
> (cherry picked from commit ed67db073168bd0dedbbc9cdc5de4089b5abc997)
> 
> Bug: 895541
> Change-Id: I437a5d116d5047739ca4e915589121589a05302d
> Reviewed-on: https://chromium-review.googlesource.com/c/1290141
> Reviewed-by: Justin DeWitt <dewittj@chromium.org>
> Reviewed-by: Steven Holte <holte@chromium.org>
> Commit-Queue: Peter Williamson <petewil@chromium.org>
> Cr-Original-Commit-Position: refs/heads/master@{#602849}
> Reviewed-on: https://chromium-review.googlesource.com/c/1308817
> Reviewed-by: Peter Williamson <petewil@chromium.org>
> Cr-Commit-Position: refs/branch-heads/3578@{#422}
> Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}

TBR=petewil@chromium.org,dewittj@chromium.org,holte@chromium.org

Change-Id: I2f2be0c7b9512a5b95d3abf2eab20ccdbf83165a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 895541
Reviewed-on: https://chromium-review.googlesource.com/c/1308609
Reviewed-by: Justin DeWitt <dewittj@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#424}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/439ba6d76c7177712e206cf248c699f5ec55ff51/chrome/browser/android/explore_sites/explore_sites_service_impl.cc
[modify] https://crrev.com/439ba6d76c7177712e206cf248c699f5ec55ff51/chrome/browser/android/explore_sites/explore_sites_service_impl.h
[modify] https://crrev.com/439ba6d76c7177712e206cf248c699f5ec55ff51/chrome/browser/android/explore_sites/explore_sites_service_impl_unittest.cc
[modify] https://crrev.com/439ba6d76c7177712e206cf248c699f5ec55ff51/chrome/browser/android/explore_sites/explore_sites_types.h
[modify] https://crrev.com/439ba6d76c7177712e206cf248c699f5ec55ff51/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/439ba6d76c7177712e206cf248c699f5ec55ff51/tools/metrics/histograms/histograms.xml

Labels: -merge-merged-3578 -Merge-Merged-71-3578 Merge-Approved-71
Since this got reverted, changing the merge-merged tag back to a merge-approved tag. 
Project Member

Comment 15 by bugdroid1@chromium.org, Oct 31

Labels: -merge-approved-71 merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b764130fd169a8310ff48ec31814dd240d7873fd

commit b764130fd169a8310ff48ec31814dd240d7873fd
Author: Pete Williamson <petewil@chromium.org>
Date: Wed Oct 31 00:01:47 2018

[EoS] Add catalog validation with metrics and unit tests.

Adds new validation methods to the catalog that Explore on Sites gets
from the network.  We now check:
Categories have a valid title and the category type is known.
Categories have at least one valid site in them.
Sites have a valid URL and title.

TBR=petewil@chromium.org

(cherry picked from commit ed67db073168bd0dedbbc9cdc5de4089b5abc997)

Bug: 895541
Change-Id: Ie5ea6811131b573b11dda2cd59514ead442570fd
Reviewed-on: https://chromium-review.googlesource.com/c/1290141
Reviewed-by: Justin DeWitt <dewittj@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Peter Williamson <petewil@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#602849}
Reviewed-on: https://chromium-review.googlesource.com/c/1309221
Reviewed-by: Peter Williamson <petewil@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#426}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/b764130fd169a8310ff48ec31814dd240d7873fd/chrome/browser/android/explore_sites/explore_sites_service_impl.cc
[modify] https://crrev.com/b764130fd169a8310ff48ec31814dd240d7873fd/chrome/browser/android/explore_sites/explore_sites_service_impl.h
[modify] https://crrev.com/b764130fd169a8310ff48ec31814dd240d7873fd/chrome/browser/android/explore_sites/explore_sites_service_impl_unittest.cc
[modify] https://crrev.com/b764130fd169a8310ff48ec31814dd240d7873fd/chrome/browser/android/explore_sites/explore_sites_types.h
[modify] https://crrev.com/b764130fd169a8310ff48ec31814dd240d7873fd/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/b764130fd169a8310ff48ec31814dd240d7873fd/tools/metrics/histograms/histograms.xml

Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/b764130fd169a8310ff48ec31814dd240d7873fd

Commit: b764130fd169a8310ff48ec31814dd240d7873fd
Author: petewil@chromium.org
Commiter: petewil@chromium.org
Date: 2018-10-31 00:01:47 +0000 UTC

[EoS] Add catalog validation with metrics and unit tests.

Adds new validation methods to the catalog that Explore on Sites gets
from the network.  We now check:
Categories have a valid title and the category type is known.
Categories have at least one valid site in them.
Sites have a valid URL and title.

TBR=petewil@chromium.org

(cherry picked from commit ed67db073168bd0dedbbc9cdc5de4089b5abc997)

Bug: 895541
Change-Id: Ie5ea6811131b573b11dda2cd59514ead442570fd
Reviewed-on: https://chromium-review.googlesource.com/c/1290141
Reviewed-by: Justin DeWitt <dewittj@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Peter Williamson <petewil@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#602849}
Reviewed-on: https://chromium-review.googlesource.com/c/1309221
Reviewed-by: Peter Williamson <petewil@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#426}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}

Sign in to add a comment