Sorry, I mean SRP not AGSA.

Might be the same as issue 738177; jam, do you think it is?

I don't have enable-site-per-process flipped so I don't think they're the same *but* it seems like the root cause of issue 738177 is still unclear?

hmm, it's possible it's related to the myriad bugs that are side effects of my SSL change. I don't think it's the same as 738177, since as far as we can tell it's only with plznavigate and we didn't experiment with plznavigate in m59. There have been other fixes in m60 though.

If this is quick for you to repro, can you try with canary or dev (with browser-side-navigation flag on and off)?

bug 738177 has a potential fix in https://codereview.chromium.org/2974553002/. I think I understand that issue now, which is a race with plznavigate when there are more than one navigation which uses the same renderframe.

Latest telemetry is that we lose the icon on 0.85% of https navigations (regardless of PlzNavigate). That is really pretty high. It's over 4% on Android.

jam and clamy, I think we need a plan to fix this reliably in M63 at latest or else we need to go back to something like the old architecture, where we pass the SSL information to the renderer and pass it back to the browser on commit, until we've figured this out. I don't like that architecture but it at least works reliably.

I don't really understand the improvements suggested in https://bugs.chromium.org/p/chromium/issues/detail?id=738177&desc=2#c17 and whether they will help with the session restore case only or all cases of this bug?

I have been looking at ways to solve the issue and we have a few ways we could achieve this with PlzNavigate. I should be sending a design doc out this week that describes the potential fixes.

In any case, I'm hoping that if we can launch PlzNavigate this particular issue will be alleviated, because the timeframe for the race condition that leads to the deletion of the NavigationHandle is much shorter when PlzNavigate is enabled.

few comments and questions:
-@estark: looks like on Android, beta is half the error rate of stable. We'll see if this carries through after stable launch tomorrow...
-@estark: I just checked and the stats are coming for all frames. Should I restrict it to just the main frame, as that's the only thing that would affect page lock icon, to see if it makes a difference?
-@clamy: the error rates are the same for plznavigate and old path (checked before we switched plznavigate to 100% on beta), so while I agree your change could solve this, I don't think plznavigate as is will improve this?

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

commit 509dfd6cb0a5dcbbcbd8680923165b2b1d3d01ee
Author: John Abd-El-Malek <jam@chromium.org>
Date: Tue Sep 05 21:34:49 2017

Only log stats for when an https navigation doesn't have a certificate for main frames.

BUG=740336

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: I0196b28a2f2ee3d2552d37e1ebe1612b99166de9
Reviewed-on: https://chromium-review.googlesource.com/649965
Reviewed-by: Emily Stark <estark@chromium.org>
Commit-Queue: Emily Stark <estark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499754}
[modify] https://crrev.com/509dfd6cb0a5dcbbcbd8680923165b2b1d3d01ee/content/browser/frame_host/navigation_controller_impl.cc

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

commit 35157903fdfa028189697024094f25d3238098c1
Author: John Abd-El-Malek <jam@chromium.org>
Date: Wed Sep 06 21:26:48 2017

Use correct URL for SSL cert histograms

Some paths were incorrectly checking old URLs (which might be https that redirected to http).

Bug=740336

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: Ifeaf963d176f4db8544c84a49947e0be01e8da9a
Reviewed-on: https://chromium-review.googlesource.com/653297
Reviewed-by: Emily Stark <estark@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500093}
[modify] https://crrev.com/35157903fdfa028189697024094f25d3238098c1/content/browser/frame_host/navigation_controller_impl.cc

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

commit 380d3c5921b861af944ad3824214b96fb756bf87
Author: John Abd-El-Malek <jam@chromium.org>
Date: Fri Sep 08 00:20:31 2017

Only log SSL cert error histograms for successful page loads.

Otherwise navigating to https://doesnt-exit.com or possible captive error pages would get incorrectly flagged in the stats.

Bug=740336

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: Id170382d3407d21f502b9d71fb6245c69fa2f0a9
Reviewed-on: https://chromium-review.googlesource.com/656017
Commit-Queue: Emily Stark <estark@chromium.org>
Reviewed-by: Emily Stark <estark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500447}
[modify] https://crrev.com/380d3c5921b861af944ad3824214b96fb756bf87/content/browser/frame_host/navigation_controller_impl.cc

The data from today's canary is still coming in. The latest data ignores loads that failed, which was a measurement error in the histogram.

With that, Windows (400K data points) is showing 0.21% error rate:
https://uma.googleplex.com/p/chrome/histograms/?endDate=20170908&dayCount=1&histograms=Navigation.SecureSchemeHasSSLStatus%2CNavigation.SecureSchemeHasSSLStatus.ExistingPageBrowserInitiated%2CNavigation.SecureSchemeHasSSLStatus.ExistingPageDifferentDocumentIntendedAsNew%2CNavigation.SecureSchemeHasSSLStatus.ExistingPageDifferentDocumentRendererInitiated%2CNavigation.SecureSchemeHasSSLStatus.ExistingPageRestoredBrowserInitiated%2CNavigation.SecureSchemeHasSSLStatus.ExistingPageSameDocumentBrowserInitiated%2CNavigation.SecureSchemeHasSSLStatus.ExistingPageSameDocumentIntendedAsNew%2CNavigation.SecureSchemeHasSSLStatus.ExistingPageSameDocumentRendererInitiated%2CNavigation.SecureSchemeHasSSLStatus.ExistingPageSameDocumentRestoredBrowserInitiated%2CNavigation.SecureSchemeHasSSLStatus.NewPageInPage%2CNavigation.SecureSchemeHasSSLStatus.NewPageInPageOriginMismatch%2CNavigation.SecureSchemeHasSSLStatus.NewPageNoMatchingEntry%2CNavigation.SecureSchemeHasSSLStatus.NewPagePendingEntryMatches%2CNavigation.SecureSchemeHasSSLStatus.NewSubFrame%2CNavigation.SecureSchemeHasSSLStatus.SamePage&fixupData=true&showMax=true&filters=simple_version%2Ceq%2C63.0.3209.0%2Cisofficial%2Ceq%2CTrue&implicitFilters=isofficial

Android only has 2300 data points so far so still too little. However it's at .43% 
https://uma.googleplex.com/p/chrome/histograms/?endDate=20170908&dayCount=1&histograms=Navigation.SecureSchemeHasSSLStatus%2CNavigation.SecureSchemeHasSSLStatus.ExistingPageBrowserInitiated%2CNavigation.SecureSchemeHasSSLStatus.ExistingPageDifferentDocumentIntendedAsNew%2CNavigation.SecureSchemeHasSSLStatus.ExistingPageDifferentDocumentRendererInitiated%2CNavigation.SecureSchemeHasSSLStatus.ExistingPageRestoredBrowserInitiated%2CNavigation.SecureSchemeHasSSLStatus.ExistingPageSameDocumentBrowserInitiated%2CNavigation.SecureSchemeHasSSLStatus.ExistingPageSameDocumentIntendedAsNew%2CNavigation.SecureSchemeHasSSLStatus.ExistingPageSameDocumentRendererInitiated%2CNavigation.SecureSchemeHasSSLStatus.ExistingPageSameDocumentRestoredBrowserInitiated%2CNavigation.SecureSchemeHasSSLStatus.NewPageInPage%2CNavigation.SecureSchemeHasSSLStatus.NewPageInPageOriginMismatch%2CNavigation.SecureSchemeHasSSLStatus.NewPageNoMatchingEntry%2CNavigation.SecureSchemeHasSSLStatus.NewPagePendingEntryMatches%2CNavigation.SecureSchemeHasSSLStatus.NewSubFrame%2CNavigation.SecureSchemeHasSSLStatus.SamePage&fixupData=true&showMax=true&filters=simple_version%2Ceq%2C63.0.3210.0%2Cplatform%2Ceq%2CA%2Cisofficial%2Ceq%2CTrue&implicitFilters=isofficial


@estark: If Android sticks to roughly around .4% or lower, is that a good number that the severity/urgency of this bug can be dropped?

I'm seeing now 0.89% on Android which still seems kinda high. :(