1431924 - URLParams::ParseInput is pointlessly called in PageThumbsProtocol::ParseProtocolURL

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P2)

Description

[Anthony Ramine [:nox]]

The PageThumbsProtocol::ParseProtocolURL method just extracts a single "url" parameter from the URL's query string. There is no need to build a temporary `URLParams` value for that.

Comment 1

[Anthony Ramine [:nox]]

Attachment: Bug 1431924 - Do not use URLParams::ParseInput in PageThumbsProtocol::ParseProtocolURL;

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

Comment 2

[Anthony Ramine [:nox]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4db5c932d4c3abd92fead6ef9c6020ab1c28eefb

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8944165 [details]
Bug 1431924 - Do not use URLParams::ParseInput in PageThumbsProtocol::ParseProtocolURL;

https://reviewboard.mozilla.org/r/214458/#review220746

r=me with the added documentation.

::: dom/url/URLSearchParams.h:56
(Diff revision 1)
>    };
>  
>    static bool
>    Parse(const nsACString& aInput, ForEachIterator& aIterator);
>  
> +  static bool

This needs documentation, both for the return value and for the behavior of aValue if the param is not found.

Also needs to document what happen if aInput has the param named aName multiple times.

::: dom/url/URLSearchParams.cpp:241
(Diff revision 1)
> +  const nsAString* mName;
> +  nsAString* mValue;

Is there a reason to use pointers, not references, here?  Using references would legt you use == and = instead of Equals() and Assign()...

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8944165 [details]
Bug 1431924 - Do not use URLParams::ParseInput in PageThumbsProtocol::ParseProtocolURL;

https://reviewboard.mozilla.org/r/214458/#review220748

Comment 5

[Anthony Ramine [:nox]]

Comment on attachment 8944165 [details]
Bug 1431924 - Do not use URLParams::ParseInput in PageThumbsProtocol::ParseProtocolURL;

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

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1caea602dadb
Do not use URLParams::ParseInput in PageThumbsProtocol::ParseProtocolURL; r=bz

Comment 7

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/1caea602dadb