Closed Bug 1510934 Opened 6 years ago Closed 5 years ago

Intermittent dom/tests/mochitest/ajax/offline/browser_disableAppcache.js | Appcache is enabled - "no" == "yes" - due to lost preference updates during async content process launch

Categories

(Core :: Preferences: Backend, defect, P1)

Product:
Core
Component:
Preferences: Backend
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla66
Tracking Flags:
Tracking Status
firefox-esr60 --- unaffected
firefox64 --- unaffected
firefox65 --- fixed
firefox66 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: jld)

References

Details

(Keywords: intermittent-failure, Whiteboard: [retriggered])

Attachments

(2 files)

Bug 1510934 - Change ContentParent's alive/dead tracking to launching/alive/dead.
47 bytes, text/x-phabricator-request
Details | Review
Bug 1510934 - Ensure that pref changes during content process launch are delivered to the process.
47 bytes, text/x-phabricator-request
RyanVM
: approval-mozilla-beta+
Details | Review 
Filed by: dluca [at] mozilla.com

https://treeherder.mozilla.org/logviewer.html#?job_id=214547954&repo=autoland

https://queue.taskcluster.net/v1/task/FZ76eTYeQw6ToqAOo7WDLQ/runs/0/artifacts/public/logs/live_backing.log

[task 2018-11-29T06:21:52.615Z] 06:21:52     INFO - GECKO(7014) | MEMORY STAT vsizeMaxContiguous not supported in this build configuration.
[task 2018-11-29T06:21:52.616Z] 06:21:52     INFO - GECKO(7014) | MEMORY STAT | vsize 621MB | residentFast 293MB | heapAllocated 105MB
[task 2018-11-29T06:21:52.617Z] 06:21:52     INFO - TEST-OK | dom/tests/mochitest/ajax/offline/browser_disableAppcache.js | took 4055ms
[task 2018-11-29T06:21:52.653Z] 06:21:52     INFO - checking window state
[task 2018-11-29T06:21:54.667Z] 06:21:54     INFO - GECKO(7014) | Completed ShutdownLeaks collections in process 7378
[task 2018-11-29T06:21:54.724Z] 06:21:54     INFO - GECKO(7014) | Completed ShutdownLeaks collections in process 7344
[task 2018-11-29T06:21:54.725Z] 06:21:54     INFO - GECKO(7014) | Completed ShutdownLeaks collections in process 7304
[task 2018-11-29T06:21:54.733Z] 06:21:54     INFO - GECKO(7014) | Completed ShutdownLeaks collections in process 7132
[task 2018-11-29T06:21:54.735Z] 06:21:54     INFO - GECKO(7014) | Completed ShutdownLeaks collections in process 7184
[task 2018-11-29T06:21:54.736Z] 06:21:54     INFO - GECKO(7014) | Completed ShutdownLeaks collections in process 7272
[task 2018-11-29T06:21:54.744Z] 06:21:54     INFO - GECKO(7014) | Completed ShutdownLeaks collections in process 7225
[task 2018-11-29T06:21:54.744Z] 06:21:54     INFO - GECKO(7014) | Completed ShutdownLeaks collections in process 7330
[task 2018-11-29T06:21:54.781Z] 06:21:54     INFO - GECKO(7014) | Completed ShutdownLeaks collections in process 7215
[task 2018-11-29T06:21:54.782Z] 06:21:54     INFO - GECKO(7014) | Completed ShutdownLeaks collections in process 7109
[task 2018-11-29T06:21:55.127Z] 06:21:55     INFO - GECKO(7014) | Completed ShutdownLeaks collections in process 7014
[task 2018-11-29T06:21:55.128Z] 06:21:55     INFO - TEST-START | Shutdown
[task 2018-11-29T06:21:55.129Z] 06:21:55     INFO - Browser Chrome Test Summary
[task 2018-11-29T06:21:55.131Z] 06:21:55     INFO - Passed:  6
[task 2018-11-29T06:21:55.132Z] 06:21:55     INFO - Failed:  2
[task 2018-11-29T06:21:55.133Z] 06:21:55     INFO - Todo:    0
[task 2018-11-29T06:21:55.140Z] 06:21:55     INFO - Mode:    e10s
[task 2018-11-29T06:21:55.144Z] 06:21:55     INFO - *** End BrowserChrome Test Results ***
[task 2018-11-29T06:21:55.627Z] 06:21:55     INFO - GECKO(7014) | 1543472515621	Marionette	DEBUG	Received observer notification xpcom-will-shutdown
[task 2018-11-29T06:21:55.627Z] 06:21:55     INFO - GECKO(7014) | 1543472515621	Marionette	INFO	Stopped listening on port 2828
[task 2018-11-29T06:21:55.627Z] 06:21:55     INFO - GECKO(7014) | 1543472515621	Marionette	DEBUG	Remote service is inactive
[task 2018-11-29T06:21:55.958Z] 06:21:55     INFO - TEST-INFO | Main app process: exit 0
[task 2018-11-29T06:21:52.508Z] 06:21:52     INFO - TEST-PASS | dom/tests/mochitest/ajax/offline/browser_disableAppcache.js | Loaded good content - "good" == "good" - 
[task 2018-11-29T06:21:52.510Z] 06:21:52     INFO - Leaving test bound 
[task 2018-11-29T06:21:52.514Z] 06:21:52     INFO - Entering test bound test_pref_removes_api
[task 2018-11-29T06:21:52.515Z] 06:21:52     INFO - Console message: [JavaScript Warning: "The Application Cache API (AppCache) is deprecated and will be removed at a future date.  Please consider using ServiceWorker for offline support." {file: "about:blank" line: 0}]
[task 2018-11-29T06:21:52.516Z] 06:21:52     INFO - Console message: [JavaScript Error: "Use of the Application Cache API (AppCache) for insecure connections will be removed in version 62." {file: "about:blank" line: 0}]
[task 2018-11-29T06:21:52.516Z] 06:21:52     INFO - Console message: [JavaScript Error: "The character encoding of the HTML document was not declared. The document will render with garbled text in some browser configurations if the document contains characters from outside the US-ASCII range. The character encoding of the page must be declared in the document or in the transfer protocol." {file: "http://example.com/browser/dom/tests/mochitest/ajax/offline/file_simpleManifest.html" line: 0}]
[task 2018-11-29T06:21:52.518Z] 06:21:52     INFO - Console message: Offline cache doesn't need to update, URL=http://example.com/browser/dom/tests/mochitest/ajax/offline/file_simpleManifest.cacheManifest
[task 2018-11-29T06:21:52.519Z] 06:21:52     INFO - Buffered messages finished
[task 2018-11-29T06:21:52.520Z] 06:21:52     INFO - TEST-UNEXPECTED-FAIL | dom/tests/mochitest/ajax/offline/browser_disableAppcache.js | Appcache is enabled - "no" == "yes" - 
[task 2018-11-29T06:21:52.521Z] 06:21:52     INFO - Stack trace:
[task 2018-11-29T06:21:52.522Z] 06:21:52     INFO - resource://testing-common/content-task.js line 59 > eval:null:5
[task 2018-11-29T06:21:52.525Z] 06:21:52     INFO - Not taking screenshot here: see the one that was previously logged
[task 2018-11-29T06:21:52.525Z] 06:21:52     INFO - TEST-UNEXPECTED-FAIL | dom/tests/mochitest/ajax/offline/browser_disableAppcache.js | OfflineResourceList is enabled - "no" == "yes" - 
[task 2018-11-29T06:21:52.526Z] 06:21:52     INFO - Stack trace:
[task 2018-11-29T06:21:52.526Z] 06:21:52     INFO - resource://testing-common/content-task.js line 59 > eval:null:6
[task 2018-11-29T06:21:52.527Z] 06:21:52     INFO - Console message: [JavaScript Error: "The character encoding of the HTML document was not declared. The document will render with garbled text in some browser configurations if the document contains characters from outside the US-ASCII range. The character encoding of the page must be declared in the document or in the transfer protocol." {file: "http://example.com/browser/dom/tests/mochitest/ajax/offline/file_simpleManifest.html" line: 0}]
[task 2018-11-29T06:21:52.528Z] 06:21:52     INFO - TEST-PASS | dom/tests/mochitest/ajax/offline/browser_disableAppcache.js | Appcache is disabled - "no" == "no" -
Component: Networking: Cache → Security
Product: Core → Firefox
Whiteboard: [retriggered]
I think I know what's going on here: that test fails because the (temporary) setting of browser.cache.offline.insecure.enable isn't picked up by the content process, so AppCache isn't available in the test page (which would normally be an insecure context and not eligible to use AppCache, if I understand correctly).

Getting prefs into content processes is a little weird: some of them need to be available before IPC is started, so there's code that runs before launch[1] to supply a snapshot; later, updates are streamed[2] over IPC once the pref observer is registered[3].  Before async launch, the main thread was blocked for that entire time; with async launch, it can do other things, which includes possibly setting prefs, which wouldn't be picked up.

This should be relatively easy to fix; the harder part will be testing it.


[1] https://searchfox.org/mozilla-central/rev/8f0db72fb6e35414fb9a6fc88af19c69f332425f/dom/ipc/ContentParent.cpp#2241
[2] https://searchfox.org/mozilla-central/rev/8f0db72fb6e35414fb9a6fc88af19c69f332425f/dom/ipc/ContentParent.cpp#3251
[3] https://searchfox.org/mozilla-central/rev/8f0db72fb6e35414fb9a6fc88af19c69f332425f/dom/ipc/ContentParent.cpp#1378
Assignee: nobody → jld
Blocks: 1446161
Component: Security → Preferences: Backend
Flags: needinfo?(jld)
Priority: P5 → P1
Product: Firefox → Core
Summary: Intermittent dom/tests/mochitest/ajax/offline/browser_disableAppcache.js | Appcache is enabled - "no" == "yes" - → Lost preference updates during async content process launch | Intermittent dom/tests/mochitest/ajax/offline/browser_disableAppcache.js | Appcache is enabled - "no" == "yes" -
Summary: Lost preference updates during async content process launch | Intermittent dom/tests/mochitest/ajax/offline/browser_disableAppcache.js | Appcache is enabled - "no" == "yes" - → Intermittent dom/tests/mochitest/ajax/offline/browser_disableAppcache.js | Appcache is enabled - "no" == "yes" - due to lost preference updates during async content process launch
We need content processes that are created but not finished launching
(not "alive" yet) to be treated differently from ones that have exited
(no longer "alive"), so the boolean mIsAlive is expanded to a 3-state
enumeration, which could be expanded more in the future if needed.

