Closed Bug 959886 Opened 11 years ago Closed 11 years ago

Handle offline cache updates in parent process after windows are being torn down in child processes

Categories

(Core :: Networking, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla29
blocking-b2g 1.3+
Tracking Status
firefox27 --- wontfix
firefox28 --- fixed
firefox29 --- fixed
b2g-v1.3 --- fixed
b2g-v1.4 --- fixed

People

(Reporter: bent.mozilla, Assigned: bent.mozilla)

References

Details

Attachments

(1 file)

Attached patch changes.patchSplinter Review
We're currently sending offline cache update notifications to child processes even after we've started the PBrowser teardown sequence, leading to MsgRouteErrors. Attached patch should fix the issue. I also removed an unnecessary function (Kill) and a nonexistent function (RefcountHitZero), and added a few more MOZ_OVERRIDE annotations.
Attachment #8360131 - Flags: review?(honzab.moz)
Comment on attachment 8360131 [details] [diff] [review] changes.patch Review of attachment 8360131 [details] [diff] [review]: ----------------------------------------------------------------- untested locally. please push to try (and expose the link here) before landing. r=honzab ::: dom/ipc/TabParent.cpp @@ +280,5 @@ > + const InfallibleTArray<POfflineCacheUpdateParent*>& ocuParents = > + ManagedPOfflineCacheUpdateParent(); > + for (uint32_t i = 0; i < ocuParents.Length(); ++i) { > + static_cast<mozilla::docshell::OfflineCacheUpdateParent*>(ocuParents[i])-> > + StopSendingMessagesToChild(); please, to a local var first (for debugging) @@ +1578,4 @@ > { > + // A reference is guaranteed here, no need for a nsRefPtr. > + auto update = > + static_cast<mozilla::docshell::OfflineCacheUpdateParent*>(aActor); I'd still rather see it used here. ::: uriloader/prefetch/OfflineCacheUpdateParent.cpp @@ -60,5 @@ > - // Make sure the service has been initialized > - nsOfflineCacheUpdateService* service = > - nsOfflineCacheUpdateService::EnsureService(); > - if (!service) > - return; Please leave this here, it inits the NSPR log.
Attachment #8360131 - Flags: review?(honzab.moz) → review+
(In reply to Honza Bambas (:mayhemer) from comment #1) > > - // Make sure the service has been initialized > > - nsOfflineCacheUpdateService* service = > > - nsOfflineCacheUpdateService::EnsureService(); > > - if (!service) > > - return; > > Please leave this here, it inits the NSPR log. Hrm, can I remove the null check then? Returning early from a constructor is... weird. Luckily failure is handled in Schedule though.
(In reply to ben turner [:bent] (use the needinfo? flag!) from comment #2) > Hrm, can I remove the null check then? Returning early from a constructor > is... weird. Luckily failure is handled in Schedule though. Definitely, go ahead.
Comment on attachment 8360131 [details] [diff] [review] changes.patch Review of attachment 8360131 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/ipc/TabParent.cpp @@ +1578,3 @@ > { > + // A reference is guaranteed here, no need for a nsRefPtr. > + auto update = btw, are you sure auto is supported by all compilers we claim to build on?
(In reply to Honza Bambas (:mayhemer) from comment #5) > btw, are you sure auto is supported by all compilers we claim to build on? Yep!
blocking a blocker
blocking-b2g: --- → 1.3+
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: