Completed implementation of devtools' `getLayout`. (#3598)

[benschulz]

First PR; be gentle. ;)
Fixes #3598.

Notes

• I'm using serde_json for serialization, I will file a separate PR for converting the rest of devtools.
• As suggested I'm accessing the shared layout data from the directly from the script task.
  • Should the corresponding field be renamed ( _shared_data → shared_data )? I couldn't find any naming conventions, but I assume the underscore means "don't access".
  • Should any other documentation be updated?

[highfive]

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @Manishearth (or someone else) soon.

[highfive]

Warning

• These commits modify unsafe code. Please review it carefully!

[Ms2ger]

r? @pcwalton for the querying bits

[Manishearth]

The _ was probably used to circumvent an unused-variable lint or something.

[bors-servo]

☔ The latest upstream changes (presumably #7265) made this pull request unmergeable. Please resolve the merge conflicts.

[bors-servo]

☔ The latest upstream changes (presumably #7401) made this pull request unmergeable. Please resolve the merge conflicts.

[pcwalton]

The layout/style bits look good to me.

[jdm]

@pcwalton Is there any danger from reading the style data directly in the script task, rather than querying layout for it?

[bors-servo]

☔ The latest upstream changes (presumably #7488) made this pull request unmergeable. Please resolve the merge conflicts.

[bors-servo]

☔ The latest upstream changes (presumably #7509) made this pull request unmergeable. Please resolve the merge conflicts.

[jdm]

edit: (wrong issue for this comment)

[jdm]

I'll poke pcwalton to get an answer out of him, but these changes look pretty great to me!
-S-awaiting-review +S-needs-code-changes

---

Reviewed 7 of 7 files at r1, 6 of 6 files at r2, 1 of 1 files at r3, 1 of 1 files at r4, 1 of 1 files at r5.
Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

---

^{components/script/devtools.rs, line 141 [r1] (raw file):}
We can use &Node here instead.

---

^{components/script/dom/node.rs, line 63 [r1] (raw file):}
This shouldn't be necessary.

---

^{components/script/dom/node.rs, line 1073 [r1] (raw file):}
Just use query(*style) instead of calling deref above.

---

Comments from the review on Reviewable.io

[jdm]

-S-awaiting-review

---

Reviewed 2 of 2 files at r6.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.

---

Comments from the review on Reviewable.io

[pcwalton]

@jdm If there are off-main-thread CSS transitions running, they could be concurrently modifying style. So I think you need to handle the synchronization somehow. Since styles are atomically copy-on-write, you can satisfy this by simply cloning the style object.

[jdm]

@benschulz Instead of using borrow, could we create a style_for_script API that clones the style value under the hood and returns it?

[benschulz]

Done.

Can I ask, how this addresses the problem? I've only just found out about Arc::make_unique and am unfamiliar with its exact mechanics. Isn't it based on the assumption that if the reference count is one then there's only one (mut) reference to the Arc? And if so, don't we break that assumption via TrustedNodeAddresses? I.e. couldn't the concurrent transition have checked that the reference count is one and then proceed to modify the ComputedValues in-place while the script task increments the ref count and reads those values?

[pcwalton]

@benschulz Yes, you're right that that's a potential race. Perhaps while there are TrustedNodeAddresses about we should take out a reference representing them? @jdm, what do you think?

[frewsxcv]

@jdm It looks like this is waiting on an answer from you?

[jdm]

Taking this so I stop forgetting about it.

[jdm]

Rebased in #9661.