1463843 - Save the initial commited value in the first update call after text property editor calls create

Status: RESOLVED FIXED

(DevTools :: Inspector: Rules, enhancement, P3)

Description

[Gabriel Luong [:gl] (ΦωΦ)]

We parse the property value and store it in this.committed when we are creating the text property editors https://searchfox.org/mozilla-central/source/devtools/client/inspector/rules/views/text-property-editor.js#188. However, this isn't really necessary since we will just this.prop.value elsewhere anyways. See https://searchfox.org/mozilla-central/source/devtools/client/inspector/rules/views/text-property-editor.js#922.

We can reduce the cost of parsing the css which is really expensive and unnecessary in this case by simply storing this.prop.value in this.committed when the value is created.

Comment 1

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: Bug 1463843 - Save the initial commited value in the first update call after text property editor calls create.

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

Comment 2

[Gabriel Luong [:gl] (ΦωΦ)]

Baseline https://treeherder.mozilla.org/#/jobs?repo=try&revision=cc6240adae8e11b60515449754719051ff0e85f1
Talos https://treeherder.mozilla.org/perf.html#/comparechooser?newProject=try&newRevision=fbadacef44b9935b76bd719c83f5df5831457567
Try https://treeherder.mozilla.org/#/jobs?repo=try&revision=8ff4faf1ccb5a706697c9bc50906a18aff0a870f

Comment 3

[Patrick Brosset <:pbro>]

(In reply to Gabriel [:gl] (ΦωΦ) from comment #0)
> However, this isn't really necessary
> since we will just this.prop.value elsewhere anyways. See
> https://searchfox.org/mozilla-central/source/devtools/client/inspector/rules/
> views/text-property-editor.js#922.
I'm confused though, cause we use this.committed.value here and we compare it to another parsed value:
https://searchfox.org/mozilla-central/rev/eb51d734a8c06ac5973291be9bcc74ed4e999bca/devtools/client/inspector/rules/views/text-property-editor.js#888-894
Isnt't that test going to start returning false in some cases now?

Comment 4

[Patrick Brosset <:pbro>]

Does you turning off the needinfo flag means you'll change this and post a new patch?

Comment 5

[Gabriel Luong [:gl] (ΦωΦ)]

(In reply to Patrick Brosset <:pbro> from comment #4)
> Does you turning off the needinfo flag means you'll change this and post a
> new patch?

Yes

Comment 6

[Gabriel Luong [:gl] (ΦωΦ)]

Comment on attachment 8980050 [details]
Bug 1463843 - Save the initial commited value in the first update call after text property editor calls create.

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

Comment 7

[Gabriel Luong [:gl] (ΦωΦ)]

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

Comment 8

[Gabriel Luong [:gl] (ΦωΦ)]

talos https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=try&originalRevision=34f91501ba1745e943f7d233e1050d7ccb6f1085&newProject=try&newRevision=99a121d26b9501230f30f9448c5184ee3052258d&originalSignature=f79b6a4f1f8f53326d3056f6c8008c0ff4de0a94&newSignature=f79b6a4f1f8f53326d3056f6c8008c0ff4de0a94&filter=inspector&framework=1

Comment 9

[Patrick Brosset <:pbro>]

Comment on attachment 8980050 [details]
Bug 1463843 - Save the initial commited value in the first update call after text property editor calls create.

https://reviewboard.mozilla.org/r/246216/#review257072

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6ce4f1c08f25
Save the initial commited value in the first update call after text property editor calls create. r=pbro

Comment 11

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/6ce4f1c08f25