Closed Bug 1375243 Opened 3 years ago Closed 3 years ago
Remove sync PHandler
Service::Msg _Exists IPC message
59 bytes, text/x-review-board-request
This seems to happen when loading the aljazeera page in the tp5o talos test with e10s enabled: https://perf-html.io/public/e05b0638aa95a62ecfd046f71760abbf37be5c50/calltree/?invertCallstack&range=0.3706_0.7437~0.3730_0.7405~0.3918_0.5029&thread=1
Blake, seems you reviewed this; can you take a look?
Priority: -- → P1
Assignee: nobody → mrbkap
In my investigation here, I basically came to the same conclusion as Gijs did in bug 3375244. In order to fix the profile from comment 0, we should be avoiding the URI fixup code altogether for links from content. That begin said, I do think it's relatively easy to fix Msg_Exists anyway, so I'll do that.
Depends on: 1375244
(In reply to Blake Kaplan (:mrbkap) from comment #2) > That begin said, I do think it's relatively easy to fix Msg_Exists anyway, > so I'll do that. This is actually a bit more complicated than I thought. I misread the code the other day and now realize that this is checking two different databases of handlers. It's possible to fix this by shipping handler apps down to the child. I'll toy with a patch that tries that tomorrow and see what it looks like. I still think way to fix this is to avoid this stuff altogether. Oh, and this case should be even more rare in the real world than this bug would suggest. This is happening because we rewrote our Tp5 suite to avoid network activity by rewriting all http: links as httpdisabled: links. That makes the fixup code think that this could actually be a mistyped URI, where I would wager that most <link href>s would be valid links (and would then skip this type of fixup). <link rel="alternate" type="application/rss+xml" title="الجزيرة نت" href="httpdisabled://www.aljazeera.net/AljazeeraRss/Rss.aspx?URL=RSS-portal.xml" />
Attachment #8895135 - Flags: review?(felipc) → review?(gijskruitbosch+bugs)
Comment on attachment 8895135 [details] Bug 1375243 - Explicitly create a URI object to avoid sync URI fixups from the content process. https://reviewboard.mozilla.org/r/166258/#review172168 Note that this will break showing RSS feeds with broken protocols that we used to fix up ("htp://foo.com/rss.xml") but I don't think we should care about that edgecase here.
Attachment #8895135 - Flags: review?(gijskruitbosch+bugs) → review+
(In reply to :Gijs from comment #6) > Note that this will break showing RSS feeds with broken protocols that we > used to fix up ("htp://foo.com/rss.xml") but I don't think we should care > about that edgecase here. To be clear, we never actually used the fixed-up URI to do anything. We'd do the security check on all of the default fixup variants (which included a few sync IPC calls) and then only ever add the invalid URI (which maps internally to an nsSimpleURI that we would be unable to load). This was purely wasted work.
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/99cf11868f47 Explicitly create a URI object to avoid sync URI fixups from the content process. r=Gijs
Had to back this out for failures like https://treeherder.mozilla.org/logviewer.html#?job_id=122399226&repo=autoland https://hg.mozilla.org/integration/autoland/rev/de7ef67a91478ceb71928e00c178a0f637050a98
Sorry about that. HTML elements must always have an ownerDocument (which will always have a characterSet). This test needs to mock its Link element more precisely.
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/87eaa504c2b5 Explicitly create a URI object to avoid sync URI fixups from the content process. r=Gijs
You need to log in before you can comment on or make changes to this bug.