Re: Proposal to adjust our clang-format rules to include spaces after the hash for nested preprocessor directives

[Ehsan Akhgari]

I've received some on and off-list responses to this, all in favour.  I've
filed https://bugzilla.mozilla.org/show_bug.cgi?id=1521000 to make the
change.

Thanks to everyone who provided feedback.

On Thu, Jan 10, 2019 at 8:01 PM Ehsan Akhgari 
wrote:

> Hi everyone,
>
> I'd like to propose the first modification to our clang-format rules based
> on the feedback I've received since the switch to our new coding style from
> the SpiderMonkey team.
>
> The problem we're trying to solve is that in code that has nested
> preprocessor directives, when the directives are far apart from each other,
> it may be difficult to spot that a given directive is nested under another
> one, and this may hide important logic in cases such as a chain of
> #if/#ifdef directives.  Here is a practical example: <
> https://github.com/sylvestre/gecko-dev/blob/master/js/src/wasm/WasmBaselineCompile.cpp#L4054>.
> When looking at code like this, it can be quite difficult to know where in
> the (potential) preprocessor nesting levels you are without scrolling
> around quite a bit and keeping a lot of context in mind.  In the example
> above, I would say that's not reasonably possible to do.
>
> The common way to deal with this problem is to indent nested preprocessor
> directives, very much similarly to how we indent normal code, for example:
>
> #if foo
> #  if bar
> #define x 1
> #  else
> #define x 2
> #  endif
> #endif
>
> This is allowed, but not required, by our coding style <
> https://google.github.io/styleguide/cppguide.html#Preprocessor_Directives>,
> and is supported by clang-format by using the IndentPPDirectives: AfterHash
> option .  So in
> order to accommodate easier reading of code like this, I propose to adopt
> that clang-format option.
>
> You can look at how the above code looks like after reformatting the tree
> with that option here: <
> https://github.com/ehsan/gecko-dev/blob/45df04c211f4dd875c58125bca5fbca381ed8fca/js/src/wasm/WasmBaselineCompile.cpp#L4824>.
> If you are curious to look at the entire tree or the resulting changes,
> they're available for viewing here respectively: <
> https://github.com/ehsan/gecko-dev/tree/afterhash> and <
> https://github.com/ehsan/gecko-dev/commit/45df04c211f4dd875c58125bca5fbca381ed8fca
> >.
>
> Please let me know if you have any suggestions or concerns.  Ideas that
> have come up before include doing nothing about this problem (which I think
> isn't a reasonable answer since I think the problem proposed is valid and
> has a viable solution covered in our coding style which we can achieve
> easily with tooling) and doing this in a small scope, for example only for
> SpiderMonkey (which the SpiderMonkey team or myself prefer not to do since
> we don't have any evidence to suggest the problem is localized to that part
> of the code base and we prefer to not have special cases in our coding
> style where we can avoid it, also this change isn't nearly as invasive as
> the tree-wide reformat so we should be able to do it with minimal
> disruption to anyone's work).
>
> I'm planning to file a bug on this by the end of next week if no serious
> concerns aren't raised by then.
>
> Thanks,
> --
> Ehsan
>


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

[Bugzilla Change] "Mac OS X" deprecated, "macOS" added

[Emma Humphries]

Per https://bugzilla.mozilla.org/show_bug.cgi?id=1520927, we've disabled
Mac OS X as an OS choice.

This doesn't affect existing bugs marked with Mac OS X.

New bugs on Mac desktop should select "macOS".

Please update your saved searches accordingly.

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

DataMutex

[Jeff Muizelaar]

DataMutex is a Rust style Mutex that's templated on the type of data
that it's locking.

We've had an implementation of this in EME for a while but it hasn't
been used more widely. I've moved DataMutex from EME to xpcom/threads
to encourage it's use. Generally, one should prefer to use a DataMutex
as it makes it clear what data is being protected by the mutex and
also ensures the mutex is acquired before giving access to the data
inside.

Also, if you're in JS engine the equivalent is js::ExclusiveData.
Perhaps one day these could be unified in MFBT but for now they're
separate.

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

Intent to Implement and Ship: The border-{start,end}-{start,end}-radius CSS properties

[Mats Palmgren]

Summary:
The border-{start,end}-{start,end}-radius CSS properties are flow-relative
versions of their corresponding physical property, border-top-left-radius etc

Bug:
https://bugzilla.mozilla.org/show_bug.cgi?id=1520684

Link to standard:
https://drafts.csswg.org/css-logical-1/#border-radius-properties

Platform coverage: All platforms

Estimated or target release: Firefox 66

Preference behind which this will be implemented: None

Is this feature enabled by default in sandboxed iframes? Yes

DevTools bug: N/A, devtools support is included in these patches

Do other browser engines implement this?
I don't know, there was no WPT for these.

web-platform-tests:
I added one: css/css-logical/logical-box-border-radius.html

Is this feature restricted to secure contexts? No


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

[IMPORTANT] QA Firefox 67 feature testing pi-requests deadline is January 23

[Tom Grabowski]

Hi,

Similar to what QA did for previous Firefox feature testing prioritization
,
we would like to do the same for Fx67. In order to help with the
process, *please
submit your pi-request
 by January 23. *This
is needed to assure QA will be involved in your feature development work
for the Nightly 67 cycle.


*Q: What happens after the deadline?*
A: After the deadline QA will come back with a prioritized list of work
that represents what we are committing to for the next cycle. We want to
ensure this list matches eng and product expectations.

*Q: What if I miss the deadline?*
A: We reserve the right to say that we can't pick up work for late requests
in the current cycle. You can still develop and execute your own test plan
or defer the work to the following cycle.

*Q: What about unknown or unexpected requests? What if there is a business
reason for a late request? What do we do with experiments and System
Add-ons?*
A: In order to remain flexible, we will keep some percentage of time open
for requests like these.

*Q: There's no way I'm going to remember to do this. *
A: Do it now! I'll also send out a reminder next week.

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

Re: Proposal to adjust testing to run on PGO builds only and not test on OPT builds

[Jan de Mooij]

Hi Joel,

Can you say more about this point in your original email: "3) This will
reduce the jobs (about 16%) we run which in turn reduces, cpu time, money
spent, turnaround time, intermittents, complexity of the taskgraph." It
seems to me that if we remove non-PGO opt builds even on Try, we might use
more cpu time because there are so many Try pushes requesting opt builds.
Do we have data on this?

Thanks,
Jan

On Thu, Jan 17, 2019 at 5:45 PM jmaher  wrote:

> Following up on this, thanks to Chris we have fast artifact builds for
> PGO, so the time to develop and use try server is in parity with current
> opt solutions for many cases (front end development, most bisection cases).
>
> I have also looked in depth at what the impact on the integration branches
> would be.  In the data set from July-December (H2 2018) there were 11
> instances of tests that we originally only scheduled in the OPT config and
> we didn't have PGO or Debug test jobs to point out the regression (this is
> due to scheduling choices).  Worse case scenario is finding the regression
> on PGO up to 1 hour later 11 times or roughly 2x/month.  Backfilling to
> find the offending patch as we do now 24% of the time would be similar
> time.  In fact running the OPT jobs on Debug instead would result in same
> time for all 11 instances (due to more chunks on debug and similar
> runtimes).  In short, little to no impact.
>
> Lastly there was a pending question about talos.  There is an edge case
> where we can see a regression on talos that is PGO, but it is unrelated to
> the code and just a side effect of how PGO works.  I looked into that in
> https://bugzilla.mozilla.org/show_bug.cgi?id=1514829.  I found that if we
> didn't get opt alerts that we would not have missed any regressions.
> Furthermore, for the regressions, for the ones that were pgo only
> regressions (very rare) there were many other regressions at the same time
> (say a build change, or test change, etc.) and usually these were accepted
> changes, backed out, or investigated on a different test or platform.  In
> the past when we have determined a regression is a PGO artifact we have
> resolved it as WONTFIX and moved on.
>
> Given this summary, I feel that most concerns around removing testing for
> OPT are addressed.  I would also like to extend the proposal to remove the
> OPT builds since no unit or perf tests would run on there.
>
> As my original timeline is not realistic, I would like to see if there are
> comments until next Wednesday- January 23rd, then I can follow up on
> remaining issues or work towards ensuring we start the process of making
> this happen and what the right timeline is.
> ___
> 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: Crash-stop integrated by default for crash reports in Bugzilla

