Re: PSA: Major ChromeUtils.import() API change

2019-01-28 Thread Jan de Mooij
On Tue, Jan 29, 2019 at 5:51 AM Kris Maglione  wrote:

> To begin with, the weirdness of our JSM scopes makes a fair amount of our
> system code un-JITtable, which should be enough on its own.


In case people are wondering: un-JITtable applies to Ion, the optimizing
JIT. The Baseline JIT is able to compile all chrome code. That said,
Baseline is also affected by the performance issues here because our inline
caches don't optimize the 'special' environment objects that are currently
used for JSMs, so accessing global variables is much slower than in content
code.

Many thanks to Kris and others for working on this. Making our chrome and
content environments more similar means chrome code will benefit from all
the optimization work we do for content code and ES6 modules.

Jan


> -Kris
> ___
> firefox-dev mailing list
> firefox-...@mozilla.org
> https://mail.mozilla.org/listinfo/firefox-dev
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: PSA: Major ChromeUtils.import() API change

2019-01-28 Thread Kris Maglione

On Mon, Jan 28, 2019 at 04:50:28PM -0800, Kris Maglione wrote:
Whatever else this whole process accomplishes, it will have the major 
side-effect of making our system JS code much more JIT-friendly, and 
in many cases, some orders of magnitude faster.


Since there's been some question about my "orders of magnitude" comment, I 
may as well explain:


To begin with, the weirdness of our JSM scopes makes a fair amount of our 
system code un-JITtable, which should be enough on its own. But even for 
code that we can JIT (which is still a lot of it), there's another problem: 
In order to make code fast, the JIT needs to be certain that it knows the 
location and type of every variable that it touches, and that those values 
can't change out-from-under it.


For code that deals with global variables and has weird scope chains, 
though, that's not really possible. There are scopes in-between the 
executing scope and the global scope that the JIT doesn't really understand, 
and can't really trust. So, in an ideal world, the following, for instance:


 for (let i = 0; i < typedArrayA.length; i++) {
   typedArrayC[i] = Math.floor(typedArrayA[i] / typedArrayB[i]);
 }

The Math.floor call can be inlined into pure assembly operations that take a 
few nanoseconds. The whole loop becomes pretty similar to the equivalent 
construct in C.


If we're in a JSM environment, though, the JIT can't trust the value of 
`Math` to stay the same from one iteration to the other. It has to walk the 
scope chain for every iteration, and do a hash lookup for the name `Math` in 
each scope, until it finds a match. Hash lookups are expensive. The same 
operation which took nanoseconds in an ideal world takes microseconds in our 
current JSM world. That's three orders of magnitude slower before we even 
take into account the memory we touch that's now taking up valuable L1 cache 
or register space, or the likelihood that we have to do a real JS function 
call rather than do some simple inline arithmetic.



This is probably more than most of you want to know, and less than the rest 
of you want to know, but it's been a long week for it being only a Monday 
night, so there you are.


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


Fwd: Intent to Desupport: Fennec Automated Updates (when Sideload Installed)

2019-01-28 Thread Justin Wood
CC'ing dev-platform, since dev-planning got blackholed with my message.

-- Forwarded message -
From: Justin Wood 
Date: Mon, Jan 28, 2019 at 3:36 PM
Subject: Intent to Desupport: Fennec Automated Updates (when Sideload
Installed)
To: planning , firefox-ci <
firefox...@mozilla.com>, release-engineering <
release-engineer...@lists.mozilla.org>, release ,
release-drivers 


Hello,

I am intending to stop offering Fennec updates when Fennec itself is
installed manually instead of via the Google Play store.

More details are at https://github.com/mozilla-releng/releng-rfcs/pull/13
or in rendered form at
https://github.com/Callek/releng-rfcs/blob/no_fennec_balrog/rfcs/0013-disable-fennec-balrog.md

Please contain (most) comments to the markdown on github.

I intend to implement this on autoland/central as of Feb 5, 2019 and allow
it to ride the trains to release.

Thank You,
~Justin Wood (Callek)
Release Engineer
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


PSA: Major ChromeUtils.import() API change

2019-01-28 Thread Kris Maglione
As of bug 1514594, the behavior ChromeUtils.import() API has changed in 
order to avoid touching the caller's global scope, or returning the imported 
module's global scope. In short, where you previously wrote:


 ChromeUtils.import("resource://gre/modules/Services.jsm");

You should now write something like:

 const {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");

We've made this change for a lot of reasons. The obvious ones include making 
it obvious to readers exactly what symbols are imported, and making it 
easier for tooling to analyze such imports.


The less obvious, and arguably more important reason, though, is that we 
would like to start loading JSMs as ES6 modules. The major stumbling block 
to that effort is that our JSM import and export APIs rely on modules being 
loaded into global-like objects that we can return, and our caller APIs 
likewise rely on callers being loaded into objects where we can define 
properties. Neither of these assumptions are compatible with ES6 module 
scripts, which have lexical scopes, but no global or pseudo-global objects 
of their own.


Changing the ChromeUtils.import() API to stop relying on these assumptions 
is the first step down this road. In the near future, we're going to need to 
make similar changes to our lazy getter and lazy import APIs to stop 
defining properties on the `this` object (which will be going away), and to 
a large number of unit tests which rely on mangling module globals.


Once all of this is done, we'll change our JSM loading infrastructure to use 
the `export` keyword rather than the `EXPORTED_SYMBOLS` array, and rewrite 
our existing code to comply with the new behavior. Whatever else this whole 
process accomplishes, it will have the major side-effect of making our 
system JS code much more JIT-friendly, and in many cases, some orders of 
magnitude faster.


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


Re: C++ method definition comments

2019-01-28 Thread gsquelart
Just a thought: Would it be worth considering a blank macro, e.g.:
static void foo();
DECLARED_STATIC void foo() {...}

On top of not being confused with other comments around, it could be 
clang-checked so it's never wrong. (And maybe eventually enforced, like 
MOZ_IMPLICIT is.)

