Closed Bug 1523457 Opened 5 years ago Closed 5 years ago

Don't call DocumentL10n::RemoveResourceIds during document's shutdown

Categories

(Core :: Internationalization, enhancement, P2)

Product:
Core
Component:
Internationalization
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla67
Tracking Flags:
Tracking Status
firefox67 --- fixed

People

(Reporter: zbraniecki, Assigned: zbraniecki)

References

Details

Attachments

(2 files)

Bug 1523457 - Don't call DocumentL10n::RemoveResourceIds during document's shutdown. r?smaug
47 bytes, text/x-phabricator-request
Details | Review
Bug 1523457 - Revert a workaround from bug 1517544. r?jaws
47 bytes, text/x-phabricator-request
Details | Review

Currently HTMLLinkElement::UnbindFromTree can sometimes call DocumentL10n::RemoveResourceIds during document's shutdown.

That's a waste of time, resources and potentially can cause bugs like bug 1517544.

In bug 1517544 we band aided it by making sure that when the last element is removed, we don't try to retranslate the document, but we can do better and not do that at all.

Assignee: nobody → gandalf
Status: NEW → ASSIGNED
Priority: -- → P2
See Also: → 1517544

Updated the patch to smaug's and jaws' feedback.
Also, adding a revert of the patch from bug 1517544 because it is not needed anymore.

New try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2e292076844fd766cfcc8f37cf9d509c432d23d8

Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/23cef9112d5e
Revert a workaround from bug 1517544. r=jaws
https://hg.mozilla.org/integration/autoland/rev/05a81f3e0729
Don't call DocumentL10n::RemoveResourceIds during document's shutdown. r=smaug

Backed out 2 changesets (Bug 1523457) for test_domloc* failures

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=05a81f3e0729b24958af6676c4b04023b2cc5a4e&selectedJob=224974750

Backout link: https://hg.mozilla.org/integration/autoland/rev/ad9a582a97bb862da7b933ec3e0a64593cd77f16

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=224974750&repo=autoland&lineNumber=6579