[Andrew Overholt]

That looks like it could be incredibly useful! Thanks for pointing it out
and for doing the work.

On Thu, Jan 17, 2019 at 11:50 AM Calixte Denizet 
wrote:

> Hi,
>
> A few months ago, we published the first version of the crash-stop addon
> [1]. The goal was to integrate crash data along with some analysis (ex:
> startup crashes) directly into Bugzilla. The goal is to help with bug
> prioritization and make it easier to determine if a patch has fixed the
> issue after it lands.
>
> Following the example of the Bugzilla Socorro Lens, we have decided to
> enable this functionality for everyone directly in Bugzilla given its
> impact and usefulness. Thanks to Kohei’s work, the table containing the
> data is now displayed in crash bugs directly without the help of the addon.
> As an example, see the bug below:
>
> https://bugzilla.mozilla.org/show_bug.cgi?id=1519468
>
> Users who have previously installed the crash-stop addon can safely remove
> it now.
>
> For more information on the displayed data:
>
> https://crash-stop-addon.herokuapp.com/
>
> If you see something wrong or if you want to have more data, please feel
> free to file a bug:
>
> https://github.com/mozilla/crash-stop-addon
>
> Calixte
>
> [1] https://addons.mozilla.org/firefox/addon/bugzilla-crash-stop/
> ___
> 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: Proposal to adjust testing to run on PGO builds only and not test on OPT builds

[James Graham]

On 17/01/2019 16:42, jmaher wrote:

Following up on this, thanks to Chris we have fast artifact builds for PGO, so 
the time to develop and use try server is in parity with current opt solutions 
for many cases (front end development, most bisection cases).


Even as someone not making frequent changes to compiled code I 
occasionally want to both rebuild and run tests on opt (e.g. because 
some test changes also require changes to moz.build files that could 
break the build in a way that isn't caught by an artifact build). In 
this case adding an extra hour of end-to-end time on try is a pretty 
serious regression.


For my specific use case it might be enough if we could schedule 
artifact builds for PGO and full builds for debug. But I suspect it's 
going to work better for more people — and save more resources overall — 
to simply keep the default try configuration as-is and just turn off 
non-PGO opt builds (or at least tests) on integration branches / central.

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

Crash-stop integrated by default for crash reports in Bugzilla

[Calixte Denizet]

Hi,

A few months ago, we published the first version of the crash-stop addon
[1]. The goal was to integrate crash data along with some analysis (ex:
startup crashes) directly into Bugzilla. The goal is to help with bug
prioritization and make it easier to determine if a patch has fixed the
issue after it lands.

Following the example of the Bugzilla Socorro Lens, we have decided to
enable this functionality for everyone directly in Bugzilla given its
impact and usefulness. Thanks to Kohei’s work, the table containing the
data is now displayed in crash bugs directly without the help of the addon.
As an example, see the bug below:

https://bugzilla.mozilla.org/show_bug.cgi?id=1519468

Users who have previously installed the crash-stop addon can safely remove
it now.

For more information on the displayed data:

https://crash-stop-addon.herokuapp.com/

If you see something wrong or if you want to have more data, please feel
free to file a bug:

https://github.com/mozilla/crash-stop-addon

Calixte

[1] https://addons.mozilla.org/firefox/addon/bugzilla-crash-stop/
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Proposal to adjust testing to run on PGO builds only and not test on OPT builds

[jmaher]

Following up on this, thanks to Chris we have fast artifact builds for PGO, so 
the time to develop and use try server is in parity with current opt solutions 
for many cases (front end development, most bisection cases).

I have also looked in depth at what the impact on the integration branches 
would be.  In the data set from July-December (H2 2018) there were 11 instances 
of tests that we originally only scheduled in the OPT config and we didn't have 
PGO or Debug test jobs to point out the regression (this is due to scheduling 
choices).  Worse case scenario is finding the regression on PGO up to 1 hour 
later 11 times or roughly 2x/month.  Backfilling to find the offending patch as 
we do now 24% of the time would be similar time.  In fact running the OPT jobs 
on Debug instead would result in same time for all 11 instances (due to more 
chunks on debug and similar runtimes).  In short, little to no impact.

Lastly there was a pending question about talos.  There is an edge case where 
we can see a regression on talos that is PGO, but it is unrelated to the code 
and just a side effect of how PGO works.  I looked into that in 
https://bugzilla.mozilla.org/show_bug.cgi?id=1514829.  I found that if we 
didn't get opt alerts that we would not have missed any regressions.  
Furthermore, for the regressions, for the ones that were pgo only regressions 
(very rare) there were many other regressions at the same time (say a build 
change, or test change, etc.) and usually these were accepted changes, backed 
out, or investigated on a different test or platform.  In the past when we have 
determined a regression is a PGO artifact we have resolved it as WONTFIX and 
moved on.

Given this summary, I feel that most concerns around removing testing for OPT 
are addressed.  I would also like to extend the proposal to remove the OPT 
builds since no unit or perf tests would run on there.

As my original timeline is not realistic, I would like to see if there are 
comments until next Wednesday- January 23rd, then I can follow up on remaining 
issues or work towards ensuring we start the process of making this happen and 
what the right timeline is.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Intent to put off to ship: Event.returnValue

[Masayuki Nakano]

Event.returnValue is legacy attribute which was introduced by IE. We 
released this feature at 63, but it broke an internet application (bug 
1478102).


Therefore, we unshipped it temporarily at 64 because the fix of bug 
1479964 will fix the intranet application.  However, unfortunately, the 
fix broke another internet app (bug 1514940).


Therefore, we put off to reship this at least until 66.

--
Masayuki Nakano 
Software Engineer, Mozilla
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Intent to ship: set keyCode or charCode of "keypress" event to the other's non-zero value

[Masayuki Nakano]

We've decided to put this off at least 66.
https://bugzilla.mozilla.org/show_bug.cgi?id=1520756

On 2018/11/30 10:37, Masayuki Nakano wrote:
Summary: We'll set keyCode of "keypress" event to its charCode value if 
keyCode is 0, charCode of "keypress" event to its keyCode value if 
charCode is 0.


Bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1479964

Link to standard: "conflated model" of 
https://w3c.github.io/uievents/#determine-keypress-keyCode (we've used 
"split model"). But no declaration of charCode value of Enter key press.


