Closed
Bug 1335368
Opened 7 years ago
Closed 7 years ago
Remove a bunch of IsCallerChrome callers
Categories
(Core :: General, defect)
Core
General
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
(Blocks 1 open bug)
Details
Attachments
(22 files, 4 obsolete files)
Just getting rid of this action-at-a-distance stuff, at least partly.
Assignee | ||
Comment 1•7 years ago
|
||
Attachment #8832003 -
Flags: review?(bobbyholley)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•7 years ago
|
||
Attachment #8832004 -
Flags: review?(bobbyholley)
Assignee | ||
Comment 3•7 years ago
|
||
Attachment #8832005 -
Flags: review?(bugs)
Assignee | ||
Comment 4•7 years ago
|
||
Attachment #8832006 -
Flags: review?(bobbyholley)
Assignee | ||
Comment 5•7 years ago
|
||
Attachment #8832007 -
Flags: review?(bobbyholley)
Assignee | ||
Comment 6•7 years ago
|
||
Attachment #8832008 -
Flags: review?(bobbyholley)
Assignee | ||
Comment 7•7 years ago
|
||
Attachment #8832009 -
Flags: review?(bobbyholley)
Assignee | ||
Comment 8•7 years ago
|
||
Attachment #8832010 -
Flags: review?(bobbyholley)
Assignee | ||
Comment 9•7 years ago
|
||
Attachment #8832011 -
Flags: review?(bobbyholley)
Assignee | ||
Comment 10•7 years ago
|
||
Attachment #8832012 -
Flags: review?(bobbyholley)
Assignee | ||
Comment 11•7 years ago
|
||
Attachment #8832013 -
Flags: review?(bugs)
Assignee | ||
Comment 12•7 years ago
|
||
Attachment #8832015 -
Flags: review?(bkelly)
Assignee | ||
Comment 13•7 years ago
|
||
Attachment #8832016 -
Flags: review?(amarchesini)
Assignee | ||
Comment 14•7 years ago
|
||
Attachment #8832017 -
Flags: review?(bugs)
Assignee | ||
Comment 15•7 years ago
|
||
Attachment #8832018 -
Flags: review?(jib)
Assignee | ||
Comment 16•7 years ago
|
||
Attachment #8832019 -
Flags: review?(jib)
Assignee | ||
Comment 17•7 years ago
|
||
Maybe I should add an xpidl annotation for passing SystemCallerGuarantee...
Attachment #8832020 -
Flags: review?(bobbyholley)
Assignee | ||
Comment 18•7 years ago
|
||
Attachment #8832021 -
Flags: review?(bobbyholley)
Assignee | ||
Comment 19•7 years ago
|
||
Attachment #8832022 -
Flags: review?(bobbyholley)
Assignee | ||
Comment 20•7 years ago
|
||
Attachment #8832023 -
Flags: review?(peterv)
Assignee | ||
Comment 21•7 years ago
|
||
Attachment #8832024 -
Flags: review?(bobbyholley)
Assignee | ||
Comment 22•7 years ago
|
||
Attachment #8832026 -
Flags: review?(dholbert)
Comment 23•7 years ago
|
||
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
Attachment #8832016 -
Flags: review?(amarchesini) → review+
Assignee | ||
Comment 24•7 years ago
|
||
> 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.
Flags: needinfo?(amarchesini)
Comment 25•7 years ago
|
||
> 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.
Flags: needinfo?(amarchesini)
Comment 26•7 years ago
|
||
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.
Attachment #8832005 -
Flags: review?(bugs) → review+
Updated•7 years ago
|
Attachment #8832015 -
Flags: review?(bkelly) → review+
Comment 27•7 years ago
|
||
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.
Attachment #8832017 -
Flags: review?(bugs) → review-
Updated•7 years ago
|
Attachment #8832013 -
Flags: review?(bugs) → review+
Comment 28•7 years ago
|
||
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.)
Attachment #8832026 -
Flags: review?(dholbert) → review+
Assignee | ||
Comment 29•7 years ago
|
||
Attachment #8832171 -
Flags: review?(bobbyholley)
Assignee | ||
Updated•7 years ago
|
Attachment #8832007 -
Attachment is obsolete: true
Attachment #8832007 -
Flags: review?(bobbyholley)
Assignee | ||
Comment 30•7 years ago
|
||
Attachment #8832172 -
Flags: review?(bobbyholley)
Assignee | ||
Updated•7 years ago
|
Attachment #8832008 -
Attachment is obsolete: true
Attachment #8832008 -
Flags: review?(bobbyholley)
Assignee | ||
Comment 31•7 years ago
|
||
Attachment #8832173 -
Flags: review?(bobbyholley)
Assignee | ||
Updated•7 years ago
|
Attachment #8832009 -
Attachment is obsolete: true
Attachment #8832009 -
Flags: review?(bobbyholley)
Assignee | ||
Comment 32•7 years ago
|
||
Attachment #8832174 -
Flags: review?(bobbyholley)
Assignee | ||
Updated•7 years ago
|
Attachment #8832172 -
Attachment is obsolete: true
Attachment #8832172 -
Flags: review?(bobbyholley)
Assignee | ||
Comment 33•7 years ago
|
||
> 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.
Updated•7 years ago
|
Attachment #8832003 -
Flags: review?(bobbyholley) → review+
Updated•7 years ago
|
Attachment #8832004 -
Flags: review?(bobbyholley) → review+
Updated•7 years ago
|
Attachment #8832006 -
Flags: review?(bobbyholley) → review+
Updated•7 years ago
|
Attachment #8832011 -
Flags: review?(bobbyholley) → review+
Updated•7 years ago
|
Attachment #8832012 -
Flags: review?(bobbyholley) → review+
Updated•7 years ago
|
Attachment #8832020 -
Flags: review?(bobbyholley) → review+
Updated•7 years ago
|
Attachment #8832021 -
Flags: review?(bobbyholley) → review+
Updated•7 years ago
|
Attachment #8832022 -
Flags: review?(bobbyholley) → review+
Updated•7 years ago
|
Attachment #8832024 -
Flags: review?(bobbyholley) → review+
Updated•7 years ago
|
Attachment #8832171 -
Flags: review?(bobbyholley) → review+
Updated•7 years ago
|
Attachment #8832173 -
Flags: review?(bobbyholley) → review+
Comment 34•7 years ago
|
||
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?
Attachment #8832174 -
Flags: review?(bobbyholley) → review+
Updated•7 years ago
|
Attachment #8832010 -
Flags: review?(bobbyholley) → review+
Assignee | ||
Comment 35•7 years ago
|
||
> Can't you just pass aCallerType?
Yes. Good catch; I thought I had updated that.
Updated•7 years ago
|
Attachment #8832019 -
Flags: review?(jib) → review+
Updated•7 years ago
|
Attachment #8832018 -
Flags: review?(jib) → review+
Comment 36•7 years ago
|
||
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.
Attachment #8832023 -
Flags: review?(peterv) → review+
Comment 37•7 years ago
|
||
Pushed by bzbarsky@mozilla.com: 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•7 years ago
|
||
bugherder |
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
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in
before you can comment on or make changes to this bug.
Description
•