Re: Coding style for C++ enums

2016-04-11 Thread Seth Fowler
I'd honestly prefer to see this discussion drag on a little longer. The original email was on Friday, so given that most participants on this list aren't actively debating C++ coding style on the weekend, we've actually had less than one full working day of discussion on this issue so far. Let me

Re: Coding style for C++ enums

2016-04-11 Thread Seth Fowler
> All caps actually makes long names *longer* because you now need to > add underscores to separate words, rather than using sentence case. > For example eSentenceCase is the same length as SENTENCE_CASE, but if > you tack on another word the "e" prefix is shorter. Presumably this > line-wrapping i

Re: Coding style for C++ enums

2016-04-11 Thread Seth Fowler
I agree in the extreme with Jeff here. Foo::Bar, in an expression context, cannot be anything other than a value. For me, at least, I have never noticed this adding any overhead in reviews - it’s immediately obvious what’s happening. I suspect that if it *does* add overhead for you, you’ll find

Re: Use of 'auto'

2015-08-03 Thread Seth Fowler
On Mon, Aug 3, 2015 at 6:07 PM, Jonas Sicking wrote: How would you make a factory function like the above fail? Returning > an nsresult will make it not much better than NS_NewFoo() pattern. > Returning null won't let you indicate a useful error code (though > maybe there's no such thing as usefu

Re: Switch to Google C++ Style Wholesale (was Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++)

2015-07-14 Thread Seth Fowler
> On Jul 14, 2015, at 8:23 AM, Benjamin Smedberg wrote: > > * no more ns prefix Are people still creating new classes with an ’ns’ prefix? Surely this is something we can drop right away, at least for new code. Much of the codebase already does not use this style. We have namespaces now, afte

Re: mozilla::TemporaryRef is gone; please use already_AddRefed

2015-07-09 Thread Seth Fowler
> On Jul 8, 2015, at 8:34 AM, Michael Layzell wrote: > > We're already working on a static analysis in > https://bugzilla.mozilla.org/show_bug.cgi?id=1180993 which will prevent code > like that from passing try. > > Hopefully that will help with your concerns. Yes, that will solve the proble

Re: mozilla::TemporaryRef is gone; please use already_AddRefed

2015-07-08 Thread Seth Fowler
I brought this up on IRC, but I think this is worth taking to the list: Replacing TemporaryRef with already_AddRefed has already caused at least one leak because already_AddRefed does not call Release() on the pointer it holds if nothing takes ownership of that pointer before its destructor runs

Re: Private members of ref counted classes and lambdas

2015-06-04 Thread Seth Fowler
> On Jun 4, 2015, at 11:17 AM, Daniel Holbert wrote: > > You may be interested in this thread from a few months back: > "Proposal to ban the usage of refcounted objects inside C++ lambdas in > Gecko" > https://groups.google.com/d/msg/mozilla.dev.platform/Ec2y6BWKrbM/xpHLGwJ337wJ > > (Not sure i

Re: Use of 'auto'

2015-06-02 Thread Seth Fowler
> On Jun 2, 2015, at 1:59 PM, L. David Baron wrote: > > Remember that if 'auto' is forbidden, people might get around it by > not using a variable at all (and instead repeating the expression) > rather than by writing the type explicitly on the variable... and > this doesn't provide type informa

Re: A question about do_QueryInterface()

2015-04-30 Thread Seth Fowler
> On Apr 30, 2015, at 12:09 PM, Joshua Cranmer 🐧 wrote: > > do_QueryInterface is the equivalent of a type-checked downcast, e.g. > (ClassName)foo in Java. (Regular C++ downcasts are not dynamically > type-checked). do_QueryInterface is, in other words, essentially equivalent to dynamic_cast

Re: Proposal to alter the coding style to not require the usage of 'virtual' where 'override' is used

2015-04-27 Thread Seth Fowler
I completely support this. Let the needless verbosity end! - Seth > On Apr 27, 2015, at 12:48 PM, Ehsan Akhgari wrote: > > Right now, our coding style requires that both the virtual and override > keywords to be specified for overridden virtual functions. A few things > have changed since we d

We now throttle requestAnimationFrame for offscreen iframes

2015-04-21 Thread Seth Fowler
Hi all, Bug 1145439 has landed, which means that we now throttle requestAnimationFrame for offscreen iframes. This should give us significant benefits in terms of CPU and energy usage for pages with iframes that do animation - think HTML5 ads. One test, on areweflashyet.com, showed a 50% improvem

Re: Can we make try builds default to a different profile than Nightly/Aurora/Beta/Release builds?

2015-04-08 Thread Seth Fowler
> On Apr 8, 2015, at 8:37 PM, Nicholas Alexander wrote: > > It's quite important for Firefox for Android that try builds look and behave > exactly like Nightly builds. On Android, it's not really possible to > move/copy/duplicate profiles, and it's rocket science to do it across > products (

Re: Can we make try builds default to a different profile than Nightly/Aurora/Beta/Release builds?

2015-04-08 Thread Seth Fowler
sed. > > Gavin > > On Wed, Apr 8, 2015 at 12:28 PM, L. David Baron wrote: >> On Wednesday 2015-04-08 12:08 -0700, Seth Fowler wrote: >>> I think one way we could reduce the burden on users would be to just make >>> try builds default to a different profile than

Can we make try builds default to a different profile than Nightly/Aurora/Beta/Release builds?

2015-04-08 Thread Seth Fowler
I sometimes ask users to test a try build to verify that the build fixes their problem. This is an important tool, especially when the problem is hard to reproduce or when the definition of “fix” is a bit nebulous (as it sometimes is in performance-related bugs). However, I don’t want to run th

Re: PSA: mozilla::Pair is now a little more flexible

2015-03-15 Thread Seth Fowler
> On Mar 15, 2015, at 6:26 PM, Joshua Cranmer 🐧 wrote: > In general, std::pair should be preferred over mozilla::Pair unless you need > the empty type optimization. If that’s the case, perhaps we should rename it to e.g. mozilla::CompactPair? It’s current name strongly suggests that it should

Re: PSA: mozilla::Pair is now a little more flexible

2015-03-15 Thread Seth Fowler
> On Mar 15, 2015, at 12:01 PM, Eric Rescorla wrote: > > I'm not sure I want to get in a long argument about this, but I'm not > convinced > this is good advice. I don’t really care what we do - keep in mind, I had nothing to do with introducing mozilla::Pair - but I think that we should reco

Re: PSA: mozilla::Pair is now a little more flexible

2015-03-13 Thread Seth Fowler
> On Mar 13, 2015, at 6:14 AM, Eric Rescorla wrote: > > Sorry if this is a dumb question, but it seems like std::pair is fairly > widely used in our > code base. Can you explain the circumstances in which you think we should be > using mozilla::Pair instead? It’s not at all a dumb question. Th

PSA: mozilla::Pair is now a little more flexible

2015-03-12 Thread Seth Fowler
I thought I’d let everyone know that bug 1142366 and bug 1142376 have added some handy new features to mozilla::Pair. In particular: - Pair objects are now movable. (It’s now a requirement that the underlying types be movable too. Every existing use satisfied this requirement.) - Pair objects a

Re: Chrome removed support for multipart/x-mixed-replace documents. We should too.

2015-03-12 Thread Seth Fowler
On Mar 12, 2015, at 4:42 PM, Boris Zbarsky wrote: > > On 3/12/15 6:37 PM, Seth Fowler wrote: >> They made main resources that use multipart/x-mixed-replace trigger >> downloads instead of being displayed. > > So what gets downloaded is the entire mixed stream, right? Y

Re: What are your pain points when running unittests?

2015-03-12 Thread Seth Fowler
> On Mar 12, 2015, at 4:17 PM, Gijs Kruitbosch wrote: > It'd be better if we could more easily get more information about failures as > they happened on infra (replay debugging stuff a la what roc has worked on, > or better logs, or somehow making it possible to remote-debug the infra > machin

Re: Chrome removed support for multipart/x-mixed-replace documents. We should too.

2015-03-12 Thread Seth Fowler
On Mar 12, 2015, at 3:52 PM, Ehsan Akhgari wrote: >> Looks like this patch landed in Chromium on June 13, 2013 and has stuck >> since then, so removing it has not resulted in a disaster for Chrome. With >> so few people using multipart/x-mixed-replace, and since now both IE and >> Chrome do not

Chrome removed support for multipart/x-mixed-replace documents. We should too.

2015-03-12 Thread Seth Fowler
Chrome removed support for multipart/x-mixed-replace main resources in this issue: https://code.google.com/p/chromium/issues/detail?id=249132 Here’s their explanation: > This feature is extremely rarely used by web sites and is the s

Re: UI Workers

2015-02-23 Thread Seth Fowler
In Gecko, we will only decode images on the main thread: (1) If they’re very small and this is the first time we’re decoding them and our heuristics suggest that decoding them quickly is important. (2) If it’s absolutely required because of a synchronous API, e.g. canvas.drawImage. We’ve been g

Re: Reversely iterating nsTArray

2015-01-28 Thread Seth Fowler
Yes, that’s a good idea! I like this even better than the original proposal. - Seth > On Jan 28, 2015, at 1:59 AM, Xidorn Quan wrote: > > On Wed, Jan 28, 2015 at 7:39 PM, Cameron McCormack > wrote: > >> Xidorn Quan: >>> I asked a question in #developers that what is the

Re: Reversely iterating nsTArray

2015-01-28 Thread Seth Fowler
time you type ’T’ instead of ‘uint32_t’ or whatever you’d have used otherwise. =) - Seth > On Jan 27, 2015, at 11:47 PM, Kyle Huey wrote: > > On Wed, Jan 28, 2015 at 1:18 PM, Seth Fowler <mailto:s...@mozilla.com>> wrote: > Sounds good! +1 from me. > > Bike sheddi

Re: Reversely iterating nsTArray

2015-01-27 Thread Seth Fowler
Sounds good! +1 from me. Bike shedding: - Make Range() and ReverseRange() templates, so you can use them with any type that supports the appropriate operators. This also implies removing ‘Integer’ from their names, I think. - It’d be nice to add a constructor that supports specifying both the

Re: PSA: Non-unified builds no longer occurring on central/inbound and friends

2015-01-15 Thread Seth Fowler
It wouldn’t be true of, say, a mach argument specifying directories that should be built non-unified. Not that it matters nearly as much now that we’ve made the decision not to support non-unified builds. > On Jan 15, 2015, at 3:36 PM, Ehsan Akhgari wrote: > > On 2015-01-15 5:57

Re: PSA: Non-unified builds no longer occurring on central/inbound and friends

2015-01-15 Thread Seth Fowler
What’s a bit unfortunate about these approaches is that you can accidentally qref them into your patch. I’ve managed to do so repeatedly. - Seth > On Jan 15, 2015, at 2:52 PM, Mike Hommey wrote: > > On Thu, Jan 15, 2015 at 02:11:16PM -0500, Benjamin Kelly wrote: >> For what its worth, you can

Re: PSA: multipart/x-mixed-replace images will soon be restricted to MJPEG

2015-01-02 Thread Seth Fowler
> On Jan 2, 2015, at 7:16 AM, L. David Baron wrote: >> IMHO, "I haven't seen" is a weak argument. When we remove features >> that are exposed to the web in some form, it's always a good idea to >> gather some telemetry first so that we know what the actual impact >> will probably be (there is som

Re: PSA: multipart/x-mixed-replace images will soon be restricted to MJPEG

2014-12-18 Thread Seth Fowler
> On Dec 17, 2014, at 10:08 PM, James May wrote: > * Is there telemetry for this? There isn’t telemetry specifically for non-MJPEG usage of multipart/x-mixed-replace images. Given that I’ve never seen them used outside of our test suite, I don’t think we need telemetry to make this particular

PSA: multipart/x-mixed-replace images will soon be restricted to MJPEG

2014-12-16 Thread Seth Fowler
Right now, ImageLib provides very general support for multipart/x-mixed-replace images. Each part may contain a different image format (we may even switch between raster and vector images from one part to the next), individual parts may be animated, and each part may have a different size. This

Re: Proposal: Change the coding style guide to allow an 'o' prefix to indicate out-params

2014-12-05 Thread Seth Fowler
ying out-parameters with a convention. > > Cheers, > Botond > > - Original Message - >> From: "Nicholas Nethercote" >> To: "Robert O'Callahan" >> Cc: "dev-platform" , "Seth Fowler" >> >> Sent: Friday,

Re: Proposal: Change the coding style guide to allow an 'o' prefix to indicate out-params

2014-12-05 Thread Seth Fowler
> On Dec 4, 2014, at 10:53 AM, L. David Baron wrote: > > If we adopt this convention, should we also have a convention for an > in-out param? (If so, what?) I considered proposing the ‘io’ prefix for this, but I was worried it would lead to bike shedding. =) I’d definitely support that, thoug

Re: Proposal: Change the coding style guide to allow an 'o' prefix to indicate out-params

2014-12-05 Thread Seth Fowler
> On Dec 4, 2014, at 11:02 AM, Eric Rescorla wrote: > In contrast, Seth's suggestion would be an extremely clear indication > that a parameter is an outparam. > > Yes, and because it's just a convention and not compiler enforced it can > also be wrong. I don’t know of any realistic, usable way

Re: Proposal: Change the coding style guide to allow an 'o' prefix to indicate out-params

2014-12-05 Thread Seth Fowler
> On Dec 4, 2014, at 11:28 AM, Robert O'Callahan wrote: > > I think this would be a slight improvement but the place where I really want > out-parameters to be visible is at the caller, not the callee. Agreed! The simplest way to achieve that in C++, though, is to use pointer arguments (so th

Proposal: Change the coding style guide to allow an 'o' prefix to indicate out-params

2014-12-04 Thread Seth Fowler
I’d like to change the coding style guide to let us make out-params more obvious by using an ‘o’ prefix for their name instead of an ‘a’. For example, nsresult Modify(int aCount, size_t aSize, char* oResult); This will make it clear just from the declaration of a function or method which parame

Re: Proposal: Standardize initializer list formatting in our coding style guide

2014-12-03 Thread Seth Fowler
2014 at 8:54 AM, Seth Fowler wrote: >> So I noticed that we don’t say anything about initializer list formatting in >> our coding style guide. I’d like to propose that we standardize this >> formatting: >> >> Foo::Foo(int aBar, char aBaz) >> : mBar(aBar

Proposal: Standardize initializer list formatting in our coding style guide

2014-12-03 Thread Seth Fowler
So I noticed that we don’t say anything about initializer list formatting in our coding style guide. I’d like to propose that we standardize this formatting: Foo::Foo(int aBar, char aBaz) : mBar(aBar) , mBaz(aBaz) { …. } In other words, we should list items in the initializer list one per

PSA: You can now use Maybe with the Auto helpers for Mutex and Monitor

2014-11-10 Thread Seth Fowler
In bug 1091921, we got support for using Maybe with the Auto helpers for Mutex and Monitor - things like MutexAutoLock and MonitorAutoEnter. This supports a pattern for optionally acquiring a RAII resource that I first saw used in the JavaScript engine, and which I’ve found very useful since. Fo

Re: power use on Yosemite

2014-11-10 Thread Seth Fowler
On Nov 3, 2014, at 1:44 PM, rviti...@mozilla.com wrote: > In particular Facebook, which practically appears in any top 10 list, had > (has?) a serious power bug that caused FF to render a hidden spinning wheel. > Because of this single bug any power benchmark performed by the press, which > was

Re: The worst piece of Mozilla code

2014-10-17 Thread Seth Fowler
Thank you Bobby and Josh for all your work to improve ImageLib! I’m pushing hard on making it better still (with a lot of help from folks like Timothy Nikkel and Michael Wu). Hopefully next time we try to decide what the worst piece of Mozilla code is, ImageLib won’t be a candidate. =) - Seth

Re: using LLDB to debug Gecko

2014-01-21 Thread Seth Fowler
Really nice Cameron, thanks for doing this! Now if only I could get LLDB to work with GUD in emacs. If anyone has managed that, I'd love to hear about it. There was some elisp in the LLDB repo at one time but last I checked it had been removed and the last version that existed didn't work with

Re: Mozilla style guide issues, from a JS point of view

2014-01-17 Thread Seth Fowler
On Jan 7, 2014, at 3:22 PM, Cameron McCormack wrote: > Patrick McManus wrote: >> Typically I have to choose between >> 1] 80 columns >> 2] descriptive and non-abbreviated naming >> 3] displaying a logic block without scrolling >> >> to me, #1 is the least valuable. > > Thoroughly agree. I

Re: Mozilla style guide issues, from a JS point of view

2014-01-17 Thread Seth Fowler
Chiming in a little late: On Jan 6, 2014, at 6:35 PM, Joshua Cranmer 🐧 wrote: > I find prefixing member variables with 'm' to be useful, although I dislike > using it in POD-ish structs where all the members are public. Fully agreed, and IMO the style guide should be changed to include this.

Re: try: -p all considered harmful?

2012-09-28 Thread Seth Fowler
On Sep 28, 2012, at 6:28 PM, Boris Zbarsky wrote: > The point is that the patches that _do_ need to land urgently are blocked on > lack of resources because people are wasting too many cycles with unnecesary > try pushes. This sounds like a notion of priority might be helpful. - Seth

Re: try: -p all considered harmful?

2012-09-28 Thread Seth Fowler
I am new to the use of the try build system and so this is not a suggestion informed by much experience with this particular setup, but time spent in other environments has shown me that running these sorts of test builds on all platforms can sometimes be quite useful. I'd suggest a variation on