(Oh, I had not realized this section when I wrote the commit message...)

Platform coverage: All

Estimated or target release: 65

DevTools bug: N/A

Do other browser engines implement this?

Yes, the other browsers use this behavior traditionally, and UI Events 
declared this behavior as "conflated model".


web-platform-tests: N/A due to requiring user input, but we have 
mochitests with synthesized events.


This new behavior was enabled in Nightly since early of 65. This caused 
breaking Google Closure Library.  However, it have been fixed by the 
cooperation of the developers.


Of course, we may meet other broken web apps especially in intranet. 
While we're testing this behavior in Nightly, we used blacklist pref 
(dom.keyboardevent.keypress.hack.use_legacy_keycode_and_charcode) to 
disable this new behavior only in specific domains.  If we'll get broken 
web apps even after release, we and users can disable it with using this 
blacklist.


Enabling patch has been landed from:
https://bugzilla.mozilla.org/show_bug.cgi?id=1496288
because we need to manage those changes as a set.




--
Masayuki Nakano 
Software Engineer, Mozilla
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Intent to ship: window.event

[Masayuki Nakano]

We've decided to put this off at least 66.
https://bugzilla.mozilla.org/show_bug.cgi?id=1520756

On 2018/11/30 10:47, Masayuki Nakano wrote:
Summary: Enable window.event which can access dispatching event from 
everywhere.


Bug: https://bugzilla.mozilla.org/show_bug.cgi?id=218415

Link to standard: https://dom.spec.whatwg.org/#ref-for-dom-window-event

Platform coverage: All

Estimated or target release: 65

DevTools bug: (not sure)

Do other browser engines implement this?

(I think so, but I'm not sure because of not my fix.)

Enabling patch has been landed from:
https://bugzilla.mozilla.org/show_bug.cgi?id=1496288
because we need to manage those changes as a set.

This was disabled temporarily due to a lot of web apps assume that 
window.event is available only in IE, and IE stores a Unicode code point 
in keyCode of "keypress" event. Google Chrome and Edge followed same 
behavior for both, so, such apps work even with them.


This is now fixed by the both fixes of:
https://groups.google.com/forum/#!topic/mozilla.dev.platform/E8DyIJBhu1g
https://groups.google.com/forum/#!topic/mozilla.dev.platform/IWLLJmoGroA




--
Masayuki Nakano 
Software Engineer, Mozilla
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform