I checked ProgressTracker because I can't get fully loaded contents in several web page. I realized that those site had bad resource which takes very long times. but in this case, ProgressTracker also wasn't updated well. attachment is my test log using GtkLauncher. as you can see, series of finishedCallback were called without got-chunk callback sometimes. so, incrementProgress wasn't called After simply adding incrementProgress in ResourceLoadNotifier::dispatchDidFinishLoading, I could get more accurate progress. but, I don't know whether it's right approach or not.
Created attachment 60501 [details] my test log
Created attachment 60705 [details] Patch
Comment on attachment 60705 [details] Patch Thanks for the patch, but a number of things are unclear to me: WebCore/loader/ProgressTracker.cpp:184 + if (frame->loader()->client()) Do we need to null-check the client here? Is the client disappearing somehow? WebCore/loader/ProgressTracker.cpp: + // FIXME: Can this ever happen? Did we resolve this FIXME? It might be worth explaining when item might be NULL. In general, we would like to see a test with each patch. Is there a way to test this code change? I'm not 100% sure this is the right direction. It's difficult to tell from this bug report and from the ChangeLog exactly what problem we're solving here. It's also unclear to me how this change effects all the different clients of resource load notifier.
Thank you for your interesting (In reply to comment #3) > (From update of attachment 60705 [details]) > Thanks for the patch, but a number of things are unclear to me: > > WebCore/loader/ProgressTracker.cpp:184 > + if (frame->loader()->client()) > Do we need to null-check the client here? Is the client disappearing somehow? > It's my mistake. I'll remove it. > WebCore/loader/ProgressTracker.cpp: > + // FIXME: Can this ever happen? > Did we resolve this FIXME? It might be worth explaining when item might be NULL. > this patch make a case which item was null. so I moved this check logic like below - item->bytesReceived += bytesReceived; - if (item->bytesReceived > item->estimatedLength) { - m_totalPageAndResourceBytesToLoad += ((item->bytesReceived * 2) - item->estimatedLength); - item->estimatedLength = item->bytesReceived * 2; + + if (item) { + item->bytesReceived += bytesReceived; + if (item->bytesReceived > item->estimatedLength) { + m_totalPageAndResourceBytesToLoad += ((item->bytesReceived * 2) - item->estimatedLength); + item->estimatedLength = item->bytesReceived * 2; + } } > In general, we would like to see a test with each patch. Is there a way to test this code change? I'm not 100% sure this is the right direction. It's difficult to tell from this bug report and from the ChangeLog exactly what problem we're solving here. It's also unclear to me how this change effects all the different clients of resource load notifier. you are right, but I don't know how to make proper test case. In almost other network situation, this wasn't happened. especially I am testing wifi through slow proxy server. anyway I'll prepare some video file and more log for you. If I am possible, I can show you this issues using android browser (based on proyo) in my test environment.