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 comments Created March 16, 2016, 3:45 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 | no next file » | 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..04299d4bf7384707ba458405d249bc7b5b7d08c4 100644
--- a/lib/crawler.js
+++ b/lib/crawler.js
@@ -25,7 +25,31 @@ let {FilterNotifier} = abprequire("filterNotifier");
let {FilterStorage} = abprequire("filterStorage");
/**
- * Creates a pool of tabs and allocates them to tasks on request.
+ * Creates a blank tab in the browser.
+ *
+ * @param {tabbrowser} browser
+ * The tabbed browser where the tab should be created.
+ * @return {Promise.<tab>} promise which resolves once the tab is fully initialized.
+ */
+function createTab(browser)
+{
+ let tab = browser.addTab("about:blank");
+ if (tab.linkedBrowser.outerWindowID)
+ return Promise.resolve(tab);
+ return new Promise((resolve, reject) =>
+ {
+ let onBrowserInit = (msg) =>
+ {
+ // https://bugzilla.mozilla.org/show_bug.cgi?id=1256602#c1
+ tab.linkedBrowser.messageManager.removeMessageListener("Browser:Init", onBrowserInit);
+ resolve(tab);
+ };
+ tab.linkedBrowser.messageManager.addMessageListener("Browser:Init", onBrowserInit);
+ });
+}
+
+/**
+ * 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 +59,37 @@ let {FilterStorage} = abprequire("filterStorage");
*/
function TabAllocator(browser, maxtabs)
{
+ 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.
+ // That tab will be removed when the first tab is requested.
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 = [];
}
TabAllocator.prototype = {
/**
- * Returns a promise that will resolve into a tab once a tab can be allocated.
+ * 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._tabs < this._maxtabs)
{
- let deferred = Promise.defer();
- this._deferred.push(deferred);
- return deferred.promise;
+ let tab = createTab(this._browser);
+ // Close initial tab, we don't need it anymore.
+ if (this._tabs == 0)
+ this._browser.removeTab(this._browser.tabs[0]);
+ this._tabs++;
+ return tab;
}
+ return new Promise((resolve, reject) =>
+ {
+ this._resolvers.push(resolve);
+ });
},
/**
@@ -73,12 +101,11 @@ TabAllocator.prototype = {
{
let browser = tab.parentNode.tabbrowser;
browser.removeTab(tab);
- tab = browser.addTab("about:blank");
- if (this._deferred.length)
- this._deferred.shift().resolve(tab);
+ if (this._resolvers.length)
+ this._resolvers.shift()(createTab(this._browser));
else
- this._tabs.push(tab);
+ this._tabs--;
}
};
@@ -231,8 +258,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 | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld