1169064 - Refactor the ObjectActor from script.js to object.js

Status: RESOLVED FIXED

(DevTools :: Debugger, defect)

Comment 1

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

Attachment: 1169064-part-1.patch [WIP]

Comment 2

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

Attachment: Part 1: Move ObjectActor to object.js

Comment 3

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

Attachment: Part 1: Move ObjectActor to object.js [1.0]

Just an observation that will be addressed in another patch as part of this bug:
- createEnvironmentActor is implemented the same in both webconsole.js and script.js. I think we can just have one implementation in object.js.

Comment 4

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

ObjectActor.onDefinitionSite also doesn't use its aRequest argument.

Comment 5

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Comment on attachment 8611575 [details] [diff] [review]
Part 1: Move ObjectActor to object.js [1.0]

Review of attachment 8611575 [details] [diff] [review]:
-----------------------------------------------------------------

Very nicely done :)

Comment 6

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

Attachment: Part 2: Formatted object.js and removed unused protocol request arguments

Notable Changes:
- Renamed all aString arguments
- Formatted to ensure 80 chars per line
- One instance where destructuring {obj, hook} simplified the code
- Removed unused request arguments

Comment 7

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

Attachment: Part 2: Formatted object.js and removed unused protocol request arguments [1.0]

Undid the destructuring of the ObjectActor in GenericObject.

Comment 8

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

Attachment: Part 1: Move ObjectActor to object.js [2.0]

s/decrementGriphDepth/decrementGripDepth

Comment 9

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

Attachment: Part 3: Refactor LongStringActor, createValueGrip, stringIsLong and longStripGrip from script.js to object.js

Comment 10

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Comment on attachment 8612181 [details] [diff] [review]
Part 3: Refactor LongStringActor, createValueGrip, stringIsLong and longStripGrip from script.js to object.js

Review of attachment 8612181 [details] [diff] [review]:
-----------------------------------------------------------------

This is _so_ much better! Thanks!

Comment 11

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

Attachment: Part 1: Move ObjectActor to object.js [3.0]

Added DevToolsUtils require in webconsole.js

Comment 12

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

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

Comment 13

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

Attachment: Part 1: Move ObjectActor to object.js [4.0]

Refactor PropertyIteratorActor from script.js to object.js

Comment 14

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

Attachment: Part 3: Refactor LongStringActor, createValueGrip, stringIsLong and longStripGrip from script.js to object.js

Part 3: Fixed createValueGrip in webaudio.js

Comment 15

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

Attachment: Part 3: Refactor LongStringActor, createValueGrip, stringIsLong and longStripGrip from script.js to object.js [1.0]

Comment 16

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

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

Comment 17

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

Attachment: Part 3: Refactor LongStringActor, createValueGrip, stringIsLong and longStripGrip from script.js to object.js [2.0]

Comment 18

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

Attachment: Part 3: Refactor LongStringActor, createValueGrip, stringIsLong and longStripGrip from script.js to object.js [3.0]

Comment 19

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

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

Comment 20

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

Attachment: Part 3: Refactor LongStringActor, createValueGrip, stringIsLong and longStripGrip from script.js to object.js [4.0]

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

Comment 21

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Comment on attachment 8613007 [details] [diff] [review]
Part 3: Refactor LongStringActor, createValueGrip, stringIsLong and longStripGrip from script.js to object.js [4.0]

Review of attachment 8613007 [details] [diff] [review]:
-----------------------------------------------------------------

Yay! Great job!

Comment 22

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

Attachment: Part 3: Refactor LongStringActor, createValueGrip, stringIsLong and longStripGrip from script.js to object.js [5.0]

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

Comment 23

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

What's different in this revision? Does it warrant rereview? I'm assuming it must be something "big", b/c you reflagged me, but it isn't obvious to me looking at the patch.

Comment 24

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

Unfortunately, the changes aren't big and it simply readds the actor deletion to LongStringActor#onRelease. I will carry over the r+.

Comment 25

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/5fa3d208ad8e
https://hg.mozilla.org/integration/fx-team/rev/a223489c052d
https://hg.mozilla.org/integration/fx-team/rev/c5d7c436a02b

Comment 26

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5fa3d208ad8e
https://hg.mozilla.org/mozilla-central/rev/a223489c052d
https://hg.mozilla.org/mozilla-central/rev/c5d7c436a02b