Cheers,
Gerald

On Tuesday, January 29, 2019 at 10:27:17 AM UTC+11, Ryan Hunt wrote:
> Yeah, personally I have found them be useful and don't have an issue with 
> keeping
> them. I just wasn't sure if that was a common experience.
> 
> So for converting from C-style to C++-style, that would be:
> 
> /* static */ void Foo::Bar() {
>  ...
> }
> 
> // static
> void Foo::Bar() {
>  ...
> }
> 
> I think that would be good. My one concern would be the presence of other 
> C++-style
> comments before the method definition. For example [1].
> 
> Ideally documentation like that should go in the header by the method 
> declaration, but I
> have no idea if we consistently do that.
> 
> [1] 
> https://hg.mozilla.org/mozilla-central/file/e4b9b1084292/layout/generic/nsFrame.cpp#l1023
> 
> Thanks,
> Ryan
> 
> ‐‐‐ Original Message ‐‐‐
> On Monday, January 28, 2019 12:51 PM, Ehsan Akhgari  
> wrote:
> 
> > This is indeed one of the cases where the reformat has made things worse.  
> > I think as a couple of people have already said, we'll find that some 
> > people do find these annotations useful, even if they're not always 
> > consistently present.
> >
> > The path to least resistance for addressing this problem may be to convert 
> > these into C++-style comments and therefore moving them into their own 
> > lines.  Would you be OK with that?
> >
> > Thanks,
> > Ehsan
> >
> > On Fri, Jan 25, 2019 at 11:49 PM Ryan Hunt  wrote:
> >
> >> Hi all,
> >>
> >> Quick C++ style question.
> >>
> >> A common pattern in Gecko is for method definitions to have a comment with 
> >> the
> >> 'static' or 'virtual' qualification.
> >>
> >> Before the reformat, the comment would be on it's own separate line [1]. 
> >> Now
> >> it's on the main line of the definition [2].
> >>
> >> For example:
> >>
> >> /* static */ void
> >> Foo::Bar() {
> >>   ...
> >> }
> >>
> >> vs.
> >>
> >> /* static */ void Foo::Bar() {
> >>   ...
> >> }
> >>
> >> Personally I think this now takes too much horizontal space from the main
> >> definition, and would prefer it to be either on its own line or just 
> >> removed.
> >>
> >> Does anyone have an opinion on whether we still want these comments? And 
> >> if so
> >> whether it makes sense to move them back to their own line.
> >>
> >> (My ulterior motive is that sublime text's indexer started failing to find
> >>  these definitions after the reformat, but that should be fixed regardless)
> >>
> >> If you're interested in what removing these would entail, I wrote a regex 
> >> to
> >> make the change [3].
> >>
> >> Thanks,
> >> Ryan
> >>
> >> [1] 
> >> https://hg.mozilla.org/mozilla-central/file/0348d472115d/layout/generic/nsFrame.cpp#l1759
> >> [2] 
> >> https://hg.mozilla.org/mozilla-central/file/e4b9b1084292/layout/generic/nsFrame.cpp#l1756
> >> [3] https://hg.mozilla.org/try/rev/31ab3e466b6f15dcdbb1aee47edabc7c358b86f2
> >>
> >
> > --
> > Ehsan
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Heads up: Fluent will now block layout

2019-01-28 Thread zbraniecki
Hi all,

We've just landed a pretty important change to how we localize our UI [0].

Starting from this week (including 66), Fluent (async and sync) will now block 
layout in all three of XUL/XHTML and HTML.

That means that flashes of untranslated content (FOUC) should not be possible 
anymore.

Please, help us test the new behavior and alert me or smaug if you see any 
other impact of the change.

Thanks,
zb.

[0] https://bugzilla.mozilla.org/show_bug.cgi?id=1518252
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: C++ method definition comments

2019-01-28 Thread Ryan Hunt
Yeah, personally I have found them be useful and don't have an issue with 
keeping
them. I just wasn't sure if that was a common experience.

So for converting from C-style to C++-style, that would be:

/* static */ void Foo::Bar() {
 ...
}

// static
void Foo::Bar() {
 ...
}

I think that would be good. My one concern would be the presence of other 
C++-style
comments before the method definition. For example [1].

Ideally documentation like that should go in the header by the method 
declaration, but I
have no idea if we consistently do that.

[1] 
https://hg.mozilla.org/mozilla-central/file/e4b9b1084292/layout/generic/nsFrame.cpp#l1023

Thanks,
Ryan

‐‐‐ Original Message ‐‐‐
On Monday, January 28, 2019 12:51 PM, Ehsan Akhgari  
wrote:

> This is indeed one of the cases where the reformat has made things worse.  I 
> think as a couple of people have already said, we'll find that some people do 
> find these annotations useful, even if they're not always consistently 
> present.
>
> The path to least resistance for addressing this problem may be to convert 
> these into C++-style comments and therefore moving them into their own lines. 
>  Would you be OK with that?
>
> Thanks,
> Ehsan
>
> On Fri, Jan 25, 2019 at 11:49 PM Ryan Hunt  wrote:
>
>> Hi all,
>>
>> Quick C++ style question.
>>
>> A common pattern in Gecko is for method definitions to have a comment with 
>> the
>> 'static' or 'virtual' qualification.
>>
>> Before the reformat, the comment would be on it's own separate line [1]. Now
>> it's on the main line of the definition [2].
>>
>> For example:
>>
>> /* static */ void
>> Foo::Bar() {
>>   ...
>> }
>>
>> vs.
>>
>> /* static */ void Foo::Bar() {
>>   ...
>> }
>>
>> Personally I think this now takes too much horizontal space from the main
>> definition, and would prefer it to be either on its own line or just removed.
>>
>> Does anyone have an opinion on whether we still want these comments? And if 
>> so
>> whether it makes sense to move them back to their own line.
>>
>> (My ulterior motive is that sublime text's indexer started failing to find
>>  these definitions after the reformat, but that should be fixed regardless)
>>
>> If you're interested in what removing these would entail, I wrote a regex to
>> make the change [3].
>>
>> Thanks,
>> Ryan
>>
>> [1] 
>> https://hg.mozilla.org/mozilla-central/file/0348d472115d/layout/generic/nsFrame.cpp#l1759
>> [2] 
>> https://hg.mozilla.org/mozilla-central/file/e4b9b1084292/layout/generic/nsFrame.cpp#l1756
>> [3] https://hg.mozilla.org/try/rev/31ab3e466b6f15dcdbb1aee47edabc7c358b86f2
>>
>> ___
>> dev-platform mailing list
>> dev-platform@lists.mozilla.org
>> https://lists.mozilla.org/listinfo/dev-platform
>
> --
> Ehsan
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


PSA: 64-bits Windows build are now the default on 64-bits Windows

2019-01-28 Thread Mike Hommey
Hi,

As of bug 1522354, now on autoland, hopefully merged in a few hours,
the default build you get on a 64-bits Windows machine will be 64-bits,
instead of 32-bits like it had been forever.

If you do wish to do a 32-bits build on a 64-bits Windows machine, you
can add:

```
ac_add_options --target=i686
```

to your mozconfig, and that will do it.

Cheers,

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


Re: C++ method definition comments

2019-01-28 Thread Jeff Gilbert
I would much rather revert to:
/*static*/ void
Foo::Bar()

The Foo::Bar is the most relevant part of that whole expression, which
makes it nice to keep up against the start of the line.

In a clang-format world, we should feel more free to make such
deviations from Google Style, since it's all handled for us.

On Mon, Jan 28, 2019 at 10:52 AM Ehsan Akhgari  wrote:
>
> This is indeed one of the cases where the reformat has made things worse.
> I think as a couple of people have already said, we'll find that some
> people do find these annotations useful, even if they're not always
> consistently present.
>
> The path to least resistance for addressing this problem may be to convert
> these into C++-style comments and therefore moving them into their own
> lines.  Would you be OK with that?
>
> Thanks,
> Ehsan
>
> On Fri, Jan 25, 2019 at 11:49 PM Ryan Hunt  wrote:
>
> > Hi all,
> >
> > Quick C++ style question.
> >
> > A common pattern in Gecko is for method definitions to have a comment with
> > the
> > 'static' or 'virtual' qualification.
> >
> > Before the reformat, the comment would be on it's own separate line [1].
> > Now
> > it's on the main line of the definition [2].
> >
> > For example:
> >
> > /* static */ void
> > Foo::Bar() {
> >   ...
> > }
> >
> > vs.
> >
> > /* static */ void Foo::Bar() {
> >   ...
> > }
> >
> > Personally I think this now takes too much horizontal space from the main
> > definition, and would prefer it to be either on its own line or just
> > removed.
> >
> > Does anyone have an opinion on whether we still want these comments? And
> > if so
> > whether it makes sense to move them back to their own line.
> >
> > (My ulterior motive is that sublime text's indexer started failing to find
> >  these definitions after the reformat, but that should be fixed regardless)
> >
> > If you're interested in what removing these would entail, I wrote a regex
> > to
> > make the change [3].
> >
> > Thanks,
> > Ryan
> >
> > [1]
> > https://hg.mozilla.org/mozilla-central/file/0348d472115d/layout/generic/nsFrame.cpp#l1759
> > [2]
> > https://hg.mozilla.org/mozilla-central/file/e4b9b1084292/layout/generic/nsFrame.cpp#l1756
> > [3]
> > https://hg.mozilla.org/try/rev/31ab3e466b6f15dcdbb1aee47edabc7c358b86f2
> >
> > ___
> > dev-platform mailing list
> > dev-platform@lists.mozilla.org
> > https://lists.mozilla.org/listinfo/dev-platform
> >
>
>
> --
> Ehsan
> ___
> 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: Intent to ship: implicit rel=noopener for target=_blank on anchor and area elements

2019-01-28 Thread Ehsan Akhgari
On Thu, Jan 24, 2019 at 6:52 AM Andrea Marchesini 
wrote:

> I intend to turn "implicit ref=noopener for anchor and area elements for
> target=_blank" on by default in 67. It has been developed behind the
> "dom.targetBlankNoOpener.enabled" preference and enabled in nightly for ~2
> cycles (it landed in FF65).
>
> Safari has already shipped this feature:
>
> https://webkit.org/blog/8475/release-notes-for-safari-technology-preview-68/
>

Just to be clear, this hasn't shipped in a release version of Safari yet,
it will probably be part of the next release of Safari.


> Bug to turn on by default:
> https://bugzilla.mozilla.org/show_bug.cgi?id=1522083
>
> This feature was previously discussed in this "intent to implement"
> thread:
>
> https://groups.google.com/forum/#!searchin/mozilla.dev.platform/intent$20to$20implement$20rel$3D%7Csort:date/mozilla.dev.platform/d4R7WIHSOMY/iHBAH0koCgAJ
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>


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


Re: Does mozilla allow modification of Strings

2019-01-28 Thread Kris Maglione

On Mon, Jan 28, 2019 at 04:51:12AM -0800, edusubscr...@gmail.com wrote:

Is it possible to add an  extra variable to mozilla string(nsTStringRepr).


I added a bool variable to nsTStringRepr class in  Xpcom/Strings/


As something of a side note, the general way to do this is to 
add a new data or class flag, and store it in one of the 
existing fields:


https://searchfox.org/mozilla-central/rev/5c8ea961d04767db723a0a15e3a8f7fbca154129/xpcom/string/nsTStringRepr.h#300-301

Changing the size or layout of the string class is more or less 
guaranteed to cause problems. Aside from affecting the size and 
padding of thousands of classes, the strings API is some of the 
oldest code in Gecko, and there is almost certainly some subtle 
code which makes assumptions about its layout, and will break if 
it changes. The Rust bindings are one such example (as you saw), 
but there are almost certainly others as well.

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


Re: C++ method definition comments

2019-01-28 Thread Ehsan Akhgari
This is indeed one of the cases where the reformat has made things worse.
I think as a couple of people have already said, we'll find that some
people do find these annotations useful, even if they're not always
consistently present.

The path to least resistance for addressing this problem may be to convert
these into C++-style comments and therefore moving them into their own
lines.  Would you be OK with that?

Thanks,
Ehsan

On Fri, Jan 25, 2019 at 11:49 PM Ryan Hunt  wrote:

> Hi all,
>
> Quick C++ style question.
>
> A common pattern in Gecko is for method definitions to have a comment with
> the
> 'static' or 'virtual' qualification.
>
> Before the reformat, the comment would be on it's own separate line [1].
> Now
> it's on the main line of the definition [2].
>
> For example:
>
> /* static */ void
> Foo::Bar() {
>   ...
> }
>
> vs.
>
> /* static */ void Foo::Bar() {
>   ...
> }
>
> Personally I think this now takes too much horizontal space from the main
> definition, and would prefer it to be either on its own line or just
> removed.
>
> Does anyone have an opinion on whether we still want these comments? And
> if so
> whether it makes sense to move them back to their own line.
>
> (My ulterior motive is that sublime text's indexer started failing to find
>  these definitions after the reformat, but that should be fixed regardless)
>
> If you're interested in what removing these would entail, I wrote a regex
> to
> make the change [3].
>
> Thanks,
> Ryan
>
> [1]
> https://hg.mozilla.org/mozilla-central/file/0348d472115d/layout/generic/nsFrame.cpp#l1759
> [2]
> https://hg.mozilla.org/mozilla-central/file/e4b9b1084292/layout/generic/nsFrame.cpp#l1756
> [3]
> https://hg.mozilla.org/try/rev/31ab3e466b6f15dcdbb1aee47edabc7c358b86f2
>
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>


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


Re: Does mozilla allow modification of Strings

2019-01-28 Thread Kris Maglione

On Mon, Jan 28, 2019 at 02:05:18PM +0100, Emilio Cobos Álvarez wrote:

There are probably two different issues.

On 1/28/19 1:51 PM, edusubscr...@gmail.com wrote:



Is it possible to add an  extra variable to mozilla string(nsTStringRepr).


I added a bool variable to nsTStringRepr class in  Xpcom/Strings/

While building the build  I got static Assertions like  below

 /User/Desktop/MOZB/mozilla-central/xpcom/base/nsCycleCollector.cpp:1954:3: error: 
static_assert failed "Don't create too large CCGraphBuilder objects"
 0:34.99   static_assert(sizeof(CCGraphBuilder) <= 4096,


If you're just experimenting this assertion can probably just be ignored
/ commented out, seems it's just to prevent accidentally wasting a lot
of memory.


Indeed. If the size of that object grows one byte above 4096, 
each instance takes 8K of (mostly unused) memory rather than 4K. 
That probably doesn't matter for debugging, but for real world 
usage, it matters a lot.

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


removing blank lines at beginnings and ends of JS blocks

2019-01-28 Thread Myk Melez

Developers,

This week I plan to remove blank lines at the beginnings and ends of JS 
blocks and update the global eslint configuration file to prohibit them 
by landing the patch in bug 1518283 
.


Note that I'm not removing all blank lines, only those at the beginnings 
and ends of blocks, which eslint calls padded-blocks 
.


Most JS blocks in the tree are already unpadded, and we already prohibit 
padded blocks in a number of subdirectories 
.


Prohibiting them tree-wide is consistent with Mark Banner's plan for 
eslint rule harmonization 
.


The patch may conflict with other patches-in-progress, given my 
experience maintaining a branch with the changes over the last few 
weeks. However, the conflict resolution is straightforward, since the 
only change being made is the removal of entirely blank lines. And 
annotation (blame) views of the codebase are unaffected, since removed 
lines don't show up there.


Let me know if you have any questions or concerns.

Cheers,
-myk

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


PSA: Inadvertently exporting third-party symbols

2019-01-28 Thread David Major
Hi,

As importing third-party code into libxul seems to be pretty popular,
I wanted to point out something that's easy to overlook.

Libraries usually have code that goes like:

#ifdef _WIN32
#define MYLIB_EXPORT __declspec(dllexport)
#else
#define MYLIB_EXPORT __attribute__((visibility("default")))
#endif

MYLIB_EXPORT int MyLibrary_DoTheThing() { return 42; }

This makes perfect sense when upstream builds their product as
MyLibrary.dll, as those APIs need to be callable from the outside
world. But when we paste the code into libxul, and its callers are
also in libxul, there is no need to cross a library boundary.

Exporting those symbols when not needed is harmful for several reasons:

* It prevents optimization and/or increases code size. The compiler
has to choose between keeping a single copy of the function with the
shape demanded by the ABI, which inhibits interprocedural
optimizations; or creating optimized/inlined copies but keeping the
original just in case the outside world ever calls it.

* It attracts wrongful blame in crash stacks. In cases where a
debugger doesn't have access to symbols, it will guess an
instruction's function based on the nearest exported symbol. This
often makes it look like function_that_landed_last_week+0x12345 caused
a crash, which wastes time with a wild goose chase.

* Exported symbols invite function hooks from questionable software.

I've filed bug 1523352 to see if we can keep track of export table
size in Perfherder similar to the existing section size metrics.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


PSA: New Multi-store Telemetry allows easier re-use of probes in multiple pings

2019-01-28 Thread Jan-Erik Rediger
Late in 2018 we landed a basic implementation of the Telemetry multi-store.
Telemetry multi-store allows the re-use of probes across multiple pings, with 
each ping having their own schedule to be sent.
It ships in Firefox 65.

Overview

The lifetime of Telemetry probes (scalars & histograms) and thus their reset 
cycle were previously closely tied to the main ping.
Custom pings had to keep track of their own data or copy out data from 
Telemetry probes.

Multi-store will now make it easier to do this without custom code to store the 
probe data.
This way data collections can still be in Histograms.json, Scalars.yaml, and 
Events.yaml and benefit from documentation, expiration, and tool integration
(like the Probe Dictionary) but be used by custom pings that have custom client 
data lifetime requirements.

Technical details

Probes (scalars & histograms) can now specify additional stores through the 
“record_into_store” property, defaulting to a single “main” store (used for the 
main ping).
Recording will happen into every store automatically.
Additionally we provide new APIs for accessing snapshots of the data:

  getSnapshotForHistograms(storeName, clearStore)
  getSnapshotForKeyedHistograms(storeName, clearStore)
  getSnapshotForScalars(storeName, clearStore)
  getSnapshotForKeyedScalars(storeName, clearStore)

Custom pings will add their store to the probes they want to include and when 
assembling the ping payload call one of the above snapshot methods to get the 
data (and clear it).

You can find a more detailed blog post with some sample code here:
https://fnordig.de/2019/01/22/multi-store-custom-telemetry-with-shared-data/
 
Telemetry APIs

The Firefox Telemetry APIs now include:

- Privileged Firefox JavaScript through nsITelemetry
,
 including the new snapshot APIs
- Submitting custom pings

- C++ code through Telemetry.h

- WebExtensions through browser.telemetry

- Specific in-product Mozilla websites through Hybrid Content Telemetry

- (newly introduced) shared test utilities in TelemetryTestUtils 



As always, please do reach out if you have any questions or concerns. 
You can find us in #telemetry on IRC, #fx-metrics on Slack and the fx-data-dev 
 mailing list.

Your Friendly Neighbourhood Firefox Telemetry Team
(:janerik, gfritzsche, :Dexter, :chutten, :travis_)
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Cookie policy/permission in live documents - proposal

2019-01-28 Thread Ehsan Akhgari
On Mon, Jan 28, 2019 at 10:51 AM Daniel Veditz  wrote:

> On Mon, Jan 28, 2019 at 12:57 AM Andrea Marchesini <
> amarches...@mozilla.com> wrote:
>
>> If we try to apply the new cookie policy immediately, 3rd party trackers
>> in opened tabs should switch to a first-party-isolation storage, but they
>> could also have already data in memory (user-tokens), and populate the new
>> cookie jar consequentially. This would break the isolation. The solution in
>> this case, is to apply the change only after the reloading.
>>
>
> That's a great point in favor of your proposal. I'm still concerned about
> "infinite-page" sites (facebook/twitter/etc) where a user typically rarely
> reloads. Would it be too ugly to apply an infobar to each active tab that
> says "The cookie policy has changed. Reload to apply the new policy
> [Reload]"? Or maybe has a [Reload this tab][Reload All] set of buttons. I
> have serious misgivings about my UX suggestion here, but maybe it will
> spark better ideas on how to communicate to users. An alert/doorhanger in
> the pref page where the setting is changed that warns the user it only
> applies to new pages and offers to reload all active tabs?
>

One option that we have for handling this change is to modify the way we
apply the change in the Preferences UI instead of asking people to reload
their pages.  For example, we can ask the user to restart their browser
when they make changes to the cookie policy/permissions (similar to how
turning permanent private browsing on/off works), or add a notice in the
Preferences saying that the changes made will only affect pages loaded from
now on, etc.

I don't think showing a message on every open tab to ask the user to reload
it is the only UX that is possible for solving this problem, it's only one
rough idea (AFAIK nobody has talked to the UX team about it yet!)...

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


Re: Cookie policy/permission in live documents - proposal

2019-01-28 Thread Daniel Veditz
On Mon, Jan 28, 2019 at 12:57 AM Andrea Marchesini 
wrote:

> If we try to apply the new cookie policy immediately, 3rd party trackers
> in opened tabs should switch to a first-party-isolation storage, but they
> could also have already data in memory (user-tokens), and populate the new
> cookie jar consequentially. This would break the isolation. The solution in
> this case, is to apply the change only after the reloading.
>

That's a great point in favor of your proposal. I'm still concerned about
"infinite-page" sites (facebook/twitter/etc) where a user typically rarely
reloads. Would it be too ugly to apply an infobar to each active tab that
says "The cookie policy has changed. Reload to apply the new policy
[Reload]"? Or maybe has a [Reload this tab][Reload All] set of buttons. I
have serious misgivings about my UX suggestion here, but maybe it will
spark better ideas on how to communicate to users. An alert/doorhanger in
the pref page where the setting is changed that warns the user it only
applies to new pages and offers to reload all active tabs?

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


Re: Does mozilla allow modification of Strings

2019-01-28 Thread Boris Zbarsky

On 1/28/19 7:51 AM, edusubscr...@gmail.com wrote:

Building was successful.


I'm really surprised the static assert at 
https://searchfox.org/mozilla-central/rev/4faab2f1b697827b93e16cb798b22b197e5235c9/dom/bindings/FakeString.h#146-147 
did not trigger, given the changes you described...


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


Re: Does mozilla allow modification of Strings

2019-01-28 Thread Emilio Cobos Álvarez
There are probably two different issues.

On 1/28/19 1:51 PM, edusubscr...@gmail.com wrote:
>   
> 
> Is it possible to add an  extra variable to mozilla string(nsTStringRepr). 
> 
> 
> I added a bool variable to nsTStringRepr class in  Xpcom/Strings/ 
> 
> While building the build  I got static Assertions like  below 
>  
>  /User/Desktop/MOZB/mozilla-central/xpcom/base/nsCycleCollector.cpp:1954:3: 
> error: static_assert failed "Don't create too large CCGraphBuilder objects" 
>  0:34.99   static_assert(sizeof(CCGraphBuilder) <= 4096, 

If you're just experimenting this assertion can probably just be ignored
/ commented out, seems it's just to prevent accidentally wasting a lot
of memory.

>  
> 
> 
> In normal execution (without adding any variable to core string )size of 
> CCGraphBuilder is 4096 
> 
> Since it is static_assert I commented that line and  proceeded  in building. 
> 
> Building was successful. 
> 
> While running  firefox it just popsup and crashes. 
> 
> I used debbugger to know where it is getting crashed 
> It is at 
> 
> /mozilla-central/servo/components/style/gecko/data.rs 
> 75: debug_assert!(!self.inner().mContents.mRawPtr.is_null()); 
> 
> I  guess it is  getting crashed because of memory issue. I don't exactly. 
> Does anyone  know what is the exact problem and help me in modification of 
> mozilla string.

This one probably means that you forgot to also update the rust version
of the string representation in:


https://searchfox.org/mozilla-central/rev/4faab2f1b697827b93e16cb798b22b197e5235c9/xpcom/rust/nsstring/src/lib.rs#368

And as a result you're corrupting some memory somewhere. I think we have
unit tests that would catch that mistake:


https://searchfox.org/mozilla-central/rev/4faab2f1b697827b93e16cb798b22b197e5235c9/xpcom/rust/nsstring/src/lib.rs#1302

If you updated both, then I don't know and I'd have to take a look on a
debugger :)

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


Does mozilla allow modification of Strings

2019-01-28 Thread edusubscribe


Is it possible to add an  extra variable to mozilla string(nsTStringRepr). 


I added a bool variable to nsTStringRepr class in  Xpcom/Strings/ 

While building the build  I got static Assertions like  below 
 
 /User/Desktop/MOZB/mozilla-central/xpcom/base/nsCycleCollector.cpp:1954:3: 
error: static_assert failed "Don't create too large CCGraphBuilder objects" 
 0:34.99   static_assert(sizeof(CCGraphBuilder) <= 4096, 

 


In normal execution (without adding any variable to core string )size of 
CCGraphBuilder is 4096 

Since it is static_assert I commented that line and  proceeded  in building. 

Building was successful. 

While running  firefox it just popsup and crashes. 

I used debbugger to know where it is getting crashed 
It is at 

/mozilla-central/servo/components/style/gecko/data.rs 
75: debug_assert!(!self.inner().mContents.mRawPtr.is_null()); 

I  guess it is  getting crashed because of memory issue. I don't exactly. Does 
anyone  know what is the exact problem and help me in modification of mozilla 
string.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Does mozilla allow to modify the core strings in it.

2019-01-28 Thread edusubscribe
Is it possible to add an  extra variable to mozilla string(nsTStringRepr).


I added a bool variable to nsTStringRepr class in  Xpcom/Strings/

While building the build  I got static Assertions like  below

 
/Users/pavan-6853/Desktop/MOZB/mozilla-central/xpcom/base/nsCycleCollector.cpp:1954:3:
 error: static_assert failed "Don't create too large CCGraphBuilder objects"
 0:34.99   static_assert(sizeof(CCGraphBuilder) <= 4096,




In normal execution (without adding any variable to core string )size of 
CCGraphBuilder is 4096

Since it is static_assert I commented that line and  proceeded  in building.

Building was successful.

While running  firefox it just popsup and crashes.

I used debbugger to know where it is getting crashed
It is at 

/mozilla-central/servo/components/style/gecko/data.rs
75: debug_assert!(!self.inner().mContents.mRawPtr.is_null());

I  guess it is  getting crashed because of memory issue. I don't exactly. Does 
anyone  know what is the exact problem and help me in modification of mozilla 
string. 
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Cookie policy/permission in live documents - proposal

2019-01-28 Thread Johann Hofmann
Thanks for writing this up and adding the great explanations, Andrea!

I think your proposal has a lot of benefits and I don't have any major
concerns about it, I would just like to add a few comments:

Just to reiterate, per your proposal the cookie policy
(network.cookieBehavior pref) and permission will be stored on the load
info.

As you said, after this change, we should definitely add some notice in our
UI to make users aware of the fact that they need to reload their tabs for
the setting to take effect. I would absolutely advise against silently
reloading tabs (which was suggested somewhere in this thread) and
potentially discarding user data without explicit choice, though. If our UX
designer find a good way to offer users the choice to reload all tabs via
an optional button, that might be a good middle ground.

To be honest, either way I think this is a slight degradation in user
experience. Nobody likes restarting/reloading their things after updating
settings. Given the benefits this is probably acceptable.

In general, I don't think a lot of people regularly update their cookie
exceptions. There might be a larger group of people
who occasionally modify their cookie policy, but I doubt that many of them
regularly switch between two settings, making this something that is most
likely to be noticed in artificial conditions e.g. by web developers or
other power users. Hence, I totally expect to receive bug reports once we
start changing this.

All in all this seems like a fair compromise to me.

Thanks,

Johann


On Mon, Jan 28, 2019 at 10:32 AM Andrea Marchesini 
wrote:

>
>
> On Fri, Jan 25, 2019 at 11:55 PM Ehsan Akhgari 
> wrote:
>
>> On Fri, Jan 25, 2019 at 2:51 PM Daniel Veditz 
>> wrote:
>>
>>>
>>> Your description equating cookies and storage within a document lifetime
>>> makes sense. Is this intended to also apply to network requests? The
>>> first-party document already has no access to 3rd party cookies so it
>>> shouldn't matter at that level if Necko's rules change "live". If I'm on
>>> twitter/facebook (which make constant background requests) and I clear my
>>> entire cookie jar those documents are going to break. If I just tossed all
>>> my cookies that's what I want! Discovering that I'm still logged into those
>>> sites would be disturbing. Similarly, if I flip the "block 3rd-party
>>> cookies" pref I'm going to react negatively if I still see tracker cookies
>>> showing up just because I've left an active page open somewhere.
>>>
>>
>> Cookies have been dynamic and racey since the dawn of time, both at the
>> HTTP layer and in their reflection in DOM (document.cookie).  Clearing your
>> cookies isn't something that is changed by this proposal.  I'm not too sure
>> how Andrea was planning to handle cookie policy at the Necko layer but we
>> have a lot of flexibility here and pages also can probably tolerate dynamic
>> changes to document.cookie.  I *think* handling the cookie policy globally
>> at the Necko layer is probably easier but I'm curious to know Andrea's
>> thoughts.
>>
>
> As Ehsan says, I don't want to change how we cleanup cookies but only how
> we expose cookie policy and how to treat cookie permissions.
> About the implementation, I would store the current cookie policy and
> cookie permission into the nsILoadInfo object of a document/subdocument
> nsIChannel. This information will be used in nsCookieService,
> document.cookies() getter/setter and propagated to any
> storage/communication API as described in this email thread (from
> BroadcastChannel to localStorage). I also would audit any cookie-permission
> check and any cookie-policy preference getter call.
>
> In this way, when the cookie behavior or a cookie permission changes, the
> document will not be affected because it has its own 'copy' of the previous
> values.
> We also need to improve the UI to communicate the the changes will apply
> at the next reload of the current tabs.
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Cookie policy/permission in live documents - proposal

2019-01-28 Thread Andrea Marchesini
On Fri, Jan 25, 2019 at 11:55 PM Ehsan Akhgari 
wrote:

> On Fri, Jan 25, 2019 at 2:51 PM Daniel Veditz  wrote:
>
>>
>> Your description equating cookies and storage within a document lifetime
>> makes sense. Is this intended to also apply to network requests? The
>> first-party document already has no access to 3rd party cookies so it
>> shouldn't matter at that level if Necko's rules change "live". If I'm on
>> twitter/facebook (which make constant background requests) and I clear my
>> entire cookie jar those documents are going to break. If I just tossed all
>> my cookies that's what I want! Discovering that I'm still logged into those
>> sites would be disturbing. Similarly, if I flip the "block 3rd-party
>> cookies" pref I'm going to react negatively if I still see tracker cookies
>> showing up just because I've left an active page open somewhere.
>>
>
> Cookies have been dynamic and racey since the dawn of time, both at the
> HTTP layer and in their reflection in DOM (document.cookie).  Clearing your
> cookies isn't something that is changed by this proposal.  I'm not too sure
> how Andrea was planning to handle cookie policy at the Necko layer but we
> have a lot of flexibility here and pages also can probably tolerate dynamic
> changes to document.cookie.  I *think* handling the cookie policy globally
> at the Necko layer is probably easier but I'm curious to know Andrea's
> thoughts.
>

As Ehsan says, I don't want to change how we cleanup cookies but only how
we expose cookie policy and how to treat cookie permissions.
About the implementation, I would store the current cookie policy and
cookie permission into the nsILoadInfo object of a document/subdocument
nsIChannel. This information will be used in nsCookieService,
document.cookies() getter/setter and propagated to any
storage/communication API as described in this email thread (from
BroadcastChannel to localStorage). I also would audit any cookie-permission
check and any cookie-policy preference getter call.

In this way, when the cookie behavior or a cookie permission changes, the
document will not be affected because it has its own 'copy' of the previous
values.
We also need to improve the UI to communicate the the changes will apply at
the next reload of the current tabs.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Cookie policy/permission in live documents - proposal

2019-01-28 Thread Andrea Marchesini
On Fri, Jan 25, 2019 at 8:50 PM Daniel Veditz  wrote:

>
> Your description equating cookies and storage within a document lifetime
> makes sense. Is this intended to also apply to network requests? The
> first-party document already has no access to 3rd party cookies so it
> shouldn't matter at that level if Necko's rules change "live". If I'm on
> twitter/facebook (which make constant background requests) and I clear my
> entire cookie jar those documents are going to break. If I just tossed all
> my cookies that's what I want! Discovering that I'm still logged into those
> sites would be disturbing. Similarly, if I flip the "block 3rd-party
> cookies" pref I'm going to react negatively if I still see tracker cookies
> showing up just because I've left an active page open somewhere.
>

Denying access to cookies doesn't make the user logged out from a website.
The website can still have tokens in memory and send them to servers, for
example. And also if the website is not able to identify you to the server,
it will probably show the same UI as you were still logged in. This is
extremely confusing. The only way to be sure you are logged out is a full
reload of the opened tabs.

Another good point to accept this proposal is that, when we will have the
StoragePrincipal in place, going from BEHAVIOR_ACCESS to
BEHAVIOR_REJECT_TRACKER would be actually a big problem without reloading
the tabs. If we try to apply the new cookie policy immediately, 3rd party
trackers in opened tabs should switch to a first-party-isolation storage,
but they could also have already data in memory (user-tokens), and populate
the new cookie jar consequentially. This would break the isolation. The
solution in this case, is to apply the change only after the reloading.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


[desktop] Bugs logged by Desktop Release QA in the last 7 days

2019-01-28 Thread Mihai Boldan

Hello,

Here's the list of new issues found and filed by the Desktop Release QA 
team last two weeks.
Additional details on the team's priorities last week, as well as the 
plans for the current week are available at: https://tinyurl.com/ya5okvx7

Bugs logged by Desktop Release QA in the last 7 days:

Firefox: Toolbars and Customization
NEW - https://bugzil.la/1521711 - Update the Toolbars dropdown menu in 
customize mode to look more similar to the other panels in customize mode


Firefox: Site Identity and Permission Panels
NEW - https://bugzil.la/1522805 - [Windows] Permission panel - disabled 
option is not readable due to color setup


Firefox: Search
NEW - https://bugzil.la/1522816 - Keywords can be duplicated for 
different search engines


Core: Audio/Video: Playback
NEW - https://bugzil.la/1521769 - MP4 2D-4K video is jerky

Core: Layout
NEW - https://bugzil.la/1522078 - [Ubuntu] Mozilla crash reporter 
panel’s width is infinite extended if the user types a comment with no space


DevTools: Inspector
NEW - https://bugzil.la/1522842 - Dark Theme-DevEd - Filter rules 
containing this property icon is colored black


This is available as a Bugzilla bug list as well: 
https://tinyurl.com/yb3ro6tt

Regards,
Mihai Boldan
QC Engineer
Softvision

The content of this communication is classified as Softvision 
Confidential and Proprietary Information.


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