(This is similar to GeckoChildProcessHost::mProcessState, but it's
synchronized with the rest of the ContentParent's state, which can lag
the GeckoChildProcessHost state due to runnable dispatch.)

This patch also removes mIsAvailable/IsAvailable/MarkAsTroubled, which
are unused as of bug 1459212.
Some prefs need to be available before IPC is started, so we serialize a
snapshot when we start launching the process, and then stream further
changes over IPC messages.  However, async launch introduces a window
between the snapshot and when the parent can start sending messages,
during which other code can run on the main thread and change prefs.

In order to not lose those updates, they're queued and sent when the
launch is complete.

Depends on D14089
Pushed by jedavis@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c2e55bc9206b
Change ContentParent's alive/dead tracking to launching/alive/dead. r=mrbkap
https://hg.mozilla.org/integration/autoland/rev/74bfa63e922c
Ensure that pref changes during content process launch are delivered to the process. r=njn
I'll request uplift once this seems stable on Nightly.
Flags: needinfo?(jld)
Flags: needinfo?(jld)
https://hg.mozilla.org/mozilla-central/rev/c2e55bc9206b
https://hg.mozilla.org/mozilla-central/rev/74bfa63e922c
Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox66: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Comment on attachment 9030294 [details]
Bug 1510934 - Ensure that pref changes during content process launch are delivered to the process.

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1446161

User impact if declined: Prefs changes might be applied inconsistently, which might include internal uses of prefs.

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): There is a little refactoring and rearranging and dead code removal involved, but this has been stable for a few days on Nightly.  The functional change makes a difference only in the case that's broken without these patches.

(This comment applies to both patches on this bug; I can also flag the other one for approval if needed.)

String changes made/needed: None.
Flags: needinfo?(jld)
Attachment #9030294 - Flags: approval-mozilla-beta?
Comment on attachment 9030294 [details]
Bug 1510934 - Ensure that pref changes during content process launch are delivered to the process.

[Triage Comment]
Fixes inconsistent application of pref changes. Approved for 65.0b6.
Attachment #9030294 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: