1577819 - Fix a few issues with ChromeUtils::RequestProcInfo()

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Andrew McCreight [:mccr8]]

I was reading over this code, and I think there are a few issues I want to clean up.

• mozilla::dom::ProcType is too similar of a name to mozilla::ProcType, so I'm going to rename it to mozilla::dom::WebIDLProcType.
• ProcType gets static casted to the WebIDL ProcType, but right now they don't actually match. I'm not sure if the idea is that only ProcType values that correspond to the WebIDL proctype should be cast there, but it seems more direct to just require that they are the same. I update the WebIDL ProcType and add a static assert that they at least have the same number of cases, and add a comment to ProcType mentioning the WebIDL ProcType.
• ChromeUtils::RequestProcInfo() tries to match a few things against processType. Right now, if any of those fail to match you end up with an uninitialized type. This causes intermittent failures with Fission. I've added an else that sets it to Unknown.
• browser_test_procinfo.js doesn't do any checks of the |type| field. I added a few basic checks for it to the test, which makes this consistently fail with Fission, as expected.

I'm going to leave fixing this with Fission for a separate bug, as I think this is enough for now.

Comment 1

[Andrew McCreight [:mccr8]]

Attachment: Bug 1577819, part 1 - Rename mozilla::dom::ProcType to mozilla::dom::WebIDLProcType.

There is also a mozilla::ProcType, which makes things
confusing. dom::ProcType seems to be used only for passing values to
JS via WebIDL, so I put WebIDL in the name.

Comment 2

[Andrew McCreight [:mccr8]]

Attachment: Bug 1577819, part 2 - WebIDLProcType should match ProcType.

ProcType gets static casted to WebIDLProcType. I assume the intention
is that they are identical. At a minimum, there should be a static
assert that both enums have the same number of cases.

Comment 3

[Andrew McCreight [:mccr8]]

Attachment: Bug 1577819, part 3 - Initialize process type in RequestProcInfo.

Right now, the type ends up being uninitialized if the type doesn't
match.

Comment 4

[Andrew McCreight [:mccr8]]

Attachment: Bug 1577819, part 4 - Add some basic validity checks for the type.

The test for procinfo doesn't check if the process type is
reasonable. With Fission, this test now asserts, so add a failure
annotation. I'll fix it up for Fission in another bug.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/01e8fd20bdc1
part 1 - Rename mozilla::dom::ProcType to mozilla::dom::WebIDLProcType. r=Ehsan
https://hg.mozilla.org/integration/autoland/rev/dd470e3dff60
part 2 - WebIDLProcType should match ProcType. r=Ehsan
https://hg.mozilla.org/integration/autoland/rev/a9ac86bbe8d0
part 3 - Initialize process type in RequestProcInfo. r=Ehsan
https://hg.mozilla.org/integration/autoland/rev/98059acdf5be
part 4 - Add some basic validity checks for the type. r=Ehsan

Comment 6

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/01e8fd20bdc1
https://hg.mozilla.org/mozilla-central/rev/dd470e3dff60
https://hg.mozilla.org/mozilla-central/rev/a9ac86bbe8d0
https://hg.mozilla.org/mozilla-central/rev/98059acdf5be

Comment 7

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/releases/mozilla-beta/rev/01e8fd20bdc1
https://hg.mozilla.org/releases/mozilla-beta/rev/dd470e3dff60
https://hg.mozilla.org/releases/mozilla-beta/rev/a9ac86bbe8d0
https://hg.mozilla.org/releases/mozilla-beta/rev/98059acdf5be