1342322 - chrome.tabs.onRemoved not fired for last tab in last window

Status: RESOLVED INVALID

(WebExtensions :: Untriaged, defect)

Description

[Martin Zhai]

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/56.0.2924.87 Safari/537.36

Steps to reproduce:

1. Open Firefox, navigate to about:debugging, load tabs-tabs-tabs as a temporary add-on (it can be found in Firefox WebExtensions official examples)
2. Click Debug to open add-on debugging window
3. Add break point at #192 line of tab.js (console.log(`The tab with id: ${tabId}, is closing`);)
4. Open some tabs, and close tab by clicking "Remove active tab" in tabs-tabs-tabs popup window, and break point will be hit
5. If there is one last tab in browser window, and only one browser window exists, click "Remove active tab" again


Actual results:

Browser quit without hit break point


Expected results:

Browser should hit break point before quit process

Comment 1

[Kris Maglione [:kmag]]

This is only because the popup itself closes before the window does. Listeners in background pages and other remaining contexts are fired as expected.

Comment 2

[Martin Zhai]

(In reply to Kris Maglione [:kmag] from comment #1)
> This is only because the popup itself closes before the window does.
> Listeners in background pages and other remaining contexts are fired as
> expected.

How can I be sure that last tabs.onRemoved was fired? 
I can't log something to console to make sure of that, since browser will quit soon after close last tab.

Comment 3

[Martin Zhai]

(In reply to Kris Maglione [:kmag] from comment #1)
> This is only because the popup itself closes before the window does.
> Listeners in background pages and other remaining contexts are fired as
> expected.

I moved `tabs.onRemoved` listener to background script, and add break point to its callback. Firefox quit without hitting break point. Same behavior as in popup.

Then I tried send message to my native application using native messaging, in callback of `tabs.onRemoved`, in background script. My native application didn't receive the message.

Question:
Does Firefox make sure that `tabs.onRemoved` callback finishes running before quit browser process?

Comment 4

[:shell escalante]

do you have any thoughts on this Bob?

Comment 5

[Bob Silverberg [:bsilverberg]]

Martin, do you know if this works as you expect in Chrome? Also, can you please describe your use case for this behaviour.

Comment 6

[Bob Silverberg [:bsilverberg]]

Also, I cannot reproduce this locally, on OS X. Firefox does not automatically quit when the last tab in the last window is closed, so the onRemoved listener fires as expected.

Perhaps this is OS specific? On what OS are you running this, Martin?

Comment 7

[Martin Zhai]

Hi Bob

I'm developing an extension for automation test. I want to record user operation on Firefox, so they can replay later.
For this case, I want to record browser close event. I need to know when closing tab causing window closing. 
I send message to my native messaging app in callback of tabs.onRemoved, then generate and save recording message.

On Chrome everything works as expected on this case.
But on Firefox, browser close event not recorded. I suppose, either last tabs.onRemoved not fired, or browser process quit before callback of tabs.onRemoved finishes running.
I'm not sure, and have no idea how to make sure which is the case.

What do you think?
Thanks

Comment 8

[Bob Silverberg [:bsilverberg]]

Thanks for the use case, Martin, but you didn't answer my followup question. On which OS are you experiencing this?

Comment 9

[Martin Zhai]

Sorry, I'm using Firefox on Windows 7.

Comment 10

[Bob Silverberg [:bsilverberg]]

I will see if I can reproduce this behaviour on my Windows VM.

Comment 11

[Bob Silverberg [:bsilverberg]]

I did some testing on my Windows VM, and I was able to reproduce this. When I close the last tab in the last window the browser closes before the tabs.onRemoved listener fires for the last tab. When I tested windows.onRemoved I found that it did fire for the last window. I also noticed that when you close a window with multiple tabs (or even just one tab) the windows.onRemoved listener fires before the tabs.onRemoved listener.

So I guess there's a timing issue where, when the final tab and window are closed, which tells the browser to also shut down, there is sufficient time for windows.onRemoved to fire (or perhaps it fires before the message is sent to the browser to quit), but there is not sufficient time for tabs.onRemoved to fire.

I'm not sure how to proceed with a fix for this, if it's even something that warrants fixing. Kris and/or Andrew, what do you propose as a fix?

Comment 12

[Martin Zhai]

Thanks Bob.

I want to know how did you make sure that last windows.onRemoved actually fired? Because I add breakpoint to callback of windows.onRemoved, if there is only one Firefox window, Firefox quits without hitting breakpoint. (If there is more than one window, breakpoint will hit.)

Comment 13

[Bob Silverberg [:bsilverberg]]

(In reply to Martin Zhai from comment #12)
> Thanks Bob.
> 
> I want to know how did you make sure that last windows.onRemoved actually
> fired? Because I add breakpoint to callback of windows.onRemoved, if there
> is only one Firefox window, Firefox quits without hitting breakpoint. (If
> there is more than one window, breakpoint will hit.)

I did it using dump() and running from a local build, but I realize now that I didn't actually confirm that the listener fired. I only confirmed that the listener was dispatched, but that is done asynchronously, so although I can see that the firing of the listener is requested, I cannot be sure that the listener actually fires.

Using your technique of sending a message to a native app would be a good way of checking that, and it sounds like maybe you've already done that.

One thing I did notice, for windows.onRemoved, is that there is an error in the console stating "windows.onRemoved event fired after context unloaded", which probably explains why the listener doesn't actually fire. I'm guessing the extension is unloaded before the listener gets a chance to fire.

Comment 14

[Andrew Swan [:aswan]]

(In reply to Bob Silverberg [:bsilverberg] from comment #11)
> I did some testing on my Windows VM, and I was able to reproduce this. When
> I close the last tab in the last window the browser closes before the
> tabs.onRemoved listener fires for the last tab. When I tested
> windows.onRemoved I found that it did fire for the last window. I also
> noticed that when you close a window with multiple tabs (or even just one
> tab) the windows.onRemoved listener fires before the tabs.onRemoved listener.
> 
> So I guess there's a timing issue where, when the final tab and window are
> closed, which tells the browser to also shut down, there is sufficient time
> for windows.onRemoved to fire (or perhaps it fires before the message is
> sent to the browser to quit), but there is not sufficient time for
> tabs.onRemoved to fire.
> 
> I'm not sure how to proceed with a fix for this, if it's even something that
> warrants fixing. Kris and/or Andrew, what do you propose as a fix?

I don't think we should block shutdown to deliver events like this.
Even if we could guarantee delivery of the event, the extension couldn't do anything asynchronous and expect to finish it before the browser exits.  Could we get more information about what the extension needs to do in response to this event?  Perhaps we can suggest an alternative approach...

Comment 15

[Martin Zhai]

As said in my previous comment 7:

"I'm developing an extension for automation test. I want to record user operation on Firefox, so they can replay later.
For this case, I want to record browser close event. I need to know when closing tab causing window closing. 
I send message to my native messaging app in callback of tabs.onRemoved, then generate and save recording message."

In a word, what I'm trying to achieve, is to make sure callback of tabs.onRemoved finishes running before browser process quits. Is that possible?

On Chrome things work as expected for this case. I don't know if Firefox uses the same mechanism as Chrome. Could Firefox know when events' callback in extension finishes running? Could it quit after that?

I note you said "the extension couldn't do anything asynchronous and expect to finish it before the browser exits". But could it be synchronous?

Since I don't know from implementation perspective of Firefox, please correct me if I'm wrong.

Comment 16

[Andrew Swan [:aswan]]

(In reply to Martin Zhai from comment #15)
> As said in my previous comment 7:
> 
> "I'm developing an extension for automation test. I want to record user
> operation on Firefox, so they can replay later.
> For this case, I want to record browser close event. I need to know when
> closing tab causing window closing. 
> I send message to my native messaging app in callback of tabs.onRemoved,
> then generate and save recording message."
> 
> In a word, what I'm trying to achieve, is to make sure callback of
> tabs.onRemoved finishes running before browser process quits. Is that
> possible?


I don't think that specifically is possible but you can listen for the "close" event in your background page to know when the browser is shutting down.  I guess you don't really have a way of knowing if the browser is shutting down because the last window closed or because the user chose to quit or restart (in which case open tabs/windows at shutdown time can be restored the next time the browser starts).

Note that from the close event handler, all you can do is synchronous work since the background page is about to be cleaned up.  If you send a message to your native application directly from the close handler, it should typically get sent assuming the native application readss the message promptly.

> On Chrome things work as expected for this case. I don't know if Firefox
> uses the same mechanism as Chrome. Could Firefox know when events' callback
> in extension finishes running? Could it quit after that?
> 
> I note you said "the extension couldn't do anything asynchronous and expect
> to finish it before the browser exits". But could it be synchronous?

No, this would allow a buggy or misbehaving extension to delay shutdown indefinitely.  The low-level tab and window handling code is vastly different between Chrome and Firefox so the specific difference you've identified here isn't really a surprise...

Comment 17

[Martin Zhai]

Andrew, thanks for the suggestion. I'll try later.

I'm wondering why with JSCTypes SDK, if there is only one tab in last window, the "close" event (https://developer.mozilla.org/en-US/Add-ons/SDK/High-Level_APIs/tabs#close) is properly fired? Something changed with WebExtensions?

Comment 18

[Andrew Swan [:aswan]]

(In reply to Martin Zhai from comment #17)
> I'm wondering why with JSCTypes SDK, if there is only one tab in last
> window, the "close" event
> (https://developer.mozilla.org/en-US/Add-ons/SDK/High-Level_APIs/tabs#close)
> is properly fired? Something changed with WebExtensions?

Yes, a lot changed.  Sorry, I'm not familiar with the sdk implementation details.

Comment 19

[Martin Zhai]

(In reply to Andrew Swan [:aswan] from comment #16)
> but you can listen for the "close" event in your background page to know when the browser is shutting down.  
Do you mean listen to window.close event in background page?

I tried that, and add break point in handler function, Firefox quit without hitting break point. Does it mean window.close event not fired? What else can I do to know if browser is shutting down?

Comment 20

[Andrew Swan [:aswan]]

Sorry, "unload" is the right event to listen for.
However, I don't think that a debugger break point is a useful way to test this, the application is closing anyway and having the debugger attached doesn't prevent that.  I understand Chrome may work differently in this scenario but ... its a completely different code base, this isn't any sort of standardized behavior that you can rely on.