Closed Bug 1023565 Opened 10 years ago Closed 9 years ago

Intermittent browser_formdata.js | editable document has been restored correctly - Got <h1>mozilla</h1><script>document.designMode='on'</script>, expected <h1>value-0.3414578621044606</h1>

Categories

(Firefox :: Session Restore, defect)

Product:
Firefox
Component:
Session Restore
Version:
31 Branch
Type:
defect
Priority:
Not set
Severity:
normal
Points:
2

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 38
Iteration:
38.2 - 9 Feb
Tracking Flags:
Tracking Status
firefox36 --- fixed
firefox37 --- fixed
firefox38 --- fixed
firefox-esr31 --- unaffected

People

(Reporter: RyanVM, Assigned: ttaubert)

References

(Depends on 1 open bug)

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

0001-Bug-1023565-Remove-setTimeout-when-restoring-content.patch
1.95 KB, patch
smacleod
: review+
Sylvestre
: approval-mozilla-aurora+
Sylvestre
: approval-mozilla-beta+
Details | Diff | Splinter Review 
https://tbpl.mozilla.org/php/getParsedLog.php?id=41460105&tree=Fx-Team

Ubuntu VM 12.04 x64 fx-team opt test mochitest-browser-chrome-1 on 2014-06-10 11:21:43 PDT for push ae6a1a81af8e
slave: tst-linux64-spot-252

11:43:10     INFO -  TEST-START | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_formdata.js
11:43:10     INFO -  TEST-INFO | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_formdata.js | Entering test test_formdata
11:43:10     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_formdata.js | outer value is correct
11:43:10     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_formdata.js | inner value is correct
11:43:11     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_formdata.js | outer value is correct
11:43:11     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_formdata.js | inner value was *not* stored
11:43:11     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_formdata.js | form data has *not* been stored
11:43:11     INFO -  TEST-INFO | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_formdata.js | Leaving test test_formdata
11:43:11     INFO -  TEST-INFO | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_formdata.js | Entering test test_old_format
11:43:11     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_formdata.js | form data restored
11:43:11     INFO -  TEST-INFO | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_formdata.js | Leaving test test_old_format
11:43:11     INFO -  TEST-INFO | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_formdata.js | Entering test test_old_format_inner_html
11:43:12  WARNING -  TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_formdata.js | editable document has been restored correctly - Got <h1>mozilla</h1><script>document.designMode='on'</script>, expected <h1>value-0.3414578621044606</h1>
11:43:12     INFO -  Stack trace:
11:43:12     INFO -      JS frame :: chrome://mochikit/content/browser-test.js :: test_is :: line 752
11:43:12     INFO -      JS frame :: chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_formdata.js :: test_old_format_inner_html :: line 103
11:43:12     INFO -      JS frame :: resource://gre/modules/Task.jsm :: TaskImpl_run :: line 298
11:43:12     INFO -      JS frame :: resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js :: Handler.prototype.process :: line 863
11:43:12     INFO -      JS frame :: resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js :: this.PromiseWalker.walkerLoop :: line 742
11:43:12     INFO -      native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0
11:43:12     INFO -  TEST-INFO | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_formdata.js | Leaving test test_old_format_inner_html
11:43:12     INFO -  TEST-INFO | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_formdata.js | Entering test test_url_check
11:43:12     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_formdata.js | form data restored
11:43:12     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_formdata.js | form data not restored
11:43:13     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_formdata.js | form data not restored
11:43:13     INFO -  TEST-INFO | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_formdata.js | Leaving test test_url_check
11:43:13     INFO -  TEST-INFO | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_formdata.js | Entering test test_nested
11:43:13     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_formdata.js | formdata for iframe stored correctly
11:43:14     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_formdata.js | formdata for iframe restored correctly
11:43:14     INFO -  TEST-INFO | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_formdata.js | Leaving test test_nested
11:43:14     INFO -  TEST-INFO | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_formdata.js | Entering test test_design_mode
11:43:15     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_formdata.js | editable document has been restored correctly
11:43:15     INFO -  TEST-INFO | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_formdata.js | Leaving test test_design_mode
11:43:15     INFO -  TEST-INFO | MEMORY STAT vsize after test: 1717260288
11:43:15     INFO -  TEST-INFO | MEMORY STAT residentFast after test: 447844352
11:43:15     INFO -  TEST-INFO | MEMORY STAT heapAllocated after test: 178579992
11:43:15     INFO -  INFO TEST-END | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_formdata.js | finished in 5064ms
OS: Linux → All
Hardware: x86_64 → All
The problem here seems to be that we use setTimeout(x, 0) before restoring .innerHTML for contentEditable elements. I do not remember why this is there - I do remember though that it has been there from probably when SessionStore the add-on was copied to the tree. I assume this wasn't a conscious decision but rather to "make something work" back then?

The setTimeout() however introduces timing issues which let browser_formdata.js fail frequently. I can't seem to reproduce those failures anymore with the patch applied:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8703b558aa26
Assignee: nobody → ttaubert
Status: NEW → ASSIGNED
Attachment #8555852 - Flags: review?(smacleod)
Attachment #8555852 - Flags: review?(smacleod) → review+
https://hg.mozilla.org/integration/fx-team/rev/63c1d07df69b
Iteration: --- → 38.2 - 9 Feb
Points: --- → 2
Flags: qe-verify-
Flags: firefox-backlog+
https://hg.mozilla.org/mozilla-central/rev/63c1d07df69b
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 38
(In reply to TBPL Robot from comment #160)
> log:
> https://treeherder.mozilla.org/logviewer.html#?repo=fx-team&job_id=1820448
> repository: fx-team
> start_time: 2015-01-28T11:23:08
> who: rvandermeulen[at]mozilla[dot]com
> machine: t-snow-r4-0062
> buildname: Rev4 MacOSX Snow Leopard 10.6 fx-team opt test
> mochitest-browser-chrome-1
> revision: a98a30ba9ce0

Just checked, that was exactly the revision before my push. All good so far.
Indeed! Please request Aurora/Beta approval on this when you get a chance.
status-firefox36: --- → affected
status-firefox37: --- → affected
status-firefox38: --- → fixed
status-firefox-esr31: --- → unaffected
Flags: needinfo?(ttaubert)
Comment on attachment 8555852 [details] [diff] [review]
0001-Bug-1023565-Remove-setTimeout-when-restoring-content.patch

Approval Request Comment
[Feature/regressing bug #]: None.
[User impact if declined]: No user impact, will get rid of a semi-frequent intermittent test failure.
[Describe test coverage new/current, TreeHerder]: Covered by existing tests.
[Risks and why]: Risk is low. A small change covered by multiple tests.
[String/UUID change made/needed]: None.
Flags: needinfo?(ttaubert)
Attachment #8555852 - Flags: approval-mozilla-beta?
Attachment #8555852 - Flags: approval-mozilla-aurora?
Attachment #8555852 - Flags: approval-mozilla-beta?
Attachment #8555852 - Flags: approval-mozilla-beta+
Attachment #8555852 - Flags: approval-mozilla-aurora?
Attachment #8555852 - Flags: approval-mozilla-aurora+
Depends on: 1445644
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: