Isolate socket code used by client / server

RESOLVED FIXED in Firefox 37

Status

RESOLVED FIXED
4 years ago
6 months ago

People

(Reporter: jryans, Assigned: jryans)

Tracking

unspecified
Firefox 37
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(7 attachments, 1 obsolete attachment)

(Assignee)

Description

4 years ago
Before I implement the TLS debugger sockets work for WiFi debugging, I think it will be clearer if we isolate the code handling sockets on either end to a single place.

Then, I'll extend these objects with secure versions.
(Assignee)

Updated

4 years ago
Assignee: nobody → jryans
Status: NEW → ASSIGNED
(Assignee)

Updated

4 years ago
Blocks: 1059001
(Assignee)

Updated

4 years ago
Depends on: 1097181
(Assignee)

Comment 1

4 years ago
Created attachment 8520864 [details]
MozReview Request: bz://1058997/jryans
Attachment #8520864 - Flags: review?(past)
(Assignee)

Comment 2

4 years ago
/r/439 - Bug 1058997 - Part 1: Extract socket connect and listen. r=past
/r/441 - Bug 1058997 - Part 2: Debugger socket style cleanup. r=past
/r/443 - Bug 1058997 - Part 3a: Move allowConnection to socket listener. r=past
/r/445 - Bug 1058997 - Part 3b: Update non-trivial security prompts. r=past
/r/447 - Bug 1058997 - Part 3c: Remove always allow init calls from tests. r=past
/r/449 - Bug 1058997 - Part 3d: Restore always allow in listener tests. r=past
/r/451 - Bug 1058997 - Part 4: Only one DebuggerServer per loader. r=past

Pull down these commits:

hg pull review -r 11a001b4a826a67a304a9fe027b057a804367ac8
(Assignee)

Comment 3

4 years ago
Panos, the main theme of this work to extract the debugger's socket related code for future extension to TLS sockets.  It also enables easier comprehension about debugger socket security, since there is less code to worry about in a smaller file.

This is my first time posting multiple commits with MozReview, so let me know if there issues!  After you click one link in this bug to MozReview, I would strongly suggest clicking each of the "/r/439" style links next to each of the commits in the "Description" box.  That will allow you to see a diff from each commit.  The initial link is a "parent" review, which presents all of the changes from all the commits, which could be hard to review.

See the user guide[1] for more details on the MozReview features as a reviewer.  Also, if you can't login to MozReview and you use Persona with Bugzilla, check my comment[2] about this.

[1]: https://mozilla-version-control-tools.readthedocs.org/en/latest/mozreview/reviewboard.html#reviewing-code
[2]: https://bugzilla.mozilla.org/show_bug.cgi?id=1094968#c5
https://reviewboard.mozilla.org/r/447/#review619

r=me with the following fixed.

::: toolkit/devtools/server/tests/unit/test_memory_footprint.js
(Diff revision 1)
> -  check_footprint("DebuggerServer.init()", 500);
> +  check_footprint("DebuggerServer.init();

Ahem. Find & replace gone wild?
Attachment #8520864 - Flags: review?(past) → review+
(Assignee)

Comment 13

4 years ago
https://reviewboard.mozilla.org/r/447/#review641

> Ahem. Find & replace gone wild?

Good catch!
(Assignee)

Updated

4 years ago
Attachment #8520864 - Flags: review+
(Assignee)

Comment 14

4 years ago
/r/439 - Bug 1058997 - Part 1: Extract socket connect and listen. r=past
/r/441 - Bug 1058997 - Part 2: Debugger socket style cleanup. r=past
/r/443 - Bug 1058997 - Part 3a: Move allowConnection to socket listener. r=past
/r/445 - Bug 1058997 - Part 3b: Update non-trivial security prompts. r=past
/r/447 - Bug 1058997 - Part 3c: Remove always allow init calls from tests. r=past
/r/449 - Bug 1058997 - Part 3d: Restore always allow in listener tests. r=past
/r/451 - Bug 1058997 - Part 4: Only one DebuggerServer per loader. r=past

Pull down these commits:

hg pull review -r c42ff82b5b2ca241eb0bedb17080684d2b128561
(Assignee)

Updated

4 years ago
Attachment #8520864 - Flags: review+

Comment 20

4 years ago
Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/12a3b0250e2e4d3f9a60871ae8aae91ce7ec25a3
Bug 1058997 - Part 3c: Remove always allow init calls from tests. r=past
(Assignee)

Comment 21

4 years ago
Comment on attachment 8520864 [details]
MozReview Request: bz://1058997/jryans
Attachment #8520864 - Attachment is obsolete: true
Attachment #8618284 - Flags: review+
Attachment #8618285 - Flags: review+
Attachment #8618286 - Flags: review+
Attachment #8618287 - Flags: review+
Attachment #8618288 - Flags: review+
Attachment #8618289 - Flags: review+
Attachment #8618290 - Flags: review+
(Assignee)

Comment 22

4 years ago
Created attachment 8618284 [details]
MozReview Request: Bug 1058997 - Part 3d: Restore always allow in listener tests. r=past
(Assignee)

Comment 23

4 years ago
Created attachment 8618285 [details]
MozReview Request: Bug 1058997 - Part 4: Only one DebuggerServer per loader. r=past
(Assignee)

Comment 24

4 years ago
Created attachment 8618286 [details]
MozReview Request: Bug 1058997 - Part 1: Extract socket connect and listen. r=past
(Assignee)

Comment 25

4 years ago
Created attachment 8618287 [details]
MozReview Request: Bug 1058997 - Part 2: Debugger socket style cleanup. r=past
(Assignee)

Comment 26

4 years ago
Created attachment 8618288 [details]
MozReview Request: Bug 1058997 - Part 3a: Move allowConnection to socket listener. r=past
(Assignee)

Comment 27

4 years ago
Created attachment 8618289 [details]
MozReview Request: Bug 1058997 - Part 3b: Update non-trivial security prompts. r=past
(Assignee)

Comment 28

4 years ago
Created attachment 8618290 [details]
MozReview Request: Bug 1058997 - Part 3c: Remove always allow init calls from tests. r=past

Updated

6 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.