1315155 - use nsStyleImageRequest in nsStyleContentData

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Cameron McCormack (:heycam)]

The last style struct image values that still use imgRequestProxy directly are those in nsStyleContentData objects.  This bug is for changing that to nsStyleImageRequest.

Comment 1

[Cameron McCormack (:heycam)]

Attachment: Bug 1315155 - Part 1: Encapsulate nsStyleContentData.

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

Comment 2

[Cameron McCormack (:heycam)]

Attachment: Bug 1315155 - Make nsStyleContentData use nsStyleImageRequest for images.

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

Comment 3

[Cameron McCormack (:heycam)]

Attachment: Regenerate bindings.

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

Comment 4

[Cameron McCormack (:heycam)]

Attachment: Stop setting the now-removed nsStyleContentData::mImageTracked.

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

Comment 5

[Manish Goregaokar [:manishearth]]

Comment on attachment 8807408 [details]
Stop setting the now-removed nsStyleContentData::mImageTracked.

https://reviewboard.mozilla.org/r/90546/#review90268

Comment 6

[Xidorn Quan [:xidorn] UTC+11]

Comment on attachment 8807405 [details]
Bug 1315155 - Part 1: Encapsulate nsStyleContentData.

https://reviewboard.mozilla.org/r/90540/#review90292

r=me with the following issues addressed

::: layout/style/nsComputedDOMStyle.cpp:1130
(Diff revision 1)
> -          }
> +        }
> -          else {
> +        else {
> -            str.AppendLiteral("counters(");
> +          str.AppendLiteral("counters(");
> -          }
> +        }
> -          // WRITE ME
> +        // WRITE ME
> -          nsCSSValue::Array *a = data.mContent.mCounters;
> +        nsCSSValue::Array *a = data.GetCounters();

As far as you are here, `nsCSSValue::Array* a = ...` instead.

::: layout/style/nsStyleStruct.h
(Diff revision 1)
>    {
> +    MOZ_ASSERT(mType == eStyleContentType_Uninitialized,
> +               "should only initialize nsStyleContentData once");
>      MOZ_ASSERT(!mImageTracked,
>                 "Setting a new image without untracking the old one!");
> -    MOZ_ASSERT(mType == eStyleContentType_Image, "Wrong type!");

Forgot to set `mType` here?

Comment 7

[Xidorn Quan [:xidorn] UTC+11]

Comment on attachment 8807406 [details]
Bug 1315155 - Make nsStyleContentData use nsStyleImageRequest for images.

https://reviewboard.mozilla.org/r/90542/#review90300

Looks good.

::: layout/style/nsStyleStruct.h:3199
(Diff revision 1)
> +    MOZ_ASSERT(mType == eStyleContentType_Image);
> +    return mContent.mImage->get();

Probably just `GetImageRequest()->get()`.

::: layout/style/nsStyleStruct.h:3243
(Diff revision 1)
> -               "Setting a new image without untracking the old one!");
> -    NS_IF_ADDREF(mContent.mImage = aRequest);
> +    mContent.mImage = aRequest;
> +    mContent.mImage->AddRef();

`do_AddRef(aRequest).take()` is probably shorter.

Comment 8

[Cameron McCormack (:heycam)]

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

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0433dc15a629
Part 1: Encapsulate nsStyleContentData. r=xidorn
https://hg.mozilla.org/integration/mozilla-inbound/rev/48004b990f96
Part 2: Make nsStyleContentData use nsStyleImageRequest for images. r=xidorn

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0433dc15a629
https://hg.mozilla.org/mozilla-central/rev/48004b990f96