Re: Rust crate approval
On Thu, Jun 28, 2018 at 4:42 PM, Nathan Froyd wrote: > Thanks for raising these points. > Thanks for the response! On Tue, Jun 26, 2018 at 10:02 PM, Adam Gashlin wrote: > > * Already vendored crates > > Can I assume any crates we have already in mozilla-central are ok to use? > > Last year there was a thread that mentioned making a list of "sanctioned" > > crates, did that ever come about? > > I don't recall the discussion on sanctioned crates, do you have a > pointer to that thread? > It turns out what I was thinking of was just a brief suggestion here: https://groups.google.com/d/msg/mozilla.dev.platform/a_vhnvoM3co/yxwzqOkUBgAJ > Regardless, anything that's already vendored should be OK > That's generally been my assumption, but a number of things have been vendored that may not have been reviewed for possible inclusion in shipping code. I was wondering what winapi was doing in the tree (as it is essential for the Windows-specific stuff I'm doing), it seems to have been pulled in for geckodriver. I'll use judgment, as recommended :) -Adam ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Rust crate approval
Thanks for raising these points. On Tue, Jun 26, 2018 at 10:02 PM, Adam Gashlin wrote: > * Already vendored crates > Can I assume any crates we have already in mozilla-central are ok to use? > Last year there was a thread that mentioned making a list of "sanctioned" > crates, did that ever come about? I don't recall the discussion on sanctioned crates, do you have a pointer to that thread? Regardless, anything that's already vendored should be OK. > * Updates > I need winapi 0.3.5 for BITS support, currently third_party/rust/winapi is > 0.3.4. There should be no problem updating it, but should I have this > reviewed by the folks who originally vendored it into mozilla-central? While we can accommodate multiple versions of crates in-tree, we would prefer that only one version of a given crate is vendored into the tree at any one time, but sometimes this is an impractical goal to achieve. So if upgrading whatever uses winapi 0.3.4 to use 0.3.5 instead is reasonable, please do that first. If it turns out to be impractical, go ahead and vendor the duplicate crate. For review concerns, see below. > * New crates > I'd like to use the windows-service crate, which seems well written and has > few dependencies, but the first 0.1.0 release was just a few weeks ago. I'd > like to have that reviewed at least as carefully as my own code, > particularly given how much unsafety there is, but where do I draw the > line? For instance, it depends on "widestring", which is small and has been > around for a while but isn't widely used, should I have that reviewed > internally as well? Is popularity a reasonable measure? Our normal review process is all that we have used so far; I think thus far we have assumed that Rust's safety guarantees enable us to forego a more stringent review process that has sometimes been used for (some) C/C++ code. (e.g. I think modules/brotli underwent some amount of scrutiny, whereas mfbt/double-conversion was a more rubber-stamp sort of import.) This is probably not a tenable long-term position, especially given how easy it is to pull in Rust code vs. a C/C++ library. We have generally trusted people to use good judgement in what they use and how much review is required. Accordingly, I think you should request review from the people who would normally review your code, and if you have concerns about specific crates that are being vendored, you should call those crates out as needing especial review. If you or your reviewers think such reviews fall outside of your comfort zone/area of expertise/Rust capabilities, please flag myself or Ehsan, and we will work on finding people to help. Thanks, -Nathan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Coding style: Making the `e` prefix for enum variants not mandatory?
On 6/29/18 12:15 AM, Mike Hommey wrote: On Thu, Jun 28, 2018 at 05:27:23PM +0200, Emilio Cobos Álvarez wrote: I asked kats@ (which added the list item regarding enums) and he was fine removing it from the coding style, so given that and the rest of the thread I've edited the page accordingly. Not that I'd oppose, but I'll note that neither kats, nor participants to this thread so far are peers of the "C++/Rust usage, tools, and style" module. Oh, I didn't realize that those peers were the only ones to be able to update the style guide, sorry. I guess it makes sense. I can revert the change if needed and try to get sign-off from some of those. Apologies again, I just followed the procedure that was followed in the previous thread to add the rule. Let me know if you want that change reverted and I'll happily do so. -- Emilio Mike ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Coding style: Making the `e` prefix for enum variants not mandatory?
On Thu, Jun 28, 2018 at 05:27:23PM +0200, Emilio Cobos Álvarez wrote: > I asked kats@ (which added the list item regarding enums) and he was fine > removing it from the coding style, so given that and the rest of the thread > I've edited the page accordingly. Not that I'd oppose, but I'll note that neither kats, nor participants to this thread so far are peers of the "C++/Rust usage, tools, and style" module. Mike ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Intent to unprefix grid-gap, grid-row-gap, and grid-column-gap and updating them to spec
On Thu, Mar 29, 2018 at 8:19 AM, Mats Palmgren wrote: > Hi, > > In bug 1398482 I'm unprefixing the grid-gap, grid-row-gap, and > grid-column-gap properties > [...] > > These properties also applies to Flexbox: > https://drafts.csswg.org/css-align-3/#gap-flex > We haven't implemented layout for that yet > Followup: Mihir Iyer has now landed a patch to make these properties work in flexbox layout, over in https://bugzilla.mozilla.org/show_bug.cgi?id=1398483 This functionality is in today's Nightly build. (See testcase on that bug for an example.) ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Coding style: Making the `e` prefix for enum variants not mandatory?
I asked kats@ (which added the list item regarding enums) and he was
fine removing it from the coding style, so given that and the rest of
the thread I've edited the page accordingly.
Cheers,
-- Emilio
On 6/26/18 2:46 AM, Jeff Gilbert wrote:
If we can't agree, it shouldn't be in the style guide.
On Mon, Jun 25, 2018 at 2:17 PM, Peter Saint-Andre wrote:
And perhaps good reason for removing it from the style guide? ;-)
On 6/25/18 3:08 PM, Emilio Cobos Álvarez wrote:
And Kris pointed out that we already had another huge thread on this:
https://groups.google.com/d/msg/mozilla.dev.platform/WAuySoTfq_w/-DggRotpBQAJ
Looks like there wasn't agreement on that one... But oh well, don't want
to repeat a lot of that discussion.
I think the argument for consistency with the other systems language we
have in-tree, the fact that it's not predominant (at least for enum
classes) even though it is in the coding style, and that there wasn't
agreement in the previous thread are good reasons for not enforcing it,
but...
-- Emilio
On 6/25/18 10:41 PM, Emilio Cobos Álvarez wrote:
Our coding style states that we should use an `e` prefix for enum
variants, that is:
enum class Foo { eBar, eBaz };
We're not really consistent about it: looking at layout/, we mostly
use CamelCase, though we do have some prefixed enums. Looking at other
modules, enum classes almost never use it either. DOM bindings also
don't use that prefix.
I think that with enum classes the usefulness of the prefix is less
justified. Plus removing them would allow us to match the Rust coding
style as well, which is nice IMO.
Would anybody object to making the prefix non-mandatory, removing that
line from the coding style doc? Maybe only making it non-mandatory for
enum classes?
Thanks,
-- Emilio
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: CPOWs are now almost completely disabled
This is great news! Thanks to everybody involved! On Thu, 28 Jun 2018 at 09:16, Alex Gaynor wrote: > Outstanding! I love a good IPC attack surface reduction! > > Alex > > On Wed, Jun 27, 2018 at 6:54 PM Tom Schuster wrote: > > > Since landing bug 1465911 [1], CPOWs [2] are only functional on our > testing > > infrastructure. In normal builds that we ship to users CPOWs can be > > created, but no operations like property lookup can be performed on them. > > > > CPOWs continue to exist, because a lot of tests still depend on them. We > > can't disable CPOW creation in user builds, because the context menu > passes > > them from the child to the parent and back like a token. > > > > This is a significant IPC attack surface reduction. > > > > [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1465911 > > [2] > > > > > https://developer.mozilla.org/en-US/Firefox/Multiprocess_Firefox/Cross_Process_Object_Wrappers > > ___ > > dev-platform mailing list > > dev-platform@lists.mozilla.org > > https://lists.mozilla.org/listinfo/dev-platform > > > ___ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform > ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: CPOWs are now almost completely disabled
Outstanding! I love a good IPC attack surface reduction! Alex On Wed, Jun 27, 2018 at 6:54 PM Tom Schuster wrote: > Since landing bug 1465911 [1], CPOWs [2] are only functional on our testing > infrastructure. In normal builds that we ship to users CPOWs can be > created, but no operations like property lookup can be performed on them. > > CPOWs continue to exist, because a lot of tests still depend on them. We > can't disable CPOW creation in user builds, because the context menu passes > them from the child to the parent and back like a token. > > This is a significant IPC attack surface reduction. > > [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1465911 > [2] > > https://developer.mozilla.org/en-US/Firefox/Multiprocess_Firefox/Cross_Process_Object_Wrappers > ___ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform > ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Intent to deprecate and remove: offset-* logical properties.
In bug 1464782 I renamed these to inset-*, and left the offset-* properties as an alias behind a (enabled-by-default) pref: layout.css.offset-logical-properties.enabled I want it to turn it off by default sooner rather than later, given no other engine supports it, but keep the pref around for at least a release, following David's advice in [1]. I just filed bug 1471838 for that. There's (somewhat unlikely) chance of compat fallout if some page relies on these, but my understanding is that this is somewhat unlikely given no other engine supports them yet, and we're at the beginning of a cycle so toggling the pref back on should be trivial should we need to do so. -- Emilio [1]: https://bugzilla.mozilla.org/show_bug.cgi?id=1464782#c9 ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Intent to ship: inset-* logical properties.
Hi, In bug 1464782 I renamed the long-shipped offset-* logical properties (offset-inline-start, offset-inline-end, offset-block-start, offset-block-end), which map to top / bottom / left / right, to their standard name (inset-*), and turned the later into aliases. No other browser implements this so far (nor the offset-*), but Blink is working on logical properties support and presumably they'll only support the inset-* ones. There's WPT tests for this already, but they fail on Firefox because of bug 1116638, which is on my radar. Let me know if there's any concern with such thing. As part of it I plan to deprecate and remove the offset-* properties, but I'll send a separate intent for that in a few :) -- Emilio ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Intent to disable 'backlog' tracking flag in Bugzilla
The 'backlog' (originally known as 'blocking-loop') tracking flag, is available for bugs in Core::Audio/Video* and Core::WebRTC*. The flag is no longer used by the groups triaging and working on bugs in those components, and per a discussion with :drno (Nils Ohlmeier) I intend to disable setting this flag in new bugs. Disabling this flag *will not* affect the value of it on bugs which have it set. It will still show up and you can search on values of the flag. If there are no objections to this, I'll disable the flag on Monday, July 2nd. The bug for this task is https://bugzilla.mozilla.org/show_bug.cgi?id=1471819. Thanks, Emma ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
