Rename ContentParent::TransmitPermissionsFor() to ContentParent::AboutToLoadDocumentForChild()

RESOLVED FIXED in Firefox 55

Status

()

enhancement
RESOLVED FIXED
2 years ago
a month ago

People

(Reporter: Ehsan, Assigned: Ehsan)

Tracking

unspecified
mozilla55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
This gives the method a better name, and it makes it more obvious that it's a good place to stick in code in the parent process that needs to run as soon as possible when we know we're about to load a document for the child process.
(Assignee)

Updated

2 years ago
Blocks: 1350637
(Assignee)

Updated

2 years ago
Assignee: nobody → ehsan
Attachment #8863048 - Flags: review?(michael) → review+

Comment 2

2 years ago
Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4dea825d159b
Rename ContentParent::TransmitPermissionsFor() to ContentParent::AboutToLoadDocumentForChild(); r=mystor
Comment on attachment 8863048 [details] [diff] [review]
Rename ContentParent::TransmitPermissionsFor() to ContentParent::AboutToLoadDocumentForChild()

>+  // This function is called when we are about to load a document from an
>+  // HTTP(S), FTP or wyciwyg channel for a content process.  It is a useful
>+  // place to start to kick off work as early as possible in response to such
>+  // document loads.
>+  nsresult AboutToLoadDocumentForChild(nsIChannel* aChannel);

If this is really called for http(s), FTP or wyciwyg only, and not for example file channel, please call this something else.
This is very confusing name now.

And even if file is handled, this would be error prone since people will forget to deal with cases like data: or srcdoc (which live in child side).
https://hg.mozilla.org/mozilla-central/rev/4dea825d159b
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
(Assignee)

Comment 5

2 years ago
(In reply to Olli Pettay [:smaug] from comment #3)
> Comment on attachment 8863048 [details] [diff] [review]
> Rename ContentParent::TransmitPermissionsFor() to
> ContentParent::AboutToLoadDocumentForChild()
> 
> >+  // This function is called when we are about to load a document from an
> >+  // HTTP(S), FTP or wyciwyg channel for a content process.  It is a useful
> >+  // place to start to kick off work as early as possible in response to such
> >+  // document loads.
> >+  nsresult AboutToLoadDocumentForChild(nsIChannel* aChannel);
> 
> If this is really called for http(s), FTP or wyciwyg only, and not for
> example file channel, please call this something else.
> This is very confusing name now.

Hmm, what do you suggest as a better name?  AboutToLoadHttpFtpWyciwygDocumentForChild?  I can't think of anything better.
Flags: needinfo?(bugs)
I could live with that, even though it is quite long. But at least it doesn't hint that
all the document loads go through that method.
Flags: needinfo?(bugs)

Comment 7

2 years ago
Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f4ad40ab582e
follow-up: Rename ContentParent::AboutToLoadDocumentForChild to ContentParent::AboutToLoadHttpFtpWyciwygDocumentForChild
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.