Closed Bug 1245723 Opened 6 years ago Closed 6 years ago

Make a bunch of toolkit browser-chrome tests e10s compatible

Categories

(Toolkit :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
e10s + ---
firefox47 --- fixed

People

(Reporter: mrbkap, Assigned: mrbkap)

References

(Blocks 1 open bug)

Details

Attachments

(5 files)

No description provided.
browser_bug471404.js had DOS line endings and an absolutely bizarre control flow. This is much better. The others were easier to convert (though I didn't bother using all of the new helpers).
Attachment #8715603 - Flags: review?(felipc)
Comment on attachment 8715600 [details] [diff] [review]
Make the satchel browser-chrome test e10s compatible.

The original version of this test appeared to have a bug: it asserted that there shouldn't be any FormHistory values at all (even in non-private mode), but that makes no sense. The code bore this out, I think, because it raced the database query with the insertion of the value in form history. In this version, I fixed the race so that in private mode, we're asserting that we never add to form history (according to the parent) in PB mode and that we do in non-PB mode. Let me know if I should go back to the old, racy, way (though someone would have to explain why that's preferable).
Attachment #8715600 - Flags: review?(felipc) → review+
Attachment #8715601 - Flags: review?(felipc) → review+
Comment on attachment 8715601 [details] [diff] [review]
Make browser_bug680727 e10s compatible.

Review of attachment 8715601 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/places/tests/browser/browser_bug680727.js
@@ +22,5 @@
> +        resolve();
> +      });
> +    });
> +  });
> +}

I wonder if we should transform this into a helper like:

{ContentTask|BrowserTestUtils}.waitOnContentEvent(browser, "DOMContentLoaded")
Attachment #8715603 - Flags: review?(felipc) → review+
Attached patch With thatSplinter Review
Here's an interdiff.
Attachment #8716518 - Flags: review?(felipc)
Attachment #8716518 - Flags: review?(felipc) → review+
Attached patch More fixesSplinter Review
The re-enabled tests in browser/../social now pass (see also bug 1150147, comment 4).
Attachment #8717168 - Flags: review?(felipc)
Attachment #8717168 - Flags: review?(felipc) → review+
I tried to uplift these to aurora to get e10s greened up, but hit conflicts. Could we get rebased patches if these would be useful on aurora?
You need to log in before you can comment on or make changes to this bug.