Do you know anything more specific than "there are cases"? MostVisitedSites deduplicates between sources, but not within sources, so solving the issue properly probably involves a lower level.

Sorry, no specific issues as we don't log URLs, and the only way we notice that is by crashes caused when the UI received duplicate URLs as Most Visited data (see linked bugs)

An issue is that I don't have crash data about this on Dev/Canary (not sure if data is not collected or the issue is just rare enough to never show up). So we would have to merge CLs to 58 to get more info on that. 

I just landed a patch on master to fix the crash in issue 703578. We could have another one logging more info before crashing, or reporting that to UMA. I'd assume we can't log URLs, but maybe just the tile source?
Ideally the backend should do that though, the UI doesn't have much visibility in how the tile data is acquired.

Chris, are you the right owner for this? I am aware of a series of open issues around duplicate tiles, perhaps this issue is a dupe of one of those, or can be marked as blocking a tracking bug?

Should this still be P1? In that case it needs a target milestone. If not, please reduce priority.

Most of the "duplicate tiles" bugs are, AFAIK, about tiles that appear to be duplicates, but don't actually have the same URL (like google.com vs. www.google.com). This is about ntp_tiles providing the same URL twice.

In the brief investigation I did, it looked like ntp_tiles was working properly, but some other component (like Most Likely or Top Sites) could still have a problem. I don't have any intention to look into that soon, so P2 I guess.

This bug started about crashes but the direction it went is pretty much what is described in  issue 752928 .

I don't understand how that bug is related. That bug appears to be about tiles where the URLs are different. This bug is about tiles where the URLs are the same.

Okay. Does this still happen? 
(In theory, we shouldn't get enough tiles in that case because we only send the required amount to the UI and UI would reduce that amount by deduplicating. I have never encountered not having enough tiles unless I manually removed popular sites.)

I recently changed the duplicate check to thrown an IllegalStateException when we run into it. So far no crashes, but we might want to let it go to Beta or later before concluding that it does not happen anymore.

Crash link: https://crash.corp.google.com/browse?q=custom_data.ChromeCrashProto.magic_signature_1.name%20LIKE%20%27%25IllegalStateException%25TileGroup%25%27%20AND%20product.Version%20LIKE%20%2762.0.%25%27&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D&unnest=&stbtiq=&reportid=&index=0

Check in code: https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chrome/browser/suggestions/TileGroup.java?l=352

Issue 776938 has been merged into this issue.

Copied labels from other bug.

I think this could be caused by overlap in supervised user whitelists. I don't suppose we can determine if the crash happened for a supervised user account…

I deleted my comment about this not being in M62 since it should. This issue is the cause of a new crash 776938 in M62 stable which seems severe. What is the current progress on this? Do we have a fix?

https://crrev.com/c/734040 is out for review. It doesn't fix the root problem (which remains unknown) but should prevent the crash.

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

commit 734fa1a007546b823df0e539ca1340e27156a428
Author: Chris Pickel <sfiera@chromium.org>
Date: Tue Oct 24 10:57:54 2017

Android NTP: don't "throw IllegalStateException()"

The ntp_tiles component is not supposed to return multiple tiles with
the same URL. In order to determine if that still happened in the wild,
the throw statement was added, with the intention of catching the
problem in Beta. However, it seems that Beta hasn't had trouble with
this, but now it's a major crasher on Stable.

It looks to me that it's sufficient to simply remove the throw and
replace it with a continue.

Bug: 703628
Change-Id: If5d1b24a8e69b34d92ae6558c7bcbffad5995906
Reviewed-on: https://chromium-review.googlesource.com/734040
Commit-Queue: Chris Pickel <sfiera@chromium.org>
Reviewed-by: Nicolas Dossou-Gbété <dgn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511086}
[modify] https://crrev.com/734fa1a007546b823df0e539ca1340e27156a428/chrome/android/java/src/org/chromium/chrome/browser/suggestions/TileGroup.java
[modify] https://crrev.com/734fa1a007546b823df0e539ca1340e27156a428/chrome/android/junit/src/org/chromium/chrome/browser/suggestions/TileGroupUnitTest.java

Requesting merge of https://crrev.com/c/734040 into M62/63. It definitely fixes the specific crash seen in https://crbug.com/776938, and I believe it's very unlikely to cause other problems.

This bug requires manual review: Request affecting a post-stable build
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Heads up: I'm likely to leave the office by 20:00 CEST/11:00 PDT. I created cherry-pick CLs already, in case a decision to move forward arrives today after I've left (the CL applies cleanly).

M62: refs/branch-heads/3202: https://crrev.com/c/735355
M63: refs/branch-heads/3239: https://crrev.com/c/735612

Any committer can, after double-checking that I've picked the right branch heads, land them.

Thanks sfiera@ Merges approved!

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

commit c716bc2910e88e25752545ea1fbfe0a68a7b66e3
Author: Chris Pickel <sfiera@chromium.org>
Date: Tue Oct 24 17:25:18 2017

Android NTP: don't "throw IllegalStateException()"

The ntp_tiles component is not supposed to return multiple tiles with
the same URL. In order to determine if that still happened in the wild,
the throw statement was added, with the intention of catching the
problem in Beta. However, it seems that Beta hasn't had trouble with
this, but now it's a major crasher on Stable.

It looks to me that it's sufficient to simply remove the throw and
replace it with a continue.

Bug: 703628
Change-Id: If5d1b24a8e69b34d92ae6558c7bcbffad5995906
Reviewed-on: https://chromium-review.googlesource.com/734040
Commit-Queue: Chris Pickel <sfiera@chromium.org>
Reviewed-by: Nicolas Dossou-Gbété <dgn@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#511086}(cherry picked from commit 734fa1a007546b823df0e539ca1340e27156a428)
Reviewed-on: https://chromium-review.googlesource.com/735612
Reviewed-by: Chris Pickel <sfiera@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#177}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/c716bc2910e88e25752545ea1fbfe0a68a7b66e3/chrome/android/java/src/org/chromium/chrome/browser/suggestions/TileGroup.java
[modify] https://crrev.com/c716bc2910e88e25752545ea1fbfe0a68a7b66e3/chrome/android/junit/src/org/chromium/chrome/browser/suggestions/TileGroupUnitTest.java

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

commit e1e9a335645d36a20237858a98ac128c2be71476
Author: Chris Pickel <sfiera@chromium.org>
Date: Tue Oct 24 17:25:17 2017

Android NTP: don't "throw IllegalStateException()"

The ntp_tiles component is not supposed to return multiple tiles with
the same URL. In order to determine if that still happened in the wild,
the throw statement was added, with the intention of catching the
problem in Beta. However, it seems that Beta hasn't had trouble with
this, but now it's a major crasher on Stable.

It looks to me that it's sufficient to simply remove the throw and
replace it with a continue.

Bug: 703628
Change-Id: If5d1b24a8e69b34d92ae6558c7bcbffad5995906
Reviewed-on: https://chromium-review.googlesource.com/734040
Commit-Queue: Chris Pickel <sfiera@chromium.org>
Reviewed-by: Nicolas Dossou-Gbété <dgn@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#511086}(cherry picked from commit 734fa1a007546b823df0e539ca1340e27156a428)
Reviewed-on: https://chromium-review.googlesource.com/735355
Reviewed-by: Chris Pickel <sfiera@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{#734}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/e1e9a335645d36a20237858a98ac128c2be71476/chrome/android/java/src/org/chromium/chrome/browser/suggestions/TileGroup.java
[modify] https://crrev.com/e1e9a335645d36a20237858a98ac128c2be71476/chrome/android/junit/src/org/chromium/chrome/browser/suggestions/TileGroupUnitTest.java

Anything work pending here?  Can we close?

It's not blocking anymore as the crash should be fixed, but we still have work to do for M-64. Updating the labels.

We need to write a postmortem for the crash since it caused a respin of the stable build. Who will own writing it? sfiera@?

I can do it, but due to travel and vacation, I won't have time in the next two weeks.

sfiera@ have you started writing the postmortem for this issue? Can you share it if that's the case?

(not working on client-side Chrome any more)