Closed Bug 1255180 Opened 8 years ago Closed 8 years ago

Convert test_bug370436.html to work with e10s

Categories

(Core :: Layout, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
e10s + ---
firefox48 --- fixed

People

(Reporter: dholbert, Assigned: dholbert)

References

(Blocks 2 open bugs)

Details

Attachments

(3 files)

Our mochitest-chrome tests are all disabled under e10s.

This test...
http://mxr.mozilla.org/mozilla-central/source/layout/base/tests/chrome/test_bug370436.html?force=1
...seems like something that we should be testing with e10s enabled, though. 

It's testing text-selection & context-menu-spawning, I think, which seems like it could differ between e10s & non-e10s browser configurations.
Handy e10s conversion doc:
 https://wiki.mozilla.org/Electrolysis/e10s_test_tips

There are some details about synthesizeKey & synthesizeMouse there. (This test uses both of those APIs.)
Blocks: e10s-tests
tracking-e10s: --- → +
So the first main part of the test that's chrome-only is here:

> 28 function onContextMenu(e)
> 29 {
> 30 	var node = e.rangeParent;
> [...]
> 35 	var word = expandStringOffsetToWord(node.data, offset);

"e.rangeParent", for our UIEvent "e", only returns something non-null if we're running with Chrome privileges.  The check for that happens here:
> 226 already_AddRefed<nsINode>
> 227 UIEvent::GetRangeParent()
> [...]
> 240       if (parent->ChromeOnlyAccess() &&
> 241           !nsContentUtils::CanAccessNativeAnon()) {
> 242         return nullptr;
> 243       }
http://mxr.mozilla.org/mozilla-central/source/dom/events/UIEvent.cpp#240

ChromeOnlyAccess() returns true because |parent| has the NODE_IS_IN_NATIVE_ANONYMOUS_SUBTREE flag  (because it's some text inside of a text area), and nsContentUtils::CanAccessNativeAnon() returns false.

I suspect we want to use (or add) a SpecialPowers API that will allow this code to succeed...

If I make a quick local hack to simply disable the security checks in GetRangeParent(), then we successfully get a non-null "node", but we fail on the attempt to read "node.data".  (Error: Permission denied to access property "data")  Not yet sure where that check happens or how to get around that.
(In reply to Daniel Holbert [:dholbert] from comment #2)
> I suspect we want to use (or add) a SpecialPowers API that will allow this
> code to succeed...

mccr8 tells me in that "SpecialPowers.wrap()" is what we want here. Patches coming shortly.
Assignee: nobody → dholbert
This patch also temporarily disables the test by commenting it out in mochitest.ini, its new home. (I'm disabling it temporarily because it won't pass until later patches in this queue have landed.)

Review commit: https://reviewboard.mozilla.org/r/47297/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/47297/
Attachment #8742565 - Flags: review?(bobbyholley)
Attachment #8742566 - Flags: review?(bobbyholley)
Attachment #8742567 - Flags: review?(bobbyholley)
NOTE: The only piece here that I'm not 100% sure about is the "Shouldn't have SOWs in chrome" check-removal.

I'm pretty sure I should remove that, because:
 (1) If I convert it over so that I can run it in mochitest-plain (with this test as an e10s-enabled test) then that check fails, because the node's type is "Proxy" instead of "Text".
 (2) The check's text explicitly mentions "in chrome", and this test is becoming no-longer-a-chrome-mochitest.

Bobby, let me know if it's important that this check stay around somehow. (You added it in Bug 832512.)
Comment on attachment 8742565 [details]
MozReview Request: Bug 1255180 part 1: Move test_bug370436.html out of its mochitest-chrome subdirectory. r?bholley

https://reviewboard.mozilla.org/r/47297/#review43995
Attachment #8742565 - Flags: review?(bobbyholley) → review+
Attachment #8742566 - Flags: review?(bobbyholley) → review+
Comment on attachment 8742566 [details]
MozReview Request: Bug 1255180 part 2: Update script/style paths in test_bug370436.html to point to mochitest-plain resources. r?bholley

https://reviewboard.mozilla.org/r/47299/#review43997
Comment on attachment 8742567 [details]
MozReview Request: Bug 1255180 part 3: Update test_bug370436.html to use SpecialPowers for privileged "rangeParent" access, & drop now-obsolete checks for SOWs in chrome. r?bholley

https://reviewboard.mozilla.org/r/47301/#review43999

We don't have SOWs anymore - they've been replaced with XBL scopes, and I'm not worried about the correctness of when we do XBL scopes.
Attachment #8742567 - Flags: review?(bobbyholley) → review+
Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=32aa3a39c9c6

It looks good, aside from unrelated mega-orange on OS X 10.10 (a platform which is hidden on trunk, presumably in part due to this mega-orange).

I verified that this test is indeed showing up in M-e10s run output, too -- on Linux Opt in that Try run, it's listed in mochitest-e10s-4:
> 18:59:20     INFO -  1882 INFO TEST-START | layout/base/tests/test_bug370436.html
> 18:59:20     INFO -  MEMORY STAT | vsize 354MB | residentFast 85MB | heapAllocated 26MB
> 18:59:20     INFO -  1883 INFO TEST-OK | layout/base/tests/test_bug370436.html | took 369ms

So, I'll go ahead and land. Thanks for the review!
You need to log in before you can comment on or make changes to this bug.