1335368 - Remove a bunch of IsCallerChrome callers

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Just getting rid of this action-at-a-distance stuff, at least partly.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Stop using IsCallerChrome in DOMParser::Constructor

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Stop using IsCallerChrome in CheckerboardReportService::IsEnabled

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: part 3. Stop using IsCallerChrome in nsContentUtils::IsCutCopyAllowed()

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: part 4. Stop using IsCallerChrome in nsContentUtils::IsRequestFullScreenAllowed

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: part 5. Stop using IsCallerChrome in nsINode

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: part 6. Add a SystemCallerGuarantee enum that we use for [ChromeOnly, NeedsCallerType] cases

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: Byg part 7. Mostly stop using IsCallerChrome in nsObjectLoadingContent

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: part 8. Stop using IsCallerChrome in UnwrapArgImpl

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: part 9. Stop using IsCallerChrome in WebGL

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: part 10. Stop using IsCallerChrome in CanvasRenderingContext2D

Comment 11

[Boris Zbarsky [:bzbarsky]]

Attachment: part 11. Remove the use of ThreadsafeIsCallerChrome in Event::Init, since that's not really interested in the _caller_ anyway

Comment 12

[Boris Zbarsky [:bzbarsky]]

Attachment: part 12. Remove the use of IsCallerChrome in FetchRequest

Comment 13

[Boris Zbarsky [:bzbarsky]]

Attachment: part 13. Remove the use of IsCallerChrome in dom/file

Comment 14

[Boris Zbarsky [:bzbarsky]]

Attachment: part 14. Remove the use of IsCallerChrome in IDBFactory

Comment 15

[Boris Zbarsky [:bzbarsky]]

Attachment: part 15. Remove the use of IsCallerChrome in ApplyConstraints

Comment 16

[Boris Zbarsky [:bzbarsky]]

Attachment: part 16. Remove the use of IsCallerChrome in GetUserMedia

Comment 17

[Boris Zbarsky [:bzbarsky]]

Attachment: part 17. Get rid of IsCallerChrome in the quota manager service

Maybe I should add an xpidl annotation for passing SystemCallerGuarantee...

Comment 18

[Boris Zbarsky [:bzbarsky]]

Attachment: part 18. Get rid of IsCallerChrome in workers

Comment 19

[Boris Zbarsky [:bzbarsky]]

Attachment: part 19. Get rid of IsCallerChrome in XMLDocument::Load

Comment 20

[Boris Zbarsky [:bzbarsky]]

Attachment: part 20. Get rid of IsCallerChrome in XSLT

Comment 21

[Boris Zbarsky [:bzbarsky]]

Attachment: part 21. Get rid of some IsCallerChrome usage in XPConnect

Comment 22

[Boris Zbarsky [:bzbarsky]]

Attachment: part 22. Get rid of IsCallerChrome in geometry utils

Comment 23

[Andrea Marchesini [:baku]]

Comment on attachment 8832016 [details] [diff] [review]
part 13.  Remove the use of IsCallerChrome in dom/file

Review of attachment 8832016 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/file/File.cpp
@@ +613,4 @@
>    nsCOMPtr<nsPIDOMWindowInner> window = do_QueryInterface(aGlobal.GetAsSupports());
>  
>    RefPtr<MultipartBlobImpl> impl = new MultipartBlobImpl(EmptyString());
> +  impl->InitializeChromeFile(window, aData, aBag, aGuarantee, aRv);

Don't pass aGuarantee here. InitializeChromeFile is chrome only as any BlobImpl.

::: dom/file/File.h
@@ +267,5 @@
>    virtual int64_t GetLastModified(ErrorResult& aRv) = 0;
>  
>    virtual void SetLastModified(int64_t aLastModified) = 0;
>  
> +  virtual void GetMozFullPath(nsAString& aName, SystemCallerGuarantee,

aGuarantee

@@ +413,5 @@
>  
>    virtual void SetLastModified(int64_t aLastModified) override;
>  
> +  virtual void GetMozFullPath(nsAString& aName,
> +                              SystemCallerGuarantee,

aGuarantee

::: dom/file/MultipartBlobImpl.cpp
@@ +327,5 @@
>  MultipartBlobImpl::InitializeChromeFile(nsPIDOMWindowInner* aWindow,
>                                          nsIFile* aFile,
>                                          const ChromeFilePropertyBag& aBag,
>                                          bool aIsFromNsIFile,
> +                                        SystemCallerGuarantee,

Don't pass this argument.

@@ -335,5 @@
>      aRv.Throw(NS_ERROR_UNEXPECTED);
>      return;
>    }
>  
> -  MOZ_ASSERT(nsContentUtils::IsCallerChrome());

Can we keep this assertion and get rid of the SystemCallerGuarantee argument?

@@ +397,5 @@
>  void
>  MultipartBlobImpl::InitializeChromeFile(nsPIDOMWindowInner* aWindow,
>                                          const nsAString& aData,
>                                          const ChromeFilePropertyBag& aBag,
> +                                        SystemCallerGuarantee aGuarantee,

same here.

::: dom/file/MultipartBlobImpl.h
@@ +70,5 @@
>    void InitializeChromeFile(nsPIDOMWindowInner* aWindow,
>                              nsIFile* aData,
>                              const ChromeFilePropertyBag& aBag,
>                              bool aIsFromNsIFile,
> +                            SystemCallerGuarantee,

aGuarantee

Comment 24

[Boris Zbarsky [:bzbarsky]]

> Don't pass aGuarantee here. InitializeChromeFile is chrome only as any BlobImpl.

I'm not sure I follow.  The whole point is that having this argument guarantees we have a system caller, hence we can remove the IsCallerChrome() assert, since we have an argument that says caller is system.

> aGuarantee

Why, if it's not used?  I sort of like the pattern of just the bare "SystemCallerGuarantee" to mean "called as system".  I have it as aGuarantee in some places that need to propagate that state to someone else just so I can avoid typing out SystemCallerGuarantee::ThisCallerIsSystem...

I can add the unused aGuarantee if you really want, of course.

Comment 25

[Andrea Marchesini [:baku]]

> I can add the unused aGuarantee if you really want, of course.

I don't know if we have a guideline for this thing. unused seems better to me.

Comment 26

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8832005 [details] [diff] [review]
part 3.  Stop using IsCallerChrome in nsContentUtils::IsCutCopyAllowed()

The change to nsHTMLDocument::EditingStateChanged() is a bit ugly. Passing somewhat random principal. But fine.

Comment 27

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8832017 [details] [diff] [review]
part 14.  Remove the use of IsCallerChrome in IDBFactory

The mainthread checks shouldn't be removed.

It might be nice to have some way to pass SystemCallerGuarantee to other methods taking CallerType, but not about this patch.

Comment 28

[Daniel Holbert [:dholbert]]

Comment on attachment 8832026 [details] [diff] [review]
part 22.  Get rid of IsCallerChrome in geometry utils

Review of attachment 8832026 [details] [diff] [review]:
-----------------------------------------------------------------

r=me, though some of this probably wants to be in an earlier patch, as noted below:

::: dom/base/nsINode.h
@@ +285,5 @@
>    typedef mozilla::dom::DOMQuad DOMQuad;
>    typedef mozilla::dom::DOMRectReadOnly DOMRectReadOnly;
>    typedef mozilla::dom::OwningNodeOrString OwningNodeOrString;
>    typedef mozilla::dom::TextOrElementOrDocument TextOrElementOrDocument;
> +  typedef mozilla::dom::CallerType CallerType;

If you like, it'd probably be cleaner to fold this change (& the chunk directly below it) into part 5 -- the part with other nsINode changes.

@@ +1776,5 @@
>    // when accessed from chrome privileged script and
>    // from content privileged script for compatibility.
>    void GetBaseURIFromJS(nsAString& aBaseURI,
> +                        CallerType aCallerType,
> +                        ErrorResult& aRv) const;

(This is the other chunk to fold into part 5, if you like -- this is just dropping some namespaces in lines that you created/modified in part 5. This folding would reduce needless code-churn & make this patch a bit more targeted at actual GeometryUtils changes. Not a big deal, though.)

Comment 29

[Boris Zbarsky [:bzbarsky]]

Attachment: part 5. Stop using IsCallerChrome in nsINode

Comment 30

[Boris Zbarsky [:bzbarsky]]

Attachment: part 6. Add a SystemCallerGuarantee enum that we use for [ChromeOnly, NeedsCallerType] cases

Comment 31

[Boris Zbarsky [:bzbarsky]]

Attachment: Byg part 7. Mostly stop using IsCallerChrome in nsObjectLoadingContent

Comment 32

[Boris Zbarsky [:bzbarsky]]

Attachment: part 6. Add a SystemCallerGuarantee class that we use for [ChromeOnly, NeedsCallerType] cases

Comment 33

[Boris Zbarsky [:bzbarsky]]

> aGuarantee

Per IRC discussion, will do /* unused */.

> The mainthread checks shouldn't be removed.

Will fix.

> It might be nice to have some way to pass SystemCallerGuarantee to other methods taking CallerType

Done, by making SystemCallerGuarantee a class with "operator CallerType()" on it.

> If you like, it'd probably be cleaner to fold this change (& the chunk directly below it) into part 5

Good catch, done.

Comment 34

[Bobby Holley (:bholley)]

Comment on attachment 8832174 [details] [diff] [review]
part 6.  Add a SystemCallerGuarantee class that we use for [ChromeOnly, NeedsCallerType] cases

Review of attachment 8832174 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/nsGlobalWindow.h
@@ +1213,5 @@
>      if (mDoc) {
>        mDoc->WarnOnceAbout(nsIDocument::eWindow_Content);
>      }
> +    // We can use CallerType::System, because we have a SystemCallerGuarantee.
> +    GetContent(aCx, aRetval, mozilla::dom::CallerType::System, aError);

Can't you just pass aCallerType?

Comment 35

[Boris Zbarsky [:bzbarsky]]

> Can't you just pass aCallerType?

Yes.  Good catch; I thought I had updated that.

Comment 36

[Nathan Froyd [:froydnj]]

Comment on attachment 8832023 [details] [diff] [review]
part 20.  Get rid of IsCallerChrome in XSLT

Review of attachment 8832023 [details] [diff] [review]:
-----------------------------------------------------------------

r=me

::: dom/xslt/xslt/txMozillaXSLTProcessor.h
@@ +129,5 @@
>          aRv = RemoveParameter(aNamespaceURI, aLocalName);
>      }
>  
> +    uint32_t Flags(mozilla::dom::SystemCallerGuarantee);
> +    void SetFlags(uint32_t aFlags, mozilla::dom::SystemCallerGuarantee);

This is so much nicer than the old system.

Comment 37

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/25b13f152004
part 1.  Stop using IsCallerChrome in DOMParser::Constructor.  r=bholley
https://hg.mozilla.org/integration/mozilla-inbound/rev/ca5a8c8c6be5
part 2.  Stop using IsCallerChrome in CheckerboardReportService::IsEnabled.  r=bholley
https://hg.mozilla.org/integration/mozilla-inbound/rev/792c2c0560ba
part 3.  Stop using IsCallerChrome in nsContentUtils::IsCutCopyAllowed().  r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/2fab67512801
part 4.  Stop using IsCallerChrome in nsContentUtils::IsRequestFullScreenAllowed.  r=bholley
https://hg.mozilla.org/integration/mozilla-inbound/rev/01a24ce3398e
part 5.  Stop using IsCallerChrome in nsINode. r=bholley
https://hg.mozilla.org/integration/mozilla-inbound/rev/18e645747093
part 6.  Add a SystemCallerGuarantee class that we use for [ChromeOnly, NeedsCallerType] cases.  r=bholley
https://hg.mozilla.org/integration/mozilla-inbound/rev/897667c66ffc
part 7.  Mostly stop using IsCallerChrome in nsObjectLoadingContent.  r=bholley
https://hg.mozilla.org/integration/mozilla-inbound/rev/0f6327de6de6
part 8.  Stop using IsCallerChrome in UnwrapArgImpl.  r=bholley
https://hg.mozilla.org/integration/mozilla-inbound/rev/21f80b927366
part 9.  Stop using IsCallerChrome in WebGL.  r=bholley
https://hg.mozilla.org/integration/mozilla-inbound/rev/be91bfa95031
part 10.  Stop using IsCallerChrome in CanvasRenderingContext2D.  r=bholley
https://hg.mozilla.org/integration/mozilla-inbound/rev/1a9329505877
part 11.  Remove the use of ThreadsafeIsCallerChrome in Event::Init, since that's not really interested in the _caller_ anyway.  r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/d75c539b95ca
part 12.  Remove the use of IsCallerChrome in FetchRequest.  r=bkelly
https://hg.mozilla.org/integration/mozilla-inbound/rev/fb37a584eab3
part 13.  Remove the use of IsCallerChrome in dom/file.  r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/25880d675650
part 14.  Remove the use of IsCallerChrome in IDBFactory.  r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/06d85eb6ac5f
part 15.  Remove the use of IsCallerChrome in ApplyConstraints. r=jib
https://hg.mozilla.org/integration/mozilla-inbound/rev/c307d838a784
part 16.  Remove the use of IsCallerChrome in GetUserMedia.  r=jib
https://hg.mozilla.org/integration/mozilla-inbound/rev/0c0dcb4003a9
part 17.  Get rid of IsCallerChrome in the quota manager service.  r=bholley
https://hg.mozilla.org/integration/mozilla-inbound/rev/127a99a30553
part 18.  Get rid of IsCallerChrome in workers.  r=bholley
https://hg.mozilla.org/integration/mozilla-inbound/rev/65ceed819549
part 19.  Get rid of IsCallerChrome in XMLDocument::Load.  r=bholley
https://hg.mozilla.org/integration/mozilla-inbound/rev/31c167d2724d
part 20.  Get rid of IsCallerChrome in XSLT.  r=froydnj
https://hg.mozilla.org/integration/mozilla-inbound/rev/50aec64a8cdd
part 21.  Get rid of some IsCallerChrome usage in XPConnect. r=bholley
https://hg.mozilla.org/integration/mozilla-inbound/rev/0681970b9082
part 22.  Get rid of IsCallerChrome in geometry utils.  r=dholbert

Comment 38

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/25b13f152004
https://hg.mozilla.org/mozilla-central/rev/ca5a8c8c6be5
https://hg.mozilla.org/mozilla-central/rev/792c2c0560ba
https://hg.mozilla.org/mozilla-central/rev/2fab67512801
https://hg.mozilla.org/mozilla-central/rev/01a24ce3398e
https://hg.mozilla.org/mozilla-central/rev/18e645747093
https://hg.mozilla.org/mozilla-central/rev/897667c66ffc
https://hg.mozilla.org/mozilla-central/rev/0f6327de6de6
https://hg.mozilla.org/mozilla-central/rev/21f80b927366
https://hg.mozilla.org/mozilla-central/rev/be91bfa95031
https://hg.mozilla.org/mozilla-central/rev/1a9329505877
https://hg.mozilla.org/mozilla-central/rev/d75c539b95ca
https://hg.mozilla.org/mozilla-central/rev/fb37a584eab3
https://hg.mozilla.org/mozilla-central/rev/25880d675650
https://hg.mozilla.org/mozilla-central/rev/06d85eb6ac5f
https://hg.mozilla.org/mozilla-central/rev/c307d838a784
https://hg.mozilla.org/mozilla-central/rev/0c0dcb4003a9
https://hg.mozilla.org/mozilla-central/rev/127a99a30553
https://hg.mozilla.org/mozilla-central/rev/65ceed819549
https://hg.mozilla.org/mozilla-central/rev/31c167d2724d
https://hg.mozilla.org/mozilla-central/rev/50aec64a8cdd
https://hg.mozilla.org/mozilla-central/rev/0681970b9082