1452947 - Get rid of useless PtrHolder / PtrHandle dance in style now that URIs are thread-safe.

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement)

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1452947: Remove useless PtrHolder / PtrHandle stuff now that URIs are thread-safe.

Review commit: https://reviewboard.mozilla.org/r/235298/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/235298/

Comment 2

[Code Review Bot [:reviewbot]]

Comment on attachment 8966571 [details]
Bug 1452947: Remove useless PtrHolder / PtrHandle stuff now that URIs are thread-safe.

https://reviewboard.mozilla.org/r/235298/#review240988


Code analysis found 2 defects in this patch:
 - 2 defects found by clang-tidy

You can run this analysis locally with:
 - `./mach static-analysis check path/to/file.cpp` (C/C++)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: layout/style/nsCSSValue.h:112
(Diff revision 1)
>    URLValueData(const nsAString& aString,
>                 already_AddRefed<URLExtraData> aExtraData);
>    URLValueData(ServoRawOffsetArc<RustString> aString,
>                 already_AddRefed<URLExtraData> aExtraData);
>    // Construct with the actual URI.
> -  URLValueData(already_AddRefed<PtrHolder<nsIURI>> aURI,
> +  URLValueData(already_AddRefed<nsIURI> aURI,

Error: Bad implicit conversion constructor for 'urlvaluedata' [clang-tidy: mozilla-implicit-constructor]

::: layout/style/nsCSSValue.h:115
(Diff revision 1)
>                 already_AddRefed<URLExtraData> aExtraData);
>    // Construct with the actual URI.
> -  URLValueData(already_AddRefed<PtrHolder<nsIURI>> aURI,
> +  URLValueData(already_AddRefed<nsIURI> aURI,
>                 const nsAString& aString,
>                 already_AddRefed<URLExtraData> aExtraData);
> -  URLValueData(already_AddRefed<PtrHolder<nsIURI>> aURI,
> +  URLValueData(already_AddRefed<nsIURI> aURI,

Error: Bad implicit conversion constructor for 'urlvaluedata' [clang-tidy: mozilla-implicit-constructor]

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8966571 [details]
Bug 1452947: Remove useless PtrHolder / PtrHandle stuff now that URIs are thread-safe.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/235298/diff/1-2/

Comment 4

[Code Review Bot [:reviewbot]]

Comment on attachment 8966571 [details]
Bug 1452947: Remove useless PtrHolder / PtrHandle stuff now that URIs are thread-safe.

https://reviewboard.mozilla.org/r/235298/#review241022


Code analysis found 2 defects in this patch:
 - 2 defects found by clang-tidy

You can run this analysis locally with:
 - `./mach static-analysis check path/to/file.cpp` (C/C++)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: layout/style/nsCSSValue.h:112
(Diff revision 2)
>    URLValueData(const nsAString& aString,
>                 already_AddRefed<URLExtraData> aExtraData);
>    URLValueData(ServoRawOffsetArc<RustString> aString,
>                 already_AddRefed<URLExtraData> aExtraData);
>    // Construct with the actual URI.
> -  URLValueData(already_AddRefed<PtrHolder<nsIURI>> aURI,
> +  URLValueData(already_AddRefed<nsIURI> aURI,

Error: Bad implicit conversion constructor for 'urlvaluedata' [clang-tidy: mozilla-implicit-constructor]

::: layout/style/nsCSSValue.h:115
(Diff revision 2)
>                 already_AddRefed<URLExtraData> aExtraData);
>    // Construct with the actual URI.
> -  URLValueData(already_AddRefed<PtrHolder<nsIURI>> aURI,
> +  URLValueData(already_AddRefed<nsIURI> aURI,
>                 const nsAString& aString,
>                 already_AddRefed<URLExtraData> aExtraData);
> -  URLValueData(already_AddRefed<PtrHolder<nsIURI>> aURI,
> +  URLValueData(already_AddRefed<nsIURI> aURI,

Error: Bad implicit conversion constructor for 'urlvaluedata' [clang-tidy: mozilla-implicit-constructor]

Comment 5

[Cameron McCormack (:heycam)]

Comment on attachment 8966571 [details]
Bug 1452947: Remove useless PtrHolder / PtrHandle stuff now that URIs are thread-safe.

https://reviewboard.mozilla.org/r/235298/#review241232

Comment 6

[Cameron McCormack (:heycam)]

Oh, relatedly I realize that we can get rid of all of the DefinitelyEqualXXX methods, since we can now call nsIURI::Equals OMT.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2aaddcc3fc77
Remove useless PtrHolder / PtrHandle stuff now that URIs are thread-safe. r=heycam

Comment 8

[Andreea Pavel [:apavel]]

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