Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: lib/crawler.js

Issue 29338442: Issue 3815 - Fix TabAllocator. Now it returns tab with initialized outerWindowID (Closed)
Patch Set: address comment about comment Created Sept. 15, 2016, 3:04 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | run.py » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: lib/crawler.js
diff --git a/lib/crawler.js b/lib/crawler.js
index 7bc28dbbe75187a1a752583c81d5c638a24c98ca..e4291269503ac049f9c700572ce60fbbb550a1de 100644
--- a/lib/crawler.js
+++ b/lib/crawler.js
@@ -25,7 +25,7 @@ let {FilterNotifier} = abprequire("filterNotifier");
let {FilterStorage} = abprequire("filterStorage");
/**
- * Creates a pool of tabs and allocates them to tasks on request.
+ * Allocates tabs on request but not more than maxtabs at the same time.
*
* @param {tabbrowser} browser
* The tabbed browser where tabs should be created
@@ -35,33 +35,67 @@ let {FilterStorage} = abprequire("filterStorage");
*/
function TabAllocator(browser, maxtabs)
{
- browser.removeAllTabsBut(browser.tabs[0])
-
- this._tabs = [];
- for (let i = 0; i < maxtabs; i++)
- this._tabs.push(browser.addTab("about:blank"));
-
- browser.removeTab(browser.tabs[0]);
-
- this._deferred = [];
+ this._browser = browser;
+ this._tabs = 0;
+ this._maxtabs = maxtabs;
+ // The queue containing resolve functions of promises waiting for a tab.
+ this._resolvers = [];
+ // Keep at least one tab alive to prevent browser from closing itself.
+ let tabToRemove = this._browser.tabs[0];
+ this._browser.removeAllTabsBut(tabToRemove);
+ // this._tab is a keep alive tab
+ this._tab = this._createTab().then(tab =>
+ {
+ // Starting from Firefox 48 (nightly) the sequence of calls addTab and
+ // removeTab can cause a closing of the browser because a new tab is still
+ // not here. Because of that we need to remove the previous tab only after
+ // the new tab is ready.
+ this._browser.removeTab(tabToRemove);
+ return tab;
+ });
}
TabAllocator.prototype = {
/**
- * Returns a promise that will resolve into a tab once a tab can be allocated.
+ * Creates a blank tab in this._browser.
+ *
+ * @return {Promise.<tab>} promise which resolves once the tab is fully initialized.
+ */
+ _createTab: function()
+ {
+ this._tabs++;
+ let tab = this._browser.addTab("about:blank");
+ if (tab.linkedBrowser.outerWindowID)
+ return Promise.resolve(tab);
+ return new Promise((resolve, reject) =>
+ {
+ let onBrowserInit = (msg) =>
+ {
+ tab.linkedBrowser.messageManager.removeMessageListener("Browser:Init", onBrowserInit);
+ resolve(tab);
+ };
+ // "Browser:Init" message is sent once the browser is ready, see
+ // https://bugzil.la/1256602#c1
+ tab.linkedBrowser.messageManager.addMessageListener("Browser:Init", onBrowserInit);
+ });
+ },
+
+ /**
+ * Returns a promise that will resolve into a tab once a tab is allocated.
* The tab cannot be used by other tasks until releaseTab() is called.
*
- * @result {Promise}
+ * @result {Promise.<tab>}
*/
getTab: function()
{
- if (this._tabs.length)
- return this._tabs.shift();
- else
+ if (this._tab)
{
- let deferred = Promise.defer();
- this._deferred.push(deferred);
- return deferred.promise;
+ let tab = this._tab;
+ delete this._tab;
+ return tab;
}
+ if (this._tabs < this._maxtabs)
+ return this._createTab();
+ return new Promise((resolve, reject) => this._resolvers.push(resolve));
},
/**
@@ -71,15 +105,33 @@ TabAllocator.prototype = {
*/
releaseTab: function(tab)
{
- let browser = tab.parentNode.tabbrowser;
- browser.removeTab(tab);
- tab = browser.addTab("about:blank");
+ // If we are about to close last tab don't close it immediately rather
+ // allocate a new blank tab and close the current one afterwards.
+ if (this._tabs == 1)
+ {
+ this._tab = this._createTab().then((resultTab) =>
+ {
+ this.releaseTab(tab);
+ return resultTab;
+ });
+ return;
+ }
- if (this._deferred.length)
- this._deferred.shift().resolve(tab);
- else
- this._tabs.push(tab);
- }
+ this._browser.removeTab(tab);
+ this._tabs--;
+ if (this._resolvers.length)
+ {
+ if (this._tab)
+ {
+ this._resolvers.shift()(this._tab);
+ delete this._tab;
+ }
+ else if (this._tabs < this._maxtabs)
+ {
+ this._resolvers.shift()(this._createTab());
+ }
+ }
+ },
};
/**
@@ -231,8 +283,8 @@ function run(window, urls, timeout, maxtabs, targetURL, onDone)
exports.run = run;
/**
- * Spawns a {Task} task to crawl each url from `urls` argument and calls
- * `onDone` when all tasks are finished.
+ * Spawns a {Task} task to crawl each url from urls argument and calls
+ * onDone when all tasks are finished.
* @param {Window} window
* The browser window we're operating in
* @param {String[]} urls
« no previous file with comments | « no previous file | run.py » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld