Migrate PageLoadTracker to non-deprecated observer APIs (addresses DidFirstPaintAfterLoad and DidFinishLoad crashers) |
||||||||||
Issue description
Unfortunately there are no repro steps for this yet.
Its hitting a DCHECK on PageLoadTracker.
void PageLoadTracker::DidFirstPaintAfterLoad(
content::RenderWidgetHost* render_widget_host) {
RenderWidgetLoadStatusMap::iterator it =
render_widget_load_status_.find(render_widget_host);
DCHECK(it != render_widget_load_status_.end()); // <-----
it->second.did_first_paint = true;
if (it->second.Loaded()) {
client_->SendPageLoadStatusUpdate(PageLoadStatus::LOADED);
render_widget_load_status_.erase(it);
}
}
Product name: Chrome_Blimp_Engine
Magic Signature: DidFirstPaintAfterLoad
Current link:
https://crash.corp.google.com/browse?q=product.name%3D'Chrome_Blimp_Engine'%20AND%20custom_data.ChromeCrashProto.ptype%3D'browser'%20AND%20(product.version%3D'55.0.2858.0'%20OR%20product.version%3D'55.0.2860.0')%20AND%20custom_data.ChromeCrashProto.magic_signature_1.name%3D'DidFirstPaintAfterLoad'%20AND%20ReportID%3D'eb7d47f500000000'&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D#3
Search properties:
product.name: Chrome_Blimp_Engine
custom_data.chromecrashproto.ptype: browser
product.version: 55.0.2858.0' OR product.version='55.0.2860.0
custom_data.chromecrashproto.magic_signature_1.name: DidFirstPaintAfterLoad
reportid: eb7d47f500000000
Metadata :
Product Name: Chrome_Blimp_Engine
Product Version: 55.0.2860.0
Report ID: eb7d47f500000000
Report Time: Thu, 15 Sep 2016 23:43:06 GMT
Uptime: 18732115 ms
Cumulative Uptime: 0 ms
User Email:
OS Name: Linux
OS Version: 0.0.0 Linux 3.16.0-4-amd64 #1 SMP Debian 3.16.7-ckt20-1+deb8u4google (2016-01-26) x86_64
CPU Architecture: amd64
CPU Info: family 6 model 63 stepping 0
,
Sep 19 2016
I don't think start load and paint should be correlated much. Commit and paint might be. Also, are you monitoring for failed provisional loads? Including kenrb@, who has done some work in this area for one of our URL spoof mitigations.
,
Sep 19 2016
So the state tracking we do keys off of the following, DidStartProvisionalLoadForFrame: We expect to see before any of the following calls. At this point we are in a state for tracking this navigation's load status. DidFinishLoad: At this point we know that the main frame has finished loading. DidFirstPaintAfterLoad: At this point we know that the main frame has performed its first paint. The 2 above can come in any order so we wait to see both before assuming that the page's data has been sent to the blimp client. DidFailLoad: Now we know that the load failed so we reset any tracking state for this frame. This is the one to listen to for failed provisional loads right?
,
Sep 19 2016
A provisional load can fail, which I think you need to account for. Also, I would suggest moving over to use the observer APIs which are not deprecated - DidStartNavigation/DidFinishNavigation. Note that loading is the phase which comes once the navigation is complete (navigation - aka we have gotten data from the network and ready to load).
,
Nov 4 2016
Issue 662233 has been merged into this issue.
,
Nov 14 2016
,
Nov 14 2016
Issue 630360 has been merged into this issue.
,
Nov 14 2016
Issue 623591 has been merged into this issue.
,
Nov 14 2016
,
Nov 15 2016
I already have a patch up to fix this, https://codereview.chromium.org/2483933003/
,
Nov 15 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1dcad9ce4e13062d7fe0fa8021749e9dab6942a6 commit 1dcad9ce4e13062d7fe0fa8021749e9dab6942a6 Author: khushalsagar <khushalsagar@chromium.org> Date: Tue Nov 15 03:34:28 2016 blimp: Fix page load status tracking. Update the PageLoadTracker to use the non-deprecated APIs for tracking the status of navigations for a RenderFrame. This change also establishes the policy used for these updates for a tab clearly: 1) The progress bar will be reset when a navigation is initiated. If the navigation fails, a completion update is sent immediately. 2) If the navigation commits, we wait for at least one frame for this page to be sent to the client, before sending a completion update. BUG= 648299 Review-Url: https://codereview.chromium.org/2483933003 Cr-Commit-Position: refs/heads/master@{#432071} [modify] https://crrev.com/1dcad9ce4e13062d7fe0fa8021749e9dab6942a6/blimp/engine/session/page_load_tracker.cc [modify] https://crrev.com/1dcad9ce4e13062d7fe0fa8021749e9dab6942a6/blimp/engine/session/page_load_tracker.h
,
Nov 15 2016
,
Dec 9 2016
|
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by khushals...@chromium.org
, Sep 19 2016