Opened 4 years ago

Closed 3 years ago

#3815 closed defect (fixed)

`outerWindowID` of a newly created tab is initialized with some delay in e10s

Reported by: sergz Assignee: sergz
Priority: Unknown Milestone:
Module: Extensions-for-Adblock-Plus Keywords: abpcrawler
Cc: trev Blocked By:
Blocking: #3792 Platform: Unknown / Cross platform
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29338442/

Description

We need to know outerWindowID of the tab at least to be notified about requests in that tab (see RequestNotifier).

Environment

Firefox e10s
ABP 2.7.2
abpcrawler https://hg.adblockplus.org/abpcrawler/rev/b8d3bb27775f

How to reproduce

Try the following code in the Browser console:

let browser = window.getBrowser();
let tab = browser.addTab("about:blank");
console.log(tab.linkedBrowser.outerWindowID);
setTimeout(function(){console.log(tab.linkedBrowser.outerWindowID);}, 20);

In abpcrawler the code is executed in chrome process and window is Services.wm.getMostRecentWindow("navigator:browser");.

Observed behaviour

In e10s tab.linkedBrowser.outerWindowID is firstly null however after some delay (~20msec on my machine) it is initialized to the actual number (e.g. 2147483652).
In FF 45 tab.linkedBrowser.outerWindowID is always already initialized to some number.

Expected behaviour

tab.linkedBrowser.outerWindowID should be already initialized to the actual number after the returning from browser.addTab("about:blank").

Additional info

The issue is already communicated to Mozilla, https://bugzilla.mozilla.org/show_bug.cgi?id=1256602.
I would propose to make a function which allocates a new tab and returns a Promise which is resolved with the tab when outerWindowID is initialized. In that case we will be able to simply change only that function and rely on the Promise returned by Firefox if Mozilla starts to return Promise as it's said in the comment https://bugzilla.mozilla.org/show_bug.cgi?id=1256602#c1.

In addition since it completely affects implementation of TabAllocator I would like to propose to refactor it at the same time.
Currently it allocates a certain number of tabs at the beginning and keeps this number of tabs, however for each new URL it takes a previously allocated tab, closes it when we finish with it and creates a new one for a potential next URL.
I completely support the idea to have a new tab for each URL because it simplifies the things a lot, it is how tests should run, each test in a fresh environment.
However there is no reason to pre allocate any number of tabs because it gives a neglidgible performance benefit. Currently the default number of tabs is 30, however we can run the crawler with hundreds and thousands URLs, there is no significant difference between allocating of 1000 tabs and 970 tabs at runtime. There is also no reason to allocate a new tab for the potential URL, it should be allocated only if there is indeed the next URL.

Change History (3)

comment:1 Changed 3 years ago by sergz

  • Status changed from new to reviewing

comment:3 Changed 3 years ago by sergz

  • Resolution set to fixed
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.