832934 - Fennec: if tabs are opened but not ready at add-on startup, 'ready' events are not fired when the tabs become ready.

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect, P1)

Description

[Taku0]

Attachment: Sample extension simply log the URLs of tabs on 'ready' events.

Description:
 'ready' events not fired for tabs exiting at startup
 on Firefox for Android, since 'open' events, on which
 event listeners for 'DOMContentLoaded' are installed,
 are not fired.

Steps to Reproduce the Problem:
 1. Install the attached extension on Firefox for Android.
    The extension simply logs the URLs of tabs on 'ready' events.
    lib/main.js:
      let tabs = require('tabs');

      tabs.on('ready', function (tab) {
        console.log(tab.url);
      });

      console.log("initialized");
 2. Open about:home.
 3. Press the home button to back to the home screen.
 4. Kill Firefox process.
 5. Start monitoring logcat.
    adb logcat | grep 'Gecko'
 6. Start Firefox.

Expected:
 Getting log like the following lines:
   "I/GeckoApp( 1819): Got message: DOMContentLoaded"
   ...
   "E/GeckoConsole( 1819): info: log-url: about:home"

Actual:
 Getting 'DOMContentLoaded' but not 'about:home'

Cause:
 The listeners for 'DOMContentLoaded' events are installed in
 the onTabOpen function in lib/sdk/windows/tabs-fennec.js,
 whereas tabs are opened before initializing the extension.
 We see "I/GeckoTabs( 1819): Got message: Tab:Added" before
 the message "initialized".

Possible Fix:
 Call onTabOpen for all tabs at startup.

 let tabs = mainWindow.BrowserApp.tabs;

 for (let i = 0; i < tabs.length; i++) {
   onTabOpen({target : tabs[i].browser});
 }

 Note that tabs-firefox.js has similar code.
 Note also that onTabOpen depends on gTabs,
 so that it cannot be called in the Tabs constructor.

Issue:
 Is it possible that an extension is initialized before opening tabs
 at startup?

GitHub:
  https://github.com/taku0/addon-sdk/tree/onTabOpen-on-startup

Comment 1

[Erik Vold [:erikvold] (please needinfo? me)]

I don't see why this should be fixed.  An add-on can be installed at any time, and there will be tabs already open, for which there will not be any 'ready' events fired, and there should not be, the add-on developer has to handle that case by looping through open tabs before adding the 'ready' event listener.  Doing so would also cover the startup case.

Comment 2

[Taku0]

Why we should fix the problem: if tabs are opened but not ready at add-on initialization, the add-on does not get a ready event even after the pages are ready. Since pages are not ready at add-on initialization, we cannot process them.
Moreover, we will not get ready events when another page is loaded on existing tabs. For example, if the user taps a link and navigated to another URL, ready event should be fired but not.
Note that if we opens a new tab, we will get a ready event whenever the user taps a link and navigated to another URL.

Cause:
Let us see how the SDK fires a ready event for a tab opened after add-on initialization:
1. When add-on is initialized. tabs-fennec.js installs an event handler for TabOpen event.
2. When a tab is opened (but still not ready), an TabOpen event is fired.
3. In response to the TabOpen event, tabs-fennec.js installs an event handler for the DOMContentLoaded event.
4. When the page is ready, Firefox fires DOMContentLoaded event.
5. In response to the DOMContentLoaded event, tabs-fennec.js fires ready event.

Since the event handlers for the DOMContentLoaded are installed in response to the open events, the SDK will not get any DOMContentLoaded events for tabs opened before add-on initialization.
Therefore, the SDK does not fire ready events even when the page is ready.

Comment 3

[Dave Townsend [:mossop]]

(In reply to Taku0 from comment #2)
> Why we should fix the problem: if tabs are opened but not ready at add-on
> initialization, the add-on does not get a ready event even after the pages
> are ready. Since pages are not ready at add-on initialization, we cannot
> process them.

This can happen regardless of whether it is at app startup or when you install the add-on during runtime. A better solution is to add the readyState property to tabs, that way when your addon starts you can iterate the existing tabs and see what is already open and loaded and what is still waiting to be loaded.

Comment 4

[Taku0]

(In reply to Dave Townsend (:Mossop) from comment #3)
> (In reply to Taku0 from comment #2)
> > Why we should fix the problem: if tabs are opened but not ready at add-on
> > initialization, the add-on does not get a ready event even after the pages
> > are ready. Since pages are not ready at add-on initialization, we cannot
> > process them.
> 
> This can happen regardless of whether it is at app startup or when you
> install the add-on during runtime. A better solution is to add the
> readyState property to tabs, that way when your addon starts you can iterate
> the existing tabs and see what is already open and loaded and what is still
> waiting to be loaded.

Yes, we indeed need the readyState property.
Nevertheless, even if the readyState is implemented, the problem is not solved.
The real problem is that tabs-fennec.js does not propagate DOMContentLoaded events when the tabs becomes ready *after* add-on initialization, so the following code does not work:

main.js:
let tabs = require('sdk/tabs');

function handleTab(tab) {...}

for (let tab of tabs) {
  if (tab.readyState == 'opened but not ready') {
    tab.on('ready', handleTab); // THIS DOES NOT WORK
                                // since the SDK ignores DOMContentLoaded
                                // and does not fire ready events.
  } if (tab.readyState == 'ready') {
    handleTab(ready);
  }
}

tabs.on('ready', handleTab); // This also does not help due to the same reason

Comment 5

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Taku0 is right. We need both:

1. Need a way to observe a tab state using `readyState` or similar.
2. Tabs that were open but were not ready at add-on stratup should emit
   `ready` events so add-on authors can do things once they are ready.

Although I would prefer to avoid `readyState` mutable property on the `tab`
object in favor of `getReadyState(thing)` style function, that can be further
extended in a future to work with things like windows, panels, etc.. if necessary.

Maybe even better to have `isReady(thing)` predicate style function instead.

Comment 6

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

BTW: I have not looked into code but it likely that we have same issue on desktop FF as well not just fennec.

Comment 7

[Taku0]

(In reply to Irakli Gozilalishvili [:irakli] [:gozala] [@gozala] from comment #6)
> BTW: I have not looked into code but it likely that we have same issue on
> desktop FF as well not just fennec.

Desktop FF seems to be doing right.

tabs-firefox.js:
    for each (let tab in getTabs(this._window)) {
      // We emulate "open" events for all open tabs since gecko does not emits
      // them on the tabs that new windows are open with. Also this is
      // necessary to synchronize tabs lists with an actual state.
      this._onTabOpen(tab);
    }

Comment 8

[Erik Vold [:erikvold] (please needinfo? me)]

this bug was fixed in https://github.com/mozilla/addon-sdk/commit/bb751291552b5b529d69f50be35564a015f36557 which is shipping with 1.14 (see bug 784224)

Also I made bug 854800 for part 2 of comment 5 about the isReady method