[task 2019-01-30T15:36:37.662Z] 15:36:37 INFO - TEST-START | intl/l10n/test/dom/test_domloc.xul
[task 2019-01-30T15:36:37.685Z] 15:36:37 INFO - TEST-INFO | started process screentopng
[task 2019-01-30T15:36:38.154Z] 15:36:38 INFO - TEST-INFO | screentopng: exit 0
[task 2019-01-30T15:36:38.155Z] 15:36:38 INFO - TEST-UNEXPECTED-FAIL | intl/l10n/test/dom/test_domloc.xul | undefined assertion name - got "", expected "File"
[task 2019-01-30T15:36:38.156Z] 15:36:38 INFO - SimpleTest.is@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:320:5
[task 2019-01-30T15:36:38.159Z] 15:36:38 INFO - foo@chrome://mochitests/content/chrome/intl/l10n/test/dom/test_domloc.xul:43:5
[task 2019-01-30T15:36:38.159Z] 15:36:38 INFO - Not taking screenshot here: see the one that was previously logged
[task 2019-01-30T15:36:38.159Z] 15:36:38 INFO - TEST-UNEXPECTED-FAIL | intl/l10n/test/dom/test_domloc.xul | undefined assertion name - got "", expected "F"
[task 2019-01-30T15:36:38.159Z] 15:36:38 INFO - SimpleTest.is@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:320:5
[task 2019-01-30T15:36:38.159Z] 15:36:38 INFO - foo@chrome://mochitests/content/chrome/intl/l10n/test/dom/test_domloc.xul:44:5
[task 2019-01-30T15:36:38.159Z] 15:36:38 INFO - Not taking screenshot here: see the one that was previously logged
[task 2019-01-30T15:36:38.159Z] 15:36:38 INFO - TEST-UNEXPECTED-FAIL | intl/l10n/test/dom/test_domloc.xul | undefined assertion name - got "", expected "New Tab"
[task 2019-01-30T15:36:38.160Z] 15:36:38 INFO - SimpleTest.is@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:320:5
[task 2019-01-30T15:36:38.161Z] 15:36:38 INFO - foo@chrome://mochitests/content/chrome/intl/l10n/test/dom/test_domloc.xul:46:5
[task 2019-01-30T15:36:38.162Z] 15:36:38 INFO - Not taking screenshot here: see the one that was previously logged
[task 2019-01-30T15:36:38.162Z] 15:36:38 INFO - TEST-UNEXPECTED-FAIL | intl/l10n/test/dom/test_domloc.xul | undefined assertion name - got "", expected "N"
[task 2019-01-30T15:36:38.163Z] 15:36:38 INFO - SimpleTest.is@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:320:5
[task 2019-01-30T15:36:38.163Z] 15:36:38 INFO - foo@chrome://mochitests/content/chrome/intl/l10n/test/dom/test_domloc.xul:47:5
[task 2019-01-30T15:36:38.165Z] 15:36:38 INFO - GECKO(8335) | MEMORY STAT | vsize 558MB | residentFast 253MB | heapAllocated 101MB
[task 2019-01-30T15:36:38.166Z] 15:36:38 INFO - TEST-OK | intl/l10n/test/dom/test_domloc.xul | took 29ms
...
[task 2019-01-30T15:36:38.179Z] 15:36:38 INFO - TEST-START | intl/l10n/test/dom/test_domloc_mutations.html
[task 2019-01-30T15:36:38.180Z] 15:36:38 INFO - Not taking screenshot here: see the one that was previously logged
[task 2019-01-30T15:36:38.183Z] 15:36:38 INFO - TEST-UNEXPECTED-FAIL | intl/l10n/test/dom/test_domloc_mutations.html | undefined assertion name - got "", expected "Hello World"
[task 2019-01-30T15:36:38.183Z] 15:36:38 INFO - SimpleTest.is@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:320:5
[task 2019-01-30T15:36:38.183Z] 15:36:38 INFO - window.onload@chrome://mochitests/content/chrome/intl/l10n/test/dom/test_domloc_mutations.html:36:5
[task 2019-01-30T15:36:38.184Z] 15:36:38 INFO - TEST-PASS | intl/l10n/test/dom/test_domloc_mutations.html | undefined assertion name
...
[task 2019-01-30T15:36:38.675Z] 15:36:38 INFO - TEST-START | intl/l10n/test/dom/test_domloc_translateRoots.html
[task 2019-01-30T15:36:38.715Z] 15:36:38 INFO - Not taking screenshot here: see the one that was previously logged
[task 2019-01-30T15:36:38.717Z] 15:36:38 INFO - TEST-UNEXPECTED-FAIL | intl/l10n/test/dom/test_domloc_translateRoots.html | undefined assertion name - got "", expected "Hello World"
[task 2019-01-30T15:36:38.719Z] 15:36:38 INFO - SimpleTest.is@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:320:5
[task 2019-01-30T15:36:38.720Z] 15:36:38 INFO - window.onload@chrome://mochitests/content/chrome/intl/l10n/test/dom/test_domloc_translateRoots.html:40:5
[task 2019-01-30T15:36:38.722Z] 15:36:38 INFO - Not taking screenshot here: see the one that was previously logged
[task 2019-01-30T15:36:38.724Z] 15:36:38 INFO - TEST-UNEXPECTED-FAIL | intl/l10n/test/dom/test_domloc_translateRoots.html | undefined assertion name - got "", expected "Hello Another World"
[task 2019-01-30T15:36:38.726Z] 15:36:38 INFO - SimpleTest.is@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:320:5
[task 2019-01-30T15:36:38.727Z] 15:36:38 INFO - window.onload@chrome://mochitests/content/chrome/intl/l10n/test/dom/test_domloc_translateRoots.html:41:5
[task 2019-01-30T15:36:38.729Z] 15:36:38 INFO - GECKO(8335) | MEMORY STAT | vsize 562MB | residentFast 260MB | heapAllocated 105MB
[task 2019-01-30T15:36:38.730Z] 15:36:38 INFO - TEST-OK | intl/l10n/test/dom/test_domloc_translateRoots.html | took 37ms

Flags: needinfo?(gandalf)
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/94e661dcc4fc
Revert a workaround from bug 1517544. r=jaws
https://hg.mozilla.org/integration/autoland/rev/daf87d7e54ff
Don't call DocumentL10n::RemoveResourceIds during document's shutdown. r=smaug

thank you! Relanding with the tests fixed.

Flags: needinfo?(gandalf)

https://hg.mozilla.org/mozilla-central/rev/94e661dcc4fc
https://hg.mozilla.org/mozilla-central/rev/daf87d7e54ff

Status: ASSIGNED → RESOLVED
Closed: 5 years ago
status-firefox67: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: