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 tab keeping window alive Created Sept. 15, 2016, 3:31 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..2e7051b8e6ff6ebb94d5f54ddf5e434d2ae246d1 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,64 @@ let {FilterStorage} = abprequire("filterStorage");
*/
function TabAllocator(browser, maxtabs)
{
- browser.removeAllTabsBut(browser.tabs[0])
+ 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.
+ this._tabKeepingWindowAlive = this._browser.tabs[0];
+ this._browser.removeAllTabsBut(this._tabKeepingWindowAlive);
+}
+TabAllocator.prototype = {
+ _removeTabKeepingWindowAlive: function()
+ {
+ if (!this._tabKeepingWindowAlive)
+ return;
+ this._browser.removeTab(this._tabKeepingWindowAlive);
+ delete this._tabKeepingWindowAlive;
+ },
- this._tabs = [];
- for (let i = 0; i < maxtabs; i++)
- this._tabs.push(browser.addTab("about:blank"));
+ /**
+ * 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)
+ {
+ this._removeTabKeepingWindowAlive();
+ return Promise.resolve(tab);
Wladimir Palant 2016/09/16 07:10:37 I think that rather than introducing a _removeTabK
sergei 2016/09/16 12:34:13 It's a valid point but I would like to keep it thi
+ }
+ return new Promise((resolve, reject) =>
+ {
+ let onBrowserInit = (msg) =>
+ {
+ tab.linkedBrowser.messageManager.removeMessageListener("Browser:Init", onBrowserInit);
+ this._removeTabKeepingWindowAlive();
+ 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);
+ });
+ },
Wladimir Palant 2016/09/16 07:10:37 Nit: Don't add this extra newline please.
sergei 2016/09/16 12:34:13 Done.
- 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
- {
- let deferred = Promise.defer();
- this._deferred.push(deferred);
- return deferred.promise;
- }
+ if (this._tabs < this._maxtabs)
+ return this._createTab();
+ return new Promise((resolve, reject) => this._resolvers.push(resolve));
},
/**
@@ -71,15 +102,23 @@ TabAllocator.prototype = {
*/
releaseTab: function(tab)
{
- let browser = tab.parentNode.tabbrowser;
- browser.removeTab(tab);
- tab = browser.addTab("about:blank");
-
- if (this._deferred.length)
- this._deferred.shift().resolve(tab);
+ // If we are about to close last tab don't close it immediately to keep
+ // the window alive. It will be closed when a new tab is created.
+ if (this._tabs > 1)
+ this._browser.removeTab(tab);
else
- this._tabs.push(tab);
- }
+ {
+ // navigate away from early opened URL
+ tab.linkedBrowser.loadURI('about:blank', null, null);
Wladimir Palant 2016/09/16 07:10:37 What's the point of navigating away if we are igno
sergei 2016/09/16 12:34:13 I have a version when the crawler is always runnin
+ this._tabKeepingWindowAlive = tab;
+ }
+
+ this._tabs--;
+ if (this._resolvers.length && this._tabs < this._maxtabs)
+ {
+ this._resolvers.shift()(this._createTab());
+ }
+ },
};
/**
@@ -231,8 +270,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