Closed Bug 1425318 Opened 2 years ago Closed 2 years ago

Allow calling NS_MutateURI.Finalize(nsIURI**)

Categories

(Core :: Networking, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: valentin, Assigned: valentin)

References

Details

(Whiteboard: [necko-triaged])

Attachments

(3 files)

This is useful so methods can do something like this:

> nsresult ChangeURI(nsIURI* aURI, nsIURI** changedURI) {
>    return NS_MutateURI(uri)
>             .SetRef(NS_LITERAL_CSTRING("test"));
>             .Finalize(changedURI);
>}
Comment on attachment 8936957 [details]
Bug 1425318 - Allow calling NS_MutateURI.Finalize(nsIURI**)

https://reviewboard.mozilla.org/r/207648/#review213920

::: netwerk/base/nsIURIMutator.idl:319
(Diff revision 2)
> +    NS_ENSURE_SUCCESS(mStatus, mStatus);
> +    nsCOMPtr<nsIURI> uri;
> +    mStatus = mMutator->Finalize(getter_AddRefs(uri));
> +    nsCOMPtr<C> result = do_QueryInterface(uri, &mStatus);
> +    result.forget(aURI);
> +    return mStatus;

some blank lines for better readability would be good, something like:

nsresult Finalize(C** aURI)
{
    NS_ENSURE_SUCCESS(mStatus, mStatus);

    nsCOMPtr<nsIURI> uri;
    mStatus = mMutator->Finalize(getter_AddRefs(uri));

    nsCOMPtr<C> result = do_QueryInterface(uri, &mStatus);
    result.forget(aURI);

    return mStatus;
}

::: netwerk/test/gtest/TestStandardURL.cpp
(Diff revision 2)
> -         .SetScheme(NS_LITERAL_CSTRING("https"))
> -         .Finalize(url);
> -  ASSERT_EQ(rv, NS_OK);
> -  ASSERT_EQ(url->GetSpec(out), NS_OK);
> -  ASSERT_TRUE(out == NS_LITERAL_CSTRING("https://mozilla.org/path?query#ref"));
> -}

honestly, I would leave this in.  it's not bad to test both apis, you never know when the upper level one goes away leaving this untested
Attachment #8936957 - Flags: review?(honzab.moz) → review+
Comment on attachment 8937301 [details]
Bug 1425318 - The second call to NS_MutateURI.Finalize should fail

https://reviewboard.mozilla.org/r/208004/#review213928
Attachment #8937301 - Flags: review?(honzab.moz) → review+
Comment on attachment 8937302 [details]
Bug 1425318 - Add MOZ_MUST_USE to methods in nsIURIMutator.idl

https://reviewboard.mozilla.org/r/208006/#review213932
Attachment #8937302 - Flags: review?(honzab.moz) → review+
Pushed by valentin.gosu@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/bf4b28e3e513
Allow calling NS_MutateURI.Finalize(nsIURI**) r=mayhemer
https://hg.mozilla.org/integration/autoland/rev/b0f9fe04079c
The second call to NS_MutateURI.Finalize should fail r=mayhemer
https://hg.mozilla.org/integration/autoland/rev/2166b03540e9
Add MOZ_MUST_USE to methods in nsIURIMutator.idl r=mayhemer
You need to log in before you can comment on or make changes to this bug.