IPDL Changes: Refcounted Types

[Nika Layzell]

Earlier this week, bug 1443954
 landed, which
improved support in IPDL for reference counted types, making it safer and
more ergonomic to directly use reference counted types (such as nsIURI,
nsIPrincipal, nsIInputStream, etc.) in IPDL protocols.

To declare a dependency on a refcounted type in IPDL, use the `using
refcounted` statement, for example:

PContent.ipdl:

using refcounted class nsIInputStream from "mozilla/ipc/IPCStreamUtils.h";

The type can then be referenced by their name. These types are passed by
raw pointer when used as an input, and passed by reference-to-RefPtr when
used as an output.

To implement ParamTraits (or IPDLParamTraits
)
for a refcounted type, specialize on the bare type:

PermissionMessageUtils.h:

template<>
struct ParamTraits
{
  static void Write(Message* aMsg, nsIPrincipal* aParam);
  static bool Read(const Message* aMsg, PickleIterator* aIter,
RefPtr* aResult);
};

These types may also be used within IPDL unions and IPDL structs. Please
prefer using these refcounted types directly over wrapper types like
`IPC::Principal` when possible.

- Nika
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Are there some lists of web sites which use vertical writing-mode?

[jsimmons]

There's a 100% vertical writing mode demo here: 
https://www.chenhuijing.com/zh-type/
(Blog post about it here: 
https://www.chenhuijing.com/blog/chinese-web-typography/#lets-build-a-demo)



On Monday, April 9, 2018 at 11:39:34 AM UTC-4, masayuki nakano wrote:
> Hi,
> 
> I'm currently reviewing bug 1358017 which tries to scroll vertical 
> writing mode contents horizontally with vertical mouse wheel operation 
> for some mice which have only vertical wheel and etc.
> https://bugzilla.mozilla.org/show_bug.cgi?id=1358017
> 
> I'd like to check actual behavior with actual devices on web sites which 
> use vertical writing-mode in real world before making it enabled in 
> release channel.
> 
> Does somebody know some lists of web sites which use vertical 
> writing-mode? Unfortunately, I don't know web sites which use 
> writing-mode heavily even though I usually read web sites written in 
> Japanese...
> 
> -- 
> Masayuki Nakano 
> Software Engineer, Mozilla

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Coding style: brace initialization syntax

[Botond Ballo]

On Fri, Apr 13, 2018 at 11:06 AM, Emilio Cobos Álvarez  wrote:
>> If we are going to have brace-initialization intermixed with
>> list-initialization (i.e. parentheses) in our codebase, I think we
>> should prefer no space prior to the brace, for consistency.
>
> Hmm, consistency with parenthesis I guess, but not with other things that
> use braces, like conditionals or other kind of declarations (where we use a
> space after the paren, for example), or lambdas where we use `mutable {`,
> etc.

Braces are used for a variety of purposes in C++. I think it makes
more sense to adapt the style to the purpose, than to try to enforce a
common style across all kinds of brace uses. We already do this; for
example, some uses of braces go on their own line, and others don't.

In this case, I think it makes sense for braces used for
initialization to be consistent with parentheses used for
initialization.

> Also, I guess per that argument we should use `mIsRootDefined{false}`
> instead of `mIsRootDefined{ false }`.

+1. `mIsRootDefined{false}` is what I've been using in constructor initializers.

Cheers,
Botond
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Coding style: brace initialization syntax

[Kartikaya Gupta]

On Fri, Apr 13, 2018 at 11:06 AM, Emilio Cobos Álvarez  wrote:
> I'd be ok with that I guess, though it's more common each time? Also, is
> there any case where you could use braces but not parenthesis? (I'm not a
> C++ expert in this regard).

I think there are. In particular if you're initializing a struct by
members which doesn't have a constructor. For instance, when I
recently upgraded layers id from a uint64_t to a struct, I had to use
{} in initializer lists instead of () because I didn't add a
constructor to the struct. Here's a simple example:

struct Foo {
int x;
};

struct Bar {
Bar()
: mFoo(0)   // <-- this fails, but {} will work
{
}

Foo mFoo;
};
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Coding style: brace initialization syntax

[Emilio Cobos Álvarez]

On 4/13/18 4:49 PM, Nathan Froyd wrote:

On Fri, Apr 13, 2018 at 9:37 AM, Emilio Cobos Álvarez  wrote:

Those changes I assume were generated with clang-format / clang-format-diff
using the "Mozilla" coding style, so I'd rather ask people to agree in
whether we prefer that style or other in order to change that if needed.

Would people agree to use:

  , mIsRootDefined { false }

Instead of:

  , mIsRootDefined{ false }

What's people's opinion on that? Would people be fine with a more general
"spaces around braces" rule? I can't think of a case right now where I
personally wouldn't prefer it.


If we are going to have brace-initialization intermixed with
list-initialization (i.e. parentheses) in our codebase, I think we
should prefer no space prior to the brace, for consistency. 


Hmm, consistency with parenthesis I guess, but not with other things 
that use braces, like conditionals or other kind of declarations (where 
we use a space after the paren, for example), or lambdas where we use 
`mutable {`, etc.


Also, I guess per that argument we should use `mIsRootDefined{false}` 
instead of `mIsRootDefined{ false }`.



If we are going to switch wholesale (which would be a big job!)... I'd
probably say "no space", just on "that's the way we've always done it"
grounds, but can be convinced otherwise.


Though this is a good point though, I've only found a couple of them 
with spaces:


  $ rg ' m[A-Z][A-Za-z]* \{ ' | wc -l 



  9

vs.

  $ rg ' m[A-Z][A-Za-z]*\{ ' | wc -l
  516

Though that could be an artifact of clang-format in the directories we 
run it.



I agree with bz on disallowing braces in constructor init lists.


I'd be ok with that I guess, though it's more common each time? Also, is 
there any case where you could use braces but not parenthesis? (I'm not 
a C++ expert in this regard).



Also, we should probably state that consistency is preferred (I assume we
generally agree on that), so in this case braces probably weren't even
needed, or everything should've switched to them.


Indeed.


Finally, while I'm here, regarding default member initialization, what's
preferred?

   uint32_t* mFoo = nullptr;

Or:

   uint32_t* mFoo { nullptr };


I lean towards the former here.  I think the former is more common in
the code I've seen, but apparently the latter is "preferred C++" or
something?


I think the former is ok, but wouldn't work for stuff like:

  Atomic mFoo = nullptr;

While:

Atomic mFoo { nullptr };

does work.

 -- Emilio
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Coding style: brace initialization syntax

[Gabriele Svelto]

On 13/04/2018 16:49, Nathan Froyd wrote:
> I lean towards the former here.  I think the former is more common in
> the code I've seen, but apparently the latter is "preferred C++" or
> something?

Yes, let's have a solid rationale if we're doing sweeping changes of
this sort. Blindly following the "C++ flavor du jour" is going to lead
to a lot of unnecessary churn.

 Gabriele
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Coding style: brace initialization syntax

[Nathan Froyd]

On Fri, Apr 13, 2018 at 9:37 AM, Emilio Cobos Álvarez  wrote:
> Those changes I assume were generated with clang-format / clang-format-diff
> using the "Mozilla" coding style, so I'd rather ask people to agree in
> whether we prefer that style or other in order to change that if needed.
>
> Would people agree to use:
>
>  , mIsRootDefined { false }
>
> Instead of:
>
>  , mIsRootDefined{ false }
>
> What's people's opinion on that? Would people be fine with a more general
> "spaces around braces" rule? I can't think of a case right now where I
> personally wouldn't prefer it.

If we are going to have brace-initialization intermixed with
list-initialization (i.e. parentheses) in our codebase, I think we
should prefer no space prior to the brace, for consistency.  If we are
going to switch wholesale (which would be a big job!)...I'd probably
say "no space", just on "that's the way we've always done it" grounds,
but can be convinced otherwise.

I agree with bz on disallowing braces in constructor init lists.

> Also, we should probably state that consistency is preferred (I assume we
> generally agree on that), so in this case braces probably weren't even
> needed, or everything should've switched to them.

Indeed.

> Finally, while I'm here, regarding default member initialization, what's
> preferred?
>
>   uint32_t* mFoo = nullptr;
>
> Or:
>
>   uint32_t* mFoo { nullptr };

I lean towards the former here.  I think the former is more common in
the code I've seen, but apparently the latter is "preferred C++" or
something?

-Nathan
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Coding style: brace initialization syntax

[Boris Zbarsky]

On 4/13/18 9:40 AM, Alex Gaynor wrote:

I don't have an opinion on the style change itself, but I'm a very strong
+1 on just picking one and making sure clang-format enforces it.


We need to fix clang-format to not produce output like 
https://hg.mozilla.org/mozilla-central/diff/d7d2f08e051c/dom/bindings/BindingDeclarations.h 
before we talk about enforcing this sort of thing with clang-format


-Boris
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Coding style: brace initialization syntax

[Boris Zbarsky]

On 4/13/18 9:37 AM, Emilio Cobos Álvarez wrote:

Would people agree to use:

  , mIsRootDefined { false }

Instead of:

  , mIsRootDefined{ false }


So my take is that we should not use braced initializer syntax in 
constructor initializer lists.  The reason for that is that it makes it 
much harder to scan for where the constructor body starts.  Doubly so 
when ifdefs in the initializer list are involved.  Triply so when 
someone writes it as:


  explicit TTextAttr()
, mIsRootDefined
  {
false
  }
#ifdef SOMETHING
#endif
  {
  }

which is what clang-format did in some of the cases in the patch for bug 
525063.


In particular, I think this should have just used:

  , mIsRootDefined(false)

I don't have a strong opinion about the one space when we do use the 
braced initializer syntax.  But we should make sure we don't end up with 
the above monstrosity where it looks like the ctor body.


-Boris
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Coding style: brace initialization syntax

[Alex Gaynor]

I don't have an opinion on the style change itself, but I'm a very strong
+1 on just picking one and making sure clang-format enforces it.

Alex

On Fri, Apr 13, 2018 at 9:37 AM, Emilio Cobos Álvarez 
wrote:

> Sorry, I know, coding style thread... But it's Friday and this is somewhat
> related to the previous thread.
>
> Bug 525063 added a lot of lines like:
>
> explicit TTextAttr(bool aGetRootValue)
>   : mGetRootValue(aGetRootValue)
>   , mIsDefined{ false }
>   , mIsRootDefined{ false }
> {
> }
>
> I think I find them hard to read and ugly.
>
> Those changes I assume were generated with clang-format /
> clang-format-diff using the "Mozilla" coding style, so I'd rather ask
> people to agree in whether we prefer that style or other in order to change
> that if needed.
>
> Would people agree to use:
>
>  , mIsRootDefined { false }
>
> Instead of:
>
>  , mIsRootDefined{ false }
>
> And:
>
>  , mFoo { }
>
> Instead of:
>
>  , mFoo{}
>
> ?
>
> I assume the same would be for variables, I find:
>
>   AutoRestore restore { mFoo };
>
> easier to read than:
>
>   AutoRestore restore{ mFoo };
>
> What's people's opinion on that? Would people be fine with a more general
> "spaces around braces" rule? I can't think of a case right now where I
> personally wouldn't prefer it.
>
> Also, we should probably state that consistency is preferred (I assume we
> generally agree on that), so in this case braces probably weren't even
> needed, or everything should've switched to them.
>
> Finally, while I'm here, regarding default member initialization, what's
> preferred?
>
>   uint32_t* mFoo = nullptr;
>
> Or:
>
>   uint32_t* mFoo { nullptr };
>
> I'm ambivalent, but brace syntax should cover more cases IIUC (that is,
> there are things that you can write with braces that you couldn't with
> equals I _think_).
>
> Should we state a preference? Or just say that both are allowed but
> consistency is better?
>
>  -- 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

Coding style: brace initialization syntax

[Emilio Cobos Álvarez]

Sorry, I know, coding style thread... But it's Friday and this is 
somewhat related to the previous thread.


Bug 525063 added a lot of lines like:

explicit TTextAttr(bool aGetRootValue)
  : mGetRootValue(aGetRootValue)
  , mIsDefined{ false }
  , mIsRootDefined{ false }
{
}

I think I find them hard to read and ugly.

Those changes I assume were generated with clang-format / 
clang-format-diff using the "Mozilla" coding style, so I'd rather ask 
people to agree in whether we prefer that style or other in order to 
change that if needed.


Would people agree to use:

 , mIsRootDefined { false }

Instead of:

 , mIsRootDefined{ false }

And:

 , mFoo { }

Instead of:

 , mFoo{}

?

I assume the same would be for variables, I find:

  AutoRestore restore { mFoo };

easier to read than:

  AutoRestore restore{ mFoo };

What's people's opinion on that? Would people be fine with a more 
general "spaces around braces" rule? I can't think of a case right now 
where I personally wouldn't prefer it.


Also, we should probably state that consistency is preferred (I assume 
we generally agree on that), so in this case braces probably weren't 
even needed, or everything should've switched to them.


Finally, while I'm here, regarding default member initialization, what's 
preferred?


  uint32_t* mFoo = nullptr;

Or:

  uint32_t* mFoo { nullptr };

I'm ambivalent, but brace syntax should cover more cases IIUC (that is, 
there are things that you can write with braces that you couldn't with 
equals I _think_).


Should we state a preference? Or just say that both are allowed but 
consistency is better?


 -- Emilio
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: redundant(?) code churn and code style issues in bug 525063

[Andi-Bogdan Postelnicu]

I come with a followup on this matter, we’ve backed out the issues. Now the 
tree should be OK.
Please see:
https://hg.mozilla.org/mozilla-central/rev/8a94faa5cc60495da5d80d4b3c07bf5877d2e6d8
 


Thanks,
ANdi

> On 13 Apr 2018, at 15:48, Nathan Froyd  wrote:
> 
> FWIW, all these complaints (and more) have been raised in the bug.
> I'm not entirely sure what we're going to do yet, but rest assured
> that people are definitely aware of the issues.
> 
> Thanks,
> -Nathan
> 
> On Fri, Apr 13, 2018 at 8:31 AM, Kartikaya Gupta  wrote:
>> On Fri, Apr 13, 2018 at 6:18 AM, Jonathan Kew  wrote:
>>> It's presumably auto-generated by a static-analysis tool or something like
>>> that, but ISTM it has been overly aggressive, adding a lot more code churn
>>> than necessary (as well as committing some pretty extreme style violations
>>> such as over-long lines).
>> 
>> +1. I too was sad to see the style violations/general ugliness of the
>> changes in the patch.
>> ___
>> 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



signature.asc
Description: Message signed with OpenPGP
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: redundant(?) code churn and code style issues in bug 525063

[Andi-Bogdan Postelnicu]

Hello all,

Right now we are working on backing out the changes the caused this. I will 
keep you posted when this is done.

P.S it should be done very shortly.

> On 13 Apr 2018, at 15:48, Nathan Froyd  wrote:
> 
> we're



signature.asc
Description: Message signed with OpenPGP
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: redundant(?) code churn and code style issues in bug 525063

[Nathan Froyd]

FWIW, all these complaints (and more) have been raised in the bug.
I'm not entirely sure what we're going to do yet, but rest assured
that people are definitely aware of the issues.

Thanks,
-Nathan

On Fri, Apr 13, 2018 at 8:31 AM, Kartikaya Gupta  wrote:
> On Fri, Apr 13, 2018 at 6:18 AM, Jonathan Kew  wrote:
>> It's presumably auto-generated by a static-analysis tool or something like
>> that, but ISTM it has been overly aggressive, adding a lot more code churn
>> than necessary (as well as committing some pretty extreme style violations
>> such as over-long lines).
>
> +1. I too was sad to see the style violations/general ugliness of the
> changes in the patch.
> ___
> 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: redundant(?) code churn and code style issues in bug 525063

[Kartikaya Gupta]

On Fri, Apr 13, 2018 at 6:18 AM, Jonathan Kew  wrote:
> It's presumably auto-generated by a static-analysis tool or something like
> that, but ISTM it has been overly aggressive, adding a lot more code churn
> than necessary (as well as committing some pretty extreme style violations
> such as over-long lines).

+1. I too was sad to see the style violations/general ugliness of the
changes in the patch.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

redundant(?) code churn and code style issues in bug 525063

[Jonathan Kew]

The huge patch that recently hit mozilla-central in bug 525063 makes me 
a bit sad. Did we really need to do this? On this scale?


It's presumably auto-generated by a static-analysis tool or something 
like that, but ISTM it has been overly aggressive, adding a lot more 
code churn than necessary (as well as committing some pretty extreme 
style violations such as over-long lines).


For example, it has added explicit initializers to constructors in cases 
where the members were already default-initialized at the point of 
declaration.


Couldn't we have been a little less heavy-handed here?

(I would have just commented in the bug, but don't have access to it)

JK
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform