Closed
Bug 1010750
Opened 10 years ago
Closed 10 years ago
Cannot connect remote dev tools to fennec nightly
Categories
(DevTools :: General, defect)
Tracking
(firefox31 unaffected, firefox32- affected)
RESOLVED
FIXED
Firefox 32
Tracking | Status | |
---|---|---|
firefox31 | --- | unaffected |
firefox32 | - | affected |
People
(Reporter: Margaret, Assigned: jryans)
References
(Blocks 1 open bug)
Details
(Keywords: regression)
Attachments
(2 files, 2 obsolete files)
2.66 KB,
patch
|
jryans
:
review+
|
Details | Diff | Splinter Review |
6.74 KB,
patch
|
jryans
:
review+
|
Details | Diff | Splinter Review |
I'm not sure where this problem is happening, but right now I can only connect to Fennec Aurora, not Fennec Nightly (or my local m-c build). It just times out. Is anyone else seeing this problem? I'm using desktop Firefox Nightly.
Comment 1•10 years ago
|
||
It's caused by a regression in bug 859372. Not sure if Eddy wants to fix it here or there.
Assignee | ||
Comment 2•10 years ago
|
||
I offered to take a look at this, since Eddy doesn't currently have the Fennec workflow set up.
Updated•10 years ago
|
Assignee | ||
Comment 4•10 years ago
|
||
Here's a new test to ensure the debugger server is started, using the path that browser.js actually goes through. I've verified this test failed initially, and now passes with the addition of part 2.
Attachment #8423425 -
Flags: review?(margaret.leibovic)
Assignee | ||
Comment 5•10 years ago
|
||
Margaret, I needed to move a file (dbg-browser-actors.js) out of chrome/content and over to modules. Does that seem fine to you? Panos, can you review the rest of the work here, including changes to the content of |dbg-browser-actors.js| and the other DevTools files?
Attachment #8423429 -
Flags: review?(past)
Attachment #8423429 -
Flags: review?(margaret.leibovic)
Assignee | ||
Comment 6•10 years ago
|
||
Try: https://tbpl.mozilla.org/?tree=Try&rev=fb85d9647714
Comment 7•10 years ago
|
||
Comment on attachment 8423429 [details] [diff] [review] Part 2: Repair dbg-browser-actors on Fennec Review of attachment 8423429 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me and I bet metro and webapprt will need the same fixes.
Attachment #8423429 -
Flags: review?(past) → review+
Comment 8•10 years ago
|
||
Comment on attachment 8423425 [details] [diff] [review] Part 1: Test debug server startup on Fennec Review of attachment 8423425 [details] [diff] [review]: ----------------------------------------------------------------- That's one small step for our tests, one giant leap for devtools stability. Thank you for doing this, this is great!
Reporter | ||
Comment 9•10 years ago
|
||
Comment on attachment 8423425 [details] [diff] [review] Part 1: Test debug server startup on Fennec Review of attachment 8423425 [details] [diff] [review]: ----------------------------------------------------------------- Excellent! I love how simple this is :)
Attachment #8423425 -
Flags: review?(margaret.leibovic) → review+
Reporter | ||
Comment 10•10 years ago
|
||
Comment on attachment 8423429 [details] [diff] [review] Part 2: Repair dbg-browser-actors on Fennec Review of attachment 8423429 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/modules/moz.build @@ +7,5 @@ > EXTRA_JS_MODULES += [ > 'Accounts.jsm', > 'AndroidLog.jsm', > 'ContactService.jsm', > + 'dbg-browser-actors.js', Can we rename this file to match the other modules? Something like DebugBrowserActors.jsm?
Attachment #8423429 -
Flags: review?(margaret.leibovic) → review+
Assignee | ||
Comment 11•10 years ago
|
||
(In reply to :Margaret Leibovic from comment #10) > Comment on attachment 8423429 [details] [diff] [review] > Part 2: Repair dbg-browser-actors on Fennec > > Review of attachment 8423429 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: mobile/android/modules/moz.build > @@ +7,5 @@ > > EXTRA_JS_MODULES += [ > > 'Accounts.jsm', > > 'AndroidLog.jsm', > > 'ContactService.jsm', > > + 'dbg-browser-actors.js', > > Can we rename this file to match the other modules? Something like > DebugBrowserActors.jsm? Well, it's not actually in the JSM format, so that's why I left the name as is. It's now in the CommonJS format that the DevTools loader (and the Addon SDK) uses. Is there a better folder to use perhaps? Should I make a new one? Mainly I just need to be able refer to it with a resource:// URL.
Flags: needinfo?(margaret.leibovic)
Assignee | ||
Comment 12•10 years ago
|
||
Updated test to clear pref on cleanup.
Attachment #8423425 -
Attachment is obsolete: true
Attachment #8424039 -
Flags: review+
Comment 13•10 years ago
|
||
I applied the second patch locally, and it lets the debug server start up. I can connect to my device now, and use the inspector, for example. I can set a breakpoint in browser.js, but I can't set a breakpoint in some aboutDevices.js code I am modifying. My guess is the latter is unrelated -- I've heard of breakpoint flakiness from other folks -- so this appears to be a good fix.
Comment 14•10 years ago
|
||
Could you also export at least BrowserTabActor? Maybe it would make sense to export the other classes, in case they need to be extended by other actors.
Comment 15•10 years ago
|
||
(In reply to Nick Alexander :nalexander from comment #13) > I can set a breakpoint in browser.js, but I can't set a breakpoint in some > aboutDevices.js code I am modifying. My guess is the latter is unrelated -- > I've heard of breakpoint flakiness from other folks -- so this appears to be > a good fix. Could you file a separate bug for this, please? Does it make a difference if you connect the debugger to the device after aboutDevice.js is already loaded in Fennec?
Comment 16•10 years ago
|
||
(In reply to Philipp Kewisch [:Fallen] from comment #14) > Could you also export at least BrowserTabActor? Maybe it would make sense to > export the other classes, in case they need to be extended by other actors. From an API design POV it makes sense to only export things that are absolutely needed in order to limit the API surface. That said, we will of course expose anything that is demonstrably useful.
Comment 17•10 years ago
|
||
(In reply to Panos Astithas [:past] from comment #16) > (In reply to Philipp Kewisch [:Fallen] from comment #14) > > Could you also export at least BrowserTabActor? Maybe it would make sense to > > export the other classes, in case they need to be extended by other actors. > > From an API design POV it makes sense to only export things that are > absolutely needed in order to limit the API surface. That said, we will of > course expose anything that is demonstrably useful. Generally I do agree. Thunderbird uses a different tab list implementation, but uses the same tab actors: http://mxr.mozilla.org/comm-central/source/mail/components/devtools/modules/XULRootActor.js#152
Assignee | ||
Comment 18•10 years ago
|
||
(In reply to Philipp Kewisch [:Fallen] from comment #17) > (In reply to Panos Astithas [:past] from comment #16) > > (In reply to Philipp Kewisch [:Fallen] from comment #14) > > > Could you also export at least BrowserTabActor? Maybe it would make sense to > > > export the other classes, in case they need to be extended by other actors. > > > > From an API design POV it makes sense to only export things that are > > absolutely needed in order to limit the API surface. That said, we will of > > course expose anything that is demonstrably useful. > > Generally I do agree. Thunderbird uses a different tab list implementation, > but uses the same tab actors: > http://mxr.mozilla.org/comm-central/source/mail/components/devtools/modules/ > XULRootActor.js#152 Seems fine to export BrowserTabActor too then. I'll update part 2 to do so before landing.
Reporter | ||
Comment 19•10 years ago
|
||
(In reply to J. Ryan Stinnett [:jryans] from comment #11) > (In reply to :Margaret Leibovic from comment #10) > > Comment on attachment 8423429 [details] [diff] [review] > > Part 2: Repair dbg-browser-actors on Fennec > > > > Review of attachment 8423429 [details] [diff] [review]: > > ----------------------------------------------------------------- > > > > ::: mobile/android/modules/moz.build > > @@ +7,5 @@ > > > EXTRA_JS_MODULES += [ > > > 'Accounts.jsm', > > > 'AndroidLog.jsm', > > > 'ContactService.jsm', > > > + 'dbg-browser-actors.js', > > > > Can we rename this file to match the other modules? Something like > > DebugBrowserActors.jsm? > > Well, it's not actually in the JSM format, so that's why I left the name as > is. It's now in the CommonJS format that the DevTools loader (and the Addon > SDK) uses. > > Is there a better folder to use perhaps? Should I make a new one? Mainly I > just need to be able refer to it with a resource:// URL. Ah, I didn't look closely enough. I don't think we need to make a new directory, since there aren't very many files in there right now. So your patch is fine :)
Flags: needinfo?(margaret.leibovic)
Assignee | ||
Comment 20•10 years ago
|
||
Updated part 2 to also export BrowserTabActor. Try: https://tbpl.mozilla.org/?tree=Try&rev=36b815ca3bef
Attachment #8423429 -
Attachment is obsolete: true
Attachment #8424907 -
Flags: review+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 21•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/d6b3401d9d74 https://hg.mozilla.org/integration/fx-team/rev/89aae014a99f
Comment 22•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d6b3401d9d74 https://hg.mozilla.org/mozilla-central/rev/89aae014a99f
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 32
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•