Closed Bug 1360723 Opened 3 years ago Closed 3 years ago

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

Categories

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

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: ehsan, Assigned: ehsan)

References

Details

Attachments

(1 file)

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.
Blocks: 1350637
Assignee: nobody → ehsan
Attachment #8863048 - Flags: review?(michael) → review+
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
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
(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)
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
You need to log in before you can comment on or make changes to this bug.