gecko-dev/toolkit/components/statusfilter
Samael Wang cc0857dffa Bug 1414745 - Filter out everything except STATE_IS_NETWORK in nsBrowserStatusFilter::OnStateChange. r=mconley
The mFinishedRequests / mTotalRequests counters in nsBrowserStatusFilter has
been known buggy for around a decade. With e10s we double filtered in both
parent and child processes which makes it worse.

There are a few problems with the counters / filter:
1. The ResetMembers() were invoked on each network request starts, which means
   a iframe document loading followed by the root document loading would also
   reset the members incorrectly.
2. The filter for non-network request was incorrect. The basic idea seems to be
   that if `mFinishedRequests == mTotalRequests` and `!isLoadingDocument`, it
   should be the STOP request right after document loading finishes, and that
   STOP should be delivered so listener would get symmetric numbers of START /
   STOP of STATE_IS_REQUEST. However some requests such as imgRequest can start
   after document loading finishes, in this case the START would be filtered
   out (since mFinishedRequests != mTotalRequests), but STOP would be delivered
   to the listener. It's the reason that nsBrowserStatusFilter tend to deliver
   much more STATE_STOP than STATE_START.
3. When applying the filter on both parent & child side, the above issues often
   make the mFinishedRequests / mFinishedRequests be unmatched on parent side,
   eseentially filtered out most non-network requests and make the
   progressChange based on the counters useless.

Firefox no longer shows the ratio of progressChange on the UI (and the number
is incorrect anyway with current nsBrowserStatusFilter), and Fennec's progress
bar is based on some predefined constants [1] which doesn't rely on
progressChange either, so it not necessary to keep calculating a progress
number with request counters.

In addition, it seems tabbrowser & browser.js mostly only care about
STATE_IS_NETWORK, and Fennec has already filtered out everything else [2], it
should be safe to only pass STATE_IS_NETWORK to the listener, and we get the
benefit of reducing unused IPC messages.

[1] https://searchfox.org/mozilla-central/rev/9bab9dc5a9472e3c163ab279847d2249322c206e/mobile/android/base/java/org/mozilla/gecko/Tab.java#111-115
[2] https://searchfox.org/mozilla-central/rev/9bab9dc5a9472e3c163ab279847d2249322c206e/mobile/android/chrome/content/browser.js#4356-4357

MozReview-Commit-ID: 5tUP5SRwDoP

--HG--
extra : rebase_source : 9be66bf79f08fac176d9c35eb7a2b2d78484d8d3
2017-11-17 10:48:37 +08:00
..
moz.build Bug 1351067 - add BUG_COMPONENT to toolkit/* files. r=myk,enndeakin,mossop 2017-04-09 05:43:43 -04:00
nsBrowserStatusFilter.cpp Bug 1414745 - Filter out everything except STATE_IS_NETWORK in nsBrowserStatusFilter::OnStateChange. r=mconley 2017-11-17 10:48:37 +08:00
nsBrowserStatusFilter.h Bug 1414745 - Filter out everything except STATE_IS_NETWORK in nsBrowserStatusFilter::OnStateChange. r=mconley 2017-11-17 10:48:37 +08:00