Re: New [must_use] property in XPIDL

2016-08-22 Thread R Kent James 
On 8/22/2016 9:28 PM, Nicholas Nethercote wrote:
> On Tue, Aug 23, 2016 at 9:39 AM, R Kent James  wrote:
>> On 8/21/2016 9:14 PM, Nicholas Nethercote wrote:
>>> I strongly encourage people to do likewise on
>>> any IDL files with which they are familiar. Adding appropriate checks isn't
>>> always easy
>> Exactly, and I hope that you and others restrain your exuberance a
>> little bit for this reason. A warning would be one thing, but a hard
>> failure that forces developers to drop what they are doing and think
>> hard about an appropriate check is just having you set YOUR priorities
>> for people rather than letting people do what might be much more
>> important work.
> I see that I've caused several Thunderbird breakages with the changes
> I've been making. I apologize for not giving you advance warning about
> this.
>
> I have several desires here.
>
> - I want to greatly increase the use of MOZ_MUST_USE/[must_use] in Gecko.
>
> - I don't want to make Thunderbird developers' lives more difficult.
>
> - I don't want to have to modify comm-central when I add new uses of
> MOZ_MUST_USE/[must_use] to mozilla-central.
>
> The best solution I can see is to insert -Wno-unused-result or
> -Wno-error=unused-result in the appropriate place(s) in comm-central.
>
> Nick

Thanks for the suggestions. Generally though Thunderbird accepts that we
need to fix these breakages, so I am not discouraging you from moving
forward for our sake.

I would still encourage you though to be a little careful about forcing
checks when they are not needed. Just as an example, in nsIPipe we now have:

|[must_use] readonly attribute nsIAsyncInputStream inputStream;|

A common programming pattern that I would use, when I don't really care
about why something failed, is:

nsCOMPtr inputStream;

pipe.GetInputStream(getter_AddRefs(inputStream));

if (!inputStream) {

  ... take action

}

If I understand the must_use correctly, you are specifically not
allowing that pattern. Why? If you have good reasons, OK fine do it. Hey
I'm a hack, maybe my example sucks and you really do want to discourage
it. But I would like to understand the reasoning for this, as from my
perspective this is just code churn where your over-exuberance is
creating unneeded work (with associated risk of regressions).

Please consider this a general question and not Thunderbird-specific.

:rkent


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

Re: New [must_use] property in XPIDL

2016-08-22 Thread Nicholas Nethercote 
On Mon, Aug 22, 2016 at 2:14 PM, Nicholas Nethercote
 wrote:
>
> If you file a bug to add [must_use] or MOZ_MUST_USE somewhere, please make the
> bug block the tracking bug

Some recent work on this front:

- Jan Varga used [must_use] in nsIQuota{ManagerService,Requests}.idl:
https://bugzilla.mozilla.org/show_bug.cgi?id=1297056

- Wei-Cheng Pan is using MOZ_MUST_USE in uriloader/:
https://bugzilla.mozilla.org/show_bug.cgi?id=1293212

It's a good start, but barely scratches the surface! Let's keep them coming.

More motivation:
https://bugzilla.mozilla.org/show_bug.cgi?id=1290350#c6 explains how
an OOM crash that I thought I had straightforwardly fixed turns out to
be not so straightforward after all, due to missing checks.

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

Re: New [must_use] property in XPIDL

2016-08-22 Thread Nicholas Nethercote 
On Tue, Aug 23, 2016 at 9:39 AM, R Kent James  wrote:
> On 8/21/2016 9:14 PM, Nicholas Nethercote wrote:
>> I strongly encourage people to do likewise on
>> any IDL files with which they are familiar. Adding appropriate checks isn't
>> always easy
>
> Exactly, and I hope that you and others restrain your exuberance a
> little bit for this reason. A warning would be one thing, but a hard
> failure that forces developers to drop what they are doing and think
> hard about an appropriate check is just having you set YOUR priorities
> for people rather than letting people do what might be much more
> important work.

I see that I've caused several Thunderbird breakages with the changes
I've been making. I apologize for not giving you advance warning about
this.

I have several desires here.

- I want to greatly increase the use of MOZ_MUST_USE/[must_use] in Gecko.

- I don't want to make Thunderbird developers' lives more difficult.

- I don't want to have to modify comm-central when I add new uses of
MOZ_MUST_USE/[must_use] to mozilla-central.

The best solution I can see is to insert -Wno-unused-result or
-Wno-error=unused-result in the appropriate place(s) in comm-central.

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

Re: New [must_use] property in XPIDL

2016-08-22 Thread Nathan Froyd 
On Mon, Aug 22, 2016 at 7:39 PM, R Kent James  wrote:
> On 8/21/2016 9:14 PM, Nicholas Nethercote wrote:
>> I strongly encourage people to do likewise on
>> any IDL files with which they are familiar. Adding appropriate checks isn't
>> always easy
>
> Exactly, and I hope that you and others restrain your exuberance a
> little bit for this reason. A warning would be one thing, but a hard
> failure that forces developers to drop what they are doing and think
> hard about an appropriate check is just having you set YOUR priorities
> for people rather than letting people do what might be much more
> important work.

It's worth noting that "an appropriate check" may be as simple as:

  mozilla::Unused << MustUseMethod(...);

which effectively retains the status quo of not checking, while
quieting the compiler warning.

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

Re: New [must_use] property in XPIDL

2016-08-22 Thread Nick Fitzgerald 
On Mon, Aug 22, 2016 at 4:39 PM, R Kent James  wrote:

> On 8/21/2016 9:14 PM, Nicholas Nethercote wrote:
> > I strongly encourage people to do likewise on
> > any IDL files with which they are familiar. Adding appropriate checks
> isn't
> > always easy
>
> Exactly, and I hope that you and others restrain your exuberance a
> little bit for this reason. A warning would be one thing, but a hard
> failure that forces developers to drop what they are doing and think
> hard about an appropriate check
>

​Developers absolutely must already be thinking hard about appropriate
checks when calling fallible functions.​ These kinds of annotations make
our lives easier, not harder, in the long run.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: New [must_use] property in XPIDL

2016-08-22 Thread Bobby Holley 
On Mon, Aug 22, 2016 at 4:39 PM, R Kent James  wrote:

> On 8/21/2016 9:14 PM, Nicholas Nethercote wrote:
> > I strongly encourage people to do likewise on
> > any IDL files with which they are familiar. Adding appropriate checks
> isn't
> > always easy
>
> Exactly, and I hope that you and others restrain your exuberance a
> little bit for this reason. A warning would be one thing, but a hard
> failure that forces developers to drop what they are doing and think
> hard about an appropriate check is just having you set YOUR priorities
> for people rather than letting people do what might be much more
> important work.
>
> There's lots of legacy code around that may or may not be worth the
> effort to think hard about such failures. This is really better suited
> for a static checker that can make a list of problems, which list can be
> managed appropriately for priority, rather than a hard error that forces
> us to drop everything.
>

I don't quite follow the objection here.

Anybody who adds such an annotation needs to get the tree green before they
land the annotation. Developers writing new code that ignores the nsresult
will get instant feedback (by way of try failure) that the developer of the
API thinks the nsresult needs to be checked. This doesn't seem like an
undue burden, and enforced-by-default assertions are critical to code
hygiene and quality.

If your concern is the way this API change may break Thunderbird-only
consumers of shared XPCOM APIs, that's related to Thunderbird being a
non-Tier-1 platform, and pretty orthogonal to the specific change that Nick
made.

bholley



> :rkent
> ___
> 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: New [must_use] property in XPIDL

2016-08-22 Thread R Kent James 
On 8/21/2016 9:14 PM, Nicholas Nethercote wrote:
> I strongly encourage people to do likewise on
> any IDL files with which they are familiar. Adding appropriate checks isn't
> always easy

Exactly, and I hope that you and others restrain your exuberance a
little bit for this reason. A warning would be one thing, but a hard
failure that forces developers to drop what they are doing and think
hard about an appropriate check is just having you set YOUR priorities
for people rather than letting people do what might be much more
important work.

There's lots of legacy code around that may or may not be worth the
effort to think hard about such failures. This is really better suited
for a static checker that can make a list of problems, which list can be
managed appropriately for priority, rather than a hard error that forces
us to drop everything.

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

Intent to ship imageSmoothingEnabled and remove mozImageSmoothingEnabled.

2016-08-22 Thread Thomas Wisniewski 
Summary: ctx.imageSmoothingEnabled is a Canvas2D context property
controlling the interpolation of images drawn to 2D canvases
(especially useful for pixel art).

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

Link to standard:
https://html.spec.whatwg.org/multipage/scripting.html#image-smoothing

Platform coverage: all.

Estimated or target release: Firefox 52

Preference behind which this will be implemented: None.

This has been unprefixed in Chrome since version 41, and it should
ship unprefixed in Safari 10.

Web platform tests now cover the feature as well.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Intent to deprecate: Building mozilla-central without Skia enabled

2016-08-22 Thread George Wright 

I'm looking to remove the option to build mozilla-central without Skia.

Currently we default to Skia being disabled, and enable it using 
--enable-skia. This is problematic because all of our officially 
supported build configurations enable Skia, and as such the 
Skia-is-disabled build has been buggy for quite some time. This has 
manifested most recently in bug 1296524.


In the past year, Skia has moved from being an optional part of our 
graphics code to being a core, indispensable module. We are actively 
pursuing making it the default backend in almost all canvas and content 
configurations; it is already the default software canvas option on all 
platforms and will soon be for all software-backed content as well.


To this end, I'd like to remove support for building mozilla-central 
without Skia as I think it should now be considered an integral part of 
our code.


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

Re: Intent to implement and ship: SVGElement.prototype.dataset

2016-08-22 Thread Boris Zbarsky 

On 8/15/16 1:57 PM, Aryeh Gregor wrote:

Why doesn't this just go on Element, like .className and .id?


If you think it should, and can convince Blink and Edge to do that, we 
could do it from a technical point of view.


From a theoretical point of view, it doesn't seem like as generic a 
concept as className/id, not least because it effectively reserves a 
whole bunch of attribute names that may already be in use in random XML 
dialects.  I guess this is the reason why we would want these less on 
"whatever".  MathML doesn't have this problem, of course.


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

Re: Intent to unship: WorkerGlobalScope.onclose

2016-08-22 Thread Ms2ger 
On 22/08/16 10:03, Andrea Marchesini wrote:
> I'm planning to remove |partial interface WorkerGlobalScope {  attribute
> EventHandler onclose; }|.
> This EventHandler has been in our code base since ever but it's not part of
> the Workers spec and no other browses implement it.
> 
> In order to be compliant the spec, it's time to get rid of this.
> Any objection?
> 

... and we will stop dispatching the clone event.

This is https://bugzilla.mozilla.org/show_bug.cgi?id=790919

Really great to finally see it gone!

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

[Firefox Desktop] Issues found: August 15th to August 19th

2016-08-22 Thread Cornel Ionce 

Hi everyone,


Here's the list of new issues found and filed by the Desktop Release QA 
Team last week, *August 15 - August 19* (week 33).


Additional details on the team's priorities last week, as well as the 
plans for the current week are available at:


   https://public.etherpad-mozilla.org/p/DesktopManualQAWeeklyStatus



*RELEASE CHANNEL*
none

*BETA CHANNEL
*none

***AURORA CHANNEL*
none

*NIGHTLY CHANNEL*
ID  Summary Product Component   Is a regression 
Assigned to
1296322 
X button is misaligned on the permission dropdown
Firefox
General
YES  
Jonathan Kingston 
1296264 
	e10s / non-e10s different behavior when the previously selected option 
of a  element is chosen again

Core
DOM: Core & HTMLYES NOBODY
1296270 
e10s / non-e10s different mouse events behavior on  elements
	Core 	DOM: Core & HTML 	YES 
 
	NOBODY




*ESR CHANNEL*
none

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

Intent to unship: WorkerGlobalScope.onclose

2016-08-22 Thread Andrea Marchesini 
I'm planning to remove |partial interface WorkerGlobalScope {  attribute
EventHandler onclose; }|.
This EventHandler has been in our code base since ever but it's not part of
the Workers spec and no other browses implement it.

In order to be compliant the spec, it's time to get rid of this.
Any objection?
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform