786503 - Mark nsSMILInstanceTime::mVisited as mutable, instead of doing const_cast dance

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Daniel Holbert [:dholbert]]

nsSMILInstanceTime::mVisited is treated as a mutable member-var (and it has comments indicating that that's intentional).

We should just declare it as mutable, so that we don't need the const_cast dance when we modify it.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: fix

Comment 2

[Daniel Holbert [:dholbert]]

I'm assuming that all the compilers we care about support "mutable", because there are other instances of that keyword in our codebase.

Just to be sure, here's a try run (builds-only):
  https://tbpl.mozilla.org/?tree=Try&rev=91016b3dbb4c

Comment 3

[Brian Birtles (:birtles)]

Comment on attachment 656270 [details] [diff] [review]
fix

Review of attachment 656270 [details] [diff] [review]:
-----------------------------------------------------------------

I think the reason we didn't use 'mutable' to begin with was that not all our compilers supported it at the time. Digging up an old version of the C++ portability guide[1] I notice item 31 says "Don't use mutable". That's most likely why we didn't. I agree we should be safe now--the current coding style doesn't make any mention of it[2]--so thanks for bringing this up to date!

[1] http://www.literateprogramming.com/portablecpp.pdf
[2] https://developer.mozilla.org/en-US/docs/Developer_Guide/Coding_Style

::: content/smil/nsSMILInstanceTime.cpp
@@ +168,5 @@
>      return true;
>  
>    // mVisited is mutable
> +  mozilla::AutoRestore<bool> setVisited(mVisited);
> +  mVisited = true;

I think we can remove the comment, "mVisited is mutable" as well.

Comment 4

[Daniel Holbert [:dholbert]]

Thanks! Yeah, we use "mutable" in a bunch of places now:
https://mxr.mozilla.org/mozilla-central/search?string=+mutable+&case=on&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central
so I think we're safe to use it here, too.

> I think we can remove the comment, "mVisited is mutable" as well.

Cool -- removed.

Pushed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2f34d5cff8a0

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/2f34d5cff8a0