Make tests use enableTestPlugin.js work with e10s

RESOLVED FIXED in Firefox 45

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: xidorn, Assigned: xidorn)

Tracking

(Blocks: 1 bug)

Trunk
mozilla45
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10s+, firefox44 affected, firefox45 fixed)

Details

Attachments

(3 attachments, 3 obsolete attachments)

(Assignee)

Description

3 years ago
There are two identical enableTestPlugin.js files in the tree:
* dom/html/test/enableTestPlugin.js
* layout/base/tests/enableTestPlugin.js

When a content process load any file uses this script, it would crash, because the script sets enabledState of the plugin, but that is only allowed in the parent process.

We may rewrite that script to make it ask the parent process to change the enabledState.

FWIW, there are currently 8 tests use this script.

Updated

3 years ago
tracking-e10s: --- → +
(Assignee)

Updated

3 years ago
Summary: Make enableTestPlugin.js work with e10s → Make tests use enableTestPlugin.js work with e10s
(Assignee)

Comment 1

3 years ago
It seems asking parent process to change the enableState of plugin has been fixed in plugin tests long ago in bug 641685. That just didn't cover the plugin-related tests elsewhere.

I think there are three strategies to fix this bug:
1. copy the code in dom/plugins/test/mochitest/utils.js to the enableTestPlugin.js files
2. move dom/plugins/test/mochitest/utils.js to be global accessible (e.g. SimpleTest/PluginUtils.js)
3. move tests use enableTestPlugin.js into dom/plugins/test/mochitest/

Not quite sure which is the best. For fullscreen plugin subtest, it is probably harder to move because of the dependency to fullscreen utils script. But others look fine.

The simplest way, though, is copying code, but I guess it would increase long-term maintainance burden.
Depends on: 641685
(Assignee)

Comment 2

3 years ago
Let's do it the forth way :)
Assignee: nobody → quanxunzhen
(Assignee)

Comment 4

3 years ago
Created attachment 8677264 [details]
MozReview Request: Bug 1213710 part 1 - Convert all html/xul files in dom/plugins/test/mochitest to unix format.

Bug 1213710 part 1 - Convert all html/xul files in dom/plugins/test/mochitest to unix format.
Attachment #8677264 - Flags: review?(benjamin)
(Assignee)

Comment 5

3 years ago
Created attachment 8677265 [details]
MozReview Request: Bug 1213710 part 2 - Rename dom/plugins/test/mochitest/utils.js to plugin-utils.js.

Bug 1213710 part 2 - Rename dom/plugins/test/mochitest/utils.js to plugin-utils.js.

So that files outside this dir can also reference it without conflict.
Attachment #8677265 - Flags: review?(benjamin)
(Assignee)

Comment 6

3 years ago
Created attachment 8677266 [details]
MozReview Request: Bug 1213710 part 3 - Remove enableTestPlugin.js files and make the tests reference plugin-utils.js directly.

Bug 1213710 part 3 - Remove enableTestPlugin.js files and make the tests reference plugin-utils.js directly.
Attachment #8677266 - Flags: review?(benjamin)
(Assignee)

Comment 7

3 years ago
The review has been placed there for 2 weeks. bsmedberg, any reason not reviewing these patches?
Flags: needinfo?(benjamin)

Updated

3 years ago
Attachment #8677266 - Flags: review?(benjamin) → review+

Comment 8

3 years ago
Comment on attachment 8677266 [details]
MozReview Request: Bug 1213710 part 3 - Remove enableTestPlugin.js files and make the tests reference plugin-utils.js directly.

https://reviewboard.mozilla.org/r/22915/#review21703

Sorry for the delay. I got stuck on not being able to r+ each commit separately, but that was stupid of me.

Comment 9

3 years ago
https://reviewboard.mozilla.org/r/22909/#review21705

Or maybe this is the right place to review it? Now I'm even more confused. There's no "ship it" button here.
(Assignee)

Comment 10

3 years ago
Oh. I can submit it again via the traditional way, if you prefer. I haven't do any reviewing on reviewboard, so I have no idea how that works.
(Assignee)

Comment 11

3 years ago
Created attachment 8683367 [details] [diff] [review]
part 1 - convert files to unix format
Attachment #8677264 - Attachment is obsolete: true
Attachment #8677264 - Flags: review?(benjamin)
Attachment #8683367 - Flags: review?(benjamin)
(Assignee)

Comment 12

3 years ago
Created attachment 8683368 [details] [diff] [review]
part 2 - rename utils.js in plugins test to plugin-utils.js
Attachment #8677265 - Attachment is obsolete: true
Attachment #8677265 - Flags: review?(benjamin)
(Assignee)

Updated

3 years ago
Attachment #8683368 - Flags: review?(benjamin)
(Assignee)

Comment 13

3 years ago
Created attachment 8683369 [details] [diff] [review]
part 3 - replace enableTestPlugin.js uses with plugin-utils.js, r=bsmedberg
Attachment #8677266 - Attachment is obsolete: true
Attachment #8683369 - Flags: review+
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #9)
> https://reviewboard.mozilla.org/r/22909/#review21705
> 
> Or maybe this is the right place to review it? Now I'm even more confused.
> There's no "ship it" button here.

Carrying the r+ over to the other 2 patches. r=bsmedberg
Flags: needinfo?(benjamin)
Attachment #8683368 - Flags: review?(benjamin) → review+
Attachment #8683367 - Flags: review?(benjamin) → review+
(Assignee)

Comment 15

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/91c332b973c7f709e8c49dfaa400a5774660eb0c
Bug 1213710 part 1 - Convert all html/xul files in dom/plugins/test/mochitest to unix format. r=bsmedberg

https://hg.mozilla.org/integration/mozilla-inbound/rev/744faf917eebb624f26febc118451ed3467d2e84
Bug 1213710 part 2 - Rename dom/plugins/test/mochitest/utils.js to plugin-utils.js. r=bsmedberg

https://hg.mozilla.org/integration/mozilla-inbound/rev/c3623da0112ef4e384c42d01d7eaa7934a56113c
Bug 1213710 part 3 - Remove enableTestPlugin.js files and make the tests reference plugin-utils.js directly. r=bsmedberg

Comment 16

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/91c332b973c7
https://hg.mozilla.org/mozilla-central/rev/744faf917eeb
https://hg.mozilla.org/mozilla-central/rev/c3623da0112e
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.