928416 - win32: app manager w/adb causes cmd window to flash on startup/shutdown

Status: RESOLVED FIXED

(DevTools Graveyard :: WebIDE, defect, P3)

Description

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

This is pretty ugly -- if I have the adb stuff installed, the app manager causes a win32 console window to flash on firefox startup/shutdown.  adb is almost certainly built with SUBSYSTEM:CONSOLE, but there must be a way to redirect in/out in such a way that no console window is actually started.

Comment 1

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

There's probably something relevant in here: http://msdn.microsoft.com/en-us/library/windows/desktop/ms682073%28v=vs.85%29.aspx

Comment 2

[Paul Rouget [:paul]]

> the adb stuff installed

You're talking about the ADB helper addon?

Is this related to 921086?

Comment 3

[(no longer active)]

IIRC Calling AllocConsole on the parent process will let the child process inherit the console and not create its own.

Comment 4

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Bug 921086 is probably a symptom of this underlying issue.  The "popup" it's talking about is the console window.

Comment 5

[Alexandre Poirot [:ochameau]]

I tried to address that, but got a wontfix in bug 485946.
We will have to wait for the js implementation of adb, or launch adb with enigma's ipc code [2] that allows to have a decent process launch API. But I think we face some other issues when using this.

[1] https://github.com/mozilla/libadb.js/
[2] https://github.com/ochameau/jetpack-subprocess

Comment 6

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

I've just released adbhelper 0.2.5 which fixes this issue. \o/

Comment 8

[Paul Rouget [:paul]]

(In reply to J. Ryan Stinnett [:jryans] from comment #6)
> I've just released adbhelper 0.2.5 which fixes this issue. \o/

link to commit?

Comment 9

[Paul Rouget [:paul]]

https://github.com/jryans/adbhelper/commit/2caf411d8afd8378fadbfdb634dc936ee6f2bd81 ?

If so - can you make sure changes are reflected in github.com/mozilla/adbhelper as well? And, it's a binary change, what exactly happened there?

And thanks for the fix :)

Comment 10

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to Paul Rouget [:paul] (slow to respond. Ping me on IRC) from comment #9)
> https://github.com/jryans/adbhelper/commit/
> 2caf411d8afd8378fadbfdb634dc936ee6f2bd81 ?

Yes, that's it.

> If so - can you make sure changes are reflected in
> github.com/mozilla/adbhelper as well?

I will soon... I don't have write access to that repo at the moment, but I've requested it from Myk.  It would be there already if I had it. :)

> And, it's a binary change, what exactly happened there?

Well, Mossop's comment in bug 485946 comment 38 about compiling an executable with /SUBSYSTEM:WINDOWS to disable the console made me wonder how that flag might be implemented.

It turns out this subsystem setting is an option set in the Windows PE binary executable header[1].  So, I used a binary editor to change this option in our existing adb.exe from IMAGE_SUBSYSTEM_WINDOWS_CUI (3) to IMAGE_SUBSYSTEM_WINDOWS_GUI (2).

Et voilà! No more console window.

[1]: http://msdn.microsoft.com/en-us/library/windows/desktop/ms680339%28v=vs.85%29.aspx

Comment 11

[Paul Rouget [:paul]]

(In reply to J. Ryan Stinnett [:jryans] from comment #10)
> (In reply to Paul Rouget [:paul] (slow to respond. Ping me on IRC) from
> comment #9)
> > https://github.com/jryans/adbhelper/commit/
> > 2caf411d8afd8378fadbfdb634dc936ee6f2bd81 ?
> 
> Yes, that's it.
> 
> > If so - can you make sure changes are reflected in
> > github.com/mozilla/adbhelper as well?
> 
> I will soon... I don't have write access to that repo at the moment, but
> I've requested it from Myk.  It would be there already if I had it. :)

I added you to the devtools mozilla group. You should be able to do the merge.

> > And, it's a binary change, what exactly happened there?
> 
> Well, Mossop's comment in bug 485946 comment 38 about compiling an
> executable with /SUBSYSTEM:WINDOWS to disable the console made me wonder how
> that flag might be implemented.
> 
> It turns out this subsystem setting is an option set in the Windows PE
> binary executable header[1].  So, I used a binary editor to change this
> option in our existing adb.exe from IMAGE_SUBSYSTEM_WINDOWS_CUI (3) to
> IMAGE_SUBSYSTEM_WINDOWS_GUI (2).
> 
> Et voilà! No more console window.

Wow! Cool :)

Has it been done manually? What happens if we want to update adb.exe in the future?

Comment 12

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to Paul Rouget [:paul] (slow to respond. Ping me on IRC) from comment #11)
> (In reply to J. Ryan Stinnett [:jryans] from comment #10)
> > (In reply to Paul Rouget [:paul] (slow to respond. Ping me on IRC) from
> > comment #9)
> > > If so - can you make sure changes are reflected in
> > > github.com/mozilla/adbhelper as well?
> > 
> > I will soon... I don't have write access to that repo at the moment, but
> > I've requested it from Myk.  It would be there already if I had it. :)
> 
> I added you to the devtools mozilla group. You should be able to do the
> merge.

Thanks!  I've updated the mozilla repo just now.

> > > And, it's a binary change, what exactly happened there?
> > 
> > Well, Mossop's comment in bug 485946 comment 38 about compiling an
> > executable with /SUBSYSTEM:WINDOWS to disable the console made me wonder how
> > that flag might be implemented.
> > 
> > It turns out this subsystem setting is an option set in the Windows PE
> > binary executable header[1].  So, I used a binary editor to change this
> > option in our existing adb.exe from IMAGE_SUBSYSTEM_WINDOWS_CUI (3) to
> > IMAGE_SUBSYSTEM_WINDOWS_GUI (2).
> > 
> > Et voilà! No more console window.
> 
> Wow! Cool :)
> 
> Has it been done manually? What happens if we want to update adb.exe in the
> future?

Yes, it's manual for now.  We have not updated adb.exe to my knowledge since it was added to adbhelper's first version a year ago, so not worth automating yet.