Closed Bug 1020948 Opened 5 years ago Closed 5 years ago

discardSystemSource drops sources for privileged apps

Categories

(DevTools :: WebIDE, defect)

29 Branch
x86_64
Windows 7
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 33

People

(Reporter: felix.klee, Assigned: ochameau)

References

Details

(Keywords: regression)

Attachments

(4 files, 1 obsolete file)

Attached image Annotated screenshot
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 (Beta/Release)
Build ID: 20140506152807

Steps to reproduce:

1. Installed 2.0.0.0-prerelease on my Keon: <http://downloads.geeksphone.com/keon/master/nightly-images-keon-master-2014-05-12.Gecko-849b0f7.Gaia-014d16e.zip>

2. Opened <about:app-manager> in Firefox 29.0.1 / Windows 7 x64.

3. Connected to: full_keon

4. Updated a privileged app (WIP) to the Keon: <https://github.com/feklee/theta-control>

5. Clicked: Debug

6. Clicked on a source file.


Actual results:

For every source file, the debugger displayed:

[no source]


Expected results:

The source code should have appeared.

Note that I can get the source code to show up if I do either of:

* Install the latest 1.3 release on the Keon: <http://downloads.geeksphone.com/keon/v1.3/images-keon-v1.3-2014-05-11.Gecko-c096d5f.Gaia-f71e255.zip>

* Or, remove `"type": "privileged"` from the app manifest; But then the app doesn't work anymore - no surprise.
Component: Untriaged → Developer Tools: App Manager
Is this still happening?
Flags: needinfo?(felix.klee)
I just installed:

<http://downloads.geeksphone.com/keon/v2.0/images-keon-v2.0-2014-06-20.Gecko-ab79ce3.Gaia-27f07f4.zip>

The Firefox OS UI now looks very different. The issue with remote debugging, however, remains the same. When I click on any of the source files in <about:app-manager> running on Firefox Aurora 32, then I get:

    [no source]

Source code of my app: <https://github.com/feklee/theta-control>

Build identifier of Firefox Aurora: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:32.0) Gecko/20100101 Firefox/32.0

Going back to 1.3 now…
Flags: needinfo?(felix.klee)
I can confirm this by testing this app on the simulators.  You can see debugger sources in the 1.4 simulator, but 2.0 doesn't show any.  

As stated in comment 0, if you remove "type": "privileged" from the manifest, then sources appear in 2.0.

It appears to work in my local 2.1 simulator at first, but this is only because it sets the "javascript.options.discardSystemSource" pref to false.  If I rebuild the simulator to stop doing that, the issue remains.

So, it seems our friend discardSystemSource also discards sources for privileged apps too.

Fabrice / Bobby, is it expected that privileged apps also discard sources when discardSystemSource == true?  I had originally thought it only affected certified apps, but now it appears it also affects any privileged apps too.  This currently blocks app developers from debugging privileged apps on real devices with 2.0+.
Blocks: 990353
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(fabrice)
Flags: needinfo?(bobbyholley)
Keywords: regression
Summary: No source in App Manager for privileged app on Keon with 2.0.0.0-prerelease → discardSystemSource drops sources for privileged apps
Looks like it always discarded privileged apps:
http://hg.mozilla.org/mozilla-central/rev/42aaedb93455#l3.12

We need to figure out a way to relax source discarding somehow for debugging purposes.
So far, we arbitrary disable it when the developer toggles the chrome/certified apps debugger (manually set devtools.debugger.forbid-certified-apps to false).
But it is weak and doesn't help regular app developer working on hosted and privileged apps.

I was thinking about somehow flagging apps once we debug them, so that ShouldDiscardSystemSource() can return false if devtools codebase flaged the app/origin/principal. Then we would just have so tweak the UI/UX/documentation in order to include a "reload the app to see sources".
But I have no idea how to put such persitent flag between devtools codebase and this code in xpconnect.
(In reply to J. Ryan Stinnett [:jryans] from comment #3)
> I can confirm this by testing this app on the simulators.  You can see
> debugger sources in the 1.4 simulator, but 2.0 doesn't show any.  
> 
> As stated in comment 0, if you remove "type": "privileged" from the
> manifest, then sources appear in 2.0.
> 
> It appears to work in my local 2.1 simulator at first, but this is only
> because it sets the "javascript.options.discardSystemSource" pref to false. 
> If I rebuild the simulator to stop doing that, the issue remains.
> 
> So, it seems our friend discardSystemSource also discards sources for
> privileged apps too.
> 
> Fabrice / Bobby, is it expected that privileged apps also discard sources
> when discardSystemSource == true?  I had originally thought it only affected
> certified apps, but now it appears it also affects any privileged apps too.

This is intentional, though I don't have enough data to claim that it is the correct trade-off.

> This currently blocks app developers from debugging privileged apps on real
> devices with 2.0+.

I agree with ochameau that we need to fix this at the level of the debugger.


(In reply to Alexandre Poirot (:ochameau) from comment #4)
> Looks like it always discarded privileged apps:
> http://hg.mozilla.org/mozilla-central/rev/42aaedb93455#l3.12
> 
> We need to figure out a way to relax source discarding somehow for debugging
> purposes.
> So far, we arbitrary disable it when the developer toggles the
> chrome/certified apps debugger (manually set
> devtools.debugger.forbid-certified-apps to false).
> But it is weak and doesn't help regular app developer working on hosted and
> privileged apps.

Why can't the regular debugger just temporarily disable source discarding? That seems to be how the desktop devtools work.

We could add a knob that is more transient than a pref if that would be helpful.
Flags: needinfo?(bobbyholley)
It appears that, even if we don't discard sources, we still see
some [no source] on some JS ressources when you open a toolbox
against b2g main process (from app manager's device panel,
if you set the certified pref on device and the chrome pref in firefox,
you have a "debug main process" button).
In that case, some JS files hosted on jar: URLs fail to get sources printed.

While debugging that particular issue, I ended up discovering script.js in more details
and its fetch() method... which is never called!
It isn't called for these buggy jar URLs, nor when discardSystemSources is on.
This existing feature is a really nice way to address the discardSystemSources issue.
When DebuggerObject.text fails to retrieve sources from the JS engine,
we should just ensure to fallback to this fetch() method and retrieve sources manually.

At the end fixing this jar: issue and discardSystemSources can be done
with this simple patch.
The issue was that, DebuggerObject.text equals to "[no source]" over here:
  http://mxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/script.js#5061
That ends up latter setting _text to "[no source]" over here and returns it to the client:
  http://mxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/script.js#2657
And prevent running the code right after this if condition, that manually read script sources from its URL...

Ideally, DebuggerObject.text would return null/undefined, but let's try fix that ASAP to uplift it.

https://tbpl.mozilla.org/?tree=Try&rev=939a5d124227
(While I was patching script.js I also fixed an exception when trying to instanciate
 a chromeDebugger in child proccesses)
Attachment #8446159 - Flags: feedback?(jryans)
With the previous patch, we can stop hacking the discardSystemSource pref
and keep "optimized" performance even when devtools are turned on!
Attachment #8446159 - Flags: review?(past)
Attachment #8446165 - Flags: review?(jryans)
Flags: needinfo?(fabrice)
Assignee: nobody → poirot.alex
Comment on attachment 8446159 [details] [diff] [review]
Ensure fetching script sources when DebuggerObject.source fails.

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

Seems like a nice workaround to me! :)
Attachment #8446159 - Flags: feedback?(jryans) → feedback+
Comment on attachment 8446165 [details] [diff] [review]
Stop preventing source discarding when using devtools.

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

We should also clean up Gaia's build process which sets this as well.
Attachment #8446165 - Flags: review?(jryans) → review+
Comment on attachment 8446159 [details] [diff] [review]
Ensure fetching script sources when DebuggerObject.source fails.

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

::: toolkit/devtools/server/actors/script.js
@@ +5080,5 @@
>            .QueryInterface(Ci.nsIURL);
>          if (url.fileExtension === "js") {
>            spec.contentType = "text/javascript";
> +          // If Debugger wasn't able to expose sources,
> +          // we let a change to source() to fetch them manually.

Slightly better wording: "If the Debugger API wasn't able to load the source, because sources were discarded (javascript.options.discardSystemSource == true), give source() a chance to fetch them."
Attachment #8446159 - Flags: review?(past) → review+
Updated comment.
Attachment #8446159 - Attachment is obsolete: true
Attachment #8446524 - Flags: review+
Keywords: checkin-needed
Tim, this is more or less a revert of bug 1001348.
We figured out a way to display sources in devtools without disabling source discarding optimization.
Attachment #8446542 - Flags: review?(timdream)
Duplicate of this bug: 1030746
Attachment #8446542 - Flags: review?(timdream) → review?(yurenju.mozilla)
Comment on attachment 8446542 [details] [review]
Stop discarding sources when using DEVICE_DEBUG

r=yurenju
Attachment #8446542 - Flags: review?(yurenju.mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/9ea6813cf5cc
https://hg.mozilla.org/mozilla-central/rev/ccbe51e7e541
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 33
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.