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

Delta Between Two Patch Sets: lib/crawler.js

Issue 29338442: Issue 3815 - Fix TabAllocator. Now it returns tab with initialized outerWindowID (Closed)
Left Patch Set: Created March 16, 2016, 2:18 p.m.
Right Patch Set: remove additional empty line Created Sept. 16, 2016, 12:33 p.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
Left: Side by side diff | Download
Right: Side by side diff | Download
« no previous file with change/comment | « no previous file | run.py » ('j') | no next file with change/comment »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
LEFTRIGHT
1 /* 1 /*
2 * This Source Code is subject to the terms of the Mozilla Public License 2 * This Source Code is subject to the terms of the Mozilla Public License
3 * version 2.0 (the "License"). You can obtain a copy of the License at 3 * version 2.0 (the "License"). You can obtain a copy of the License at
4 * http://mozilla.org/MPL/2.0/. 4 * http://mozilla.org/MPL/2.0/.
5 */ 5 */
6 6
7 /** 7 /**
8 * @module crawler 8 * @module crawler
9 */ 9 */
10 10
11 Cu.import("resource://gre/modules/Services.jsm"); 11 Cu.import("resource://gre/modules/Services.jsm");
12 Cu.import("resource://gre/modules/Task.jsm"); 12 Cu.import("resource://gre/modules/Task.jsm");
13 Cu.import("resource://gre/modules/Promise.jsm"); 13 Cu.import("resource://gre/modules/Promise.jsm");
Wladimir Palant 2016/03/16 14:52:52 Please remove this if you don't use Promise.defer(
sergei 2016/03/16 15:47:00 Currently defer is still used by LoadListener.
14 14
15 function abprequire(module) 15 function abprequire(module)
16 { 16 {
17 let result = {}; 17 let result = {};
18 result.wrappedJSObject = result; 18 result.wrappedJSObject = result;
19 Services.obs.notifyObservers(result, "adblockplus-require", module); 19 Services.obs.notifyObservers(result, "adblockplus-require", module);
20 return result.exports; 20 return result.exports;
21 } 21 }
22 22
23 let {RequestNotifier} = abprequire("requestNotifier"); 23 let {RequestNotifier} = abprequire("requestNotifier");
24 let {FilterNotifier} = abprequire("filterNotifier"); 24 let {FilterNotifier} = abprequire("filterNotifier");
25 let {FilterStorage} = abprequire("filterStorage"); 25 let {FilterStorage} = abprequire("filterStorage");
26 26
27 /** 27 /**
28 * Allocates a new tab "about:blank" in the `browser`. 28 * Allocates tabs on request but not more than maxtabs at the same time.
Wladimir Palant 2016/03/16 14:52:52 Please don't use backticks, these have no special
sergei 2016/03/16 15:47:00 Done.
29 * The method returns a {Promise} promise which is resolved with the `tab`
Wladimir Palant 2016/03/16 14:52:52 "{Promise} promise" => "promise" please, the type
sergei 2016/03/16 15:47:00 Done.
30 * when `outerWindowID` is already initialized.
Wladimir Palant 2016/03/16 14:52:52 Too many implementation details here. "which is re
sergei 2016/03/16 15:47:00 Done.
31 *
32 * See: https://bugzilla.mozilla.org/show_bug.cgi?id=1256602#c1
Wladimir Palant 2016/03/16 14:52:52 This belongs into a comment above the actual imple
sergei 2016/03/16 15:47:00 Done.
33 *
34 * @param {tabbrowser} browser
35 * The tabbed browser where tabs should be created
Wladimir Palant 2016/03/16 14:52:52 "tabs" => "the tab"
sergei 2016/03/16 15:47:01 Done.
36 * @return {Promise} promise which will be resolved with the tab.
Wladimir Palant 2016/03/16 14:52:52 Please use the more specific Promise.<tab> as type
sergei 2016/03/16 15:47:00 Done.
37 */
38 function createTab(browser)
39 {
40 let tab = browser.addTab("about:blank");
41 if (tab.linkedBrowser.outerWindowID)
42 return Promise.resolve(tab);
43 return new Promise((resolve, reject)=>
Wladimir Palant 2016/03/16 14:52:52 Nit: Space before => please.
sergei 2016/03/16 15:47:00 Done.
44 {
45 let onBrowserInit = (msg) =>
46 {
47 tab.linkedBrowser.messageManager.removeMessageListener("Browser:Init", onB rowserInit);
48 resolve(tab);
49 };
50 tab.linkedBrowser.messageManager.addMessageListener("Browser:Init", onBrowse rInit);
51 });
52 }
53
54 /**
55 * Allocates tabs on request but not more than `maxtabs` at the same time.
56 * 29 *
57 * @param {tabbrowser} browser 30 * @param {tabbrowser} browser
58 * The tabbed browser where tabs should be created 31 * The tabbed browser where tabs should be created
59 * @param {int} maxtabs 32 * @param {int} maxtabs
60 * The maximum number of tabs to be allocated 33 * The maximum number of tabs to be allocated
61 * @constructor 34 * @constructor
62 */ 35 */
63 function TabAllocator(browser, maxtabs) 36 function TabAllocator(browser, maxtabs)
64 { 37 {
65 this._browser = browser; 38 this._browser = browser;
66 this._tabs = 0; 39 this._tabs = 0;
67 this._maxtabs = maxtabs; 40 this._maxtabs = maxtabs;
68 // the array of `resolve` functions of {Promise} promises returned by `getTab` . 41 // The queue containing resolve functions of promises waiting for a tab.
Wladimir Palant 2016/03/16 14:52:52 This is a code comment, it's intended to be read b
sergei 2016/03/16 15:47:01 Sounds good!
69 this._resolvers = []; 42 this._resolvers = [];
70 // Keep at least one tab alive to prevent browser from closing of it self. 43 // Keep at least one tab alive to prevent browser from closing itself.
Wladimir Palant 2016/03/16 14:52:52 "browser from closing of it self" => "the browser
sergei 2016/03/16 15:47:00 Done.
71 // That tab will be removed when the first tab is requested. 44 this._tabKeepingWindowAlive = this._browser.tabs[0];
72 browser.removeAllTabsBut(browser.tabs[0]) 45 this._browser.removeAllTabsBut(this._tabKeepingWindowAlive);
73 } 46 }
74 TabAllocator.prototype = { 47 TabAllocator.prototype = {
48 _removeTabKeepingWindowAlive: function()
49 {
50 if (!this._tabKeepingWindowAlive)
51 return;
52 this._browser.removeTab(this._tabKeepingWindowAlive);
53 delete this._tabKeepingWindowAlive;
54 },
55
56 /**
57 * Creates a blank tab in this._browser.
58 *
59 * @return {Promise.<tab>} promise which resolves once the tab is fully initia lized.
60 */
61 _createTab: function()
62 {
63 this._tabs++;
64 let tab = this._browser.addTab("about:blank");
65 if (tab.linkedBrowser.outerWindowID)
66 {
67 this._removeTabKeepingWindowAlive();
68 return Promise.resolve(tab);
69 }
70 return new Promise((resolve, reject) =>
71 {
72 let onBrowserInit = (msg) =>
73 {
74 tab.linkedBrowser.messageManager.removeMessageListener("Browser:Init", o nBrowserInit);
75 this._removeTabKeepingWindowAlive();
76 resolve(tab);
77 };
78 // "Browser:Init" message is sent once the browser is ready, see
79 // https://bugzil.la/1256602#c1
80 tab.linkedBrowser.messageManager.addMessageListener("Browser:Init", onBrow serInit);
81 });
82 },
83
75 /** 84 /**
76 * Returns a promise that will resolve into a tab once a tab is allocated. 85 * Returns a promise that will resolve into a tab once a tab is allocated.
77 * The tab cannot be used by other tasks until releaseTab() is called. 86 * The tab cannot be used by other tasks until releaseTab() is called.
78 * 87 *
79 * @result {Promise} 88 * @result {Promise.<tab>}
Wladimir Palant 2016/03/16 14:52:52 Nit: Promise.<tab> here as well.
80 */ 89 */
81 getTab: function() 90 getTab: function()
82 { 91 {
83 if (this._tabs < this._maxtabs) 92 if (this._tabs < this._maxtabs)
84 { 93 return this._createTab();
85 let tab = createTab(this._browser); 94 return new Promise((resolve, reject) => this._resolvers.push(resolve));
86 // close initial tab, we don't need it anymore.
Wladimir Palant 2016/03/16 14:52:52 Nit: Please start sentences with a capital letter.
87 if (this._tabs == 0)
88 this._browser.removeTab(this._browser.tabs[0]);
89 this._tabs++;
90 return tab;
91 }
92 return new Promise((resolve, reject) =>
93 {
94 this._resolvers.push(resolve);
95 });
96 }, 95 },
97 96
98 /** 97 /**
99 * Adds a tab back to the pool so that it can be used by other tasks. 98 * Adds a tab back to the pool so that it can be used by other tasks.
100 * 99 *
101 * @param {tab} tab 100 * @param {tab} tab
102 */ 101 */
103 releaseTab: function(tab) 102 releaseTab: function(tab)
104 { 103 {
105 let browser = tab.parentNode.tabbrowser; 104 // If we are about to close last tab don't close it immediately to keep
106 browser.removeTab(tab); 105 // the window alive. It will be closed when a new tab is created.
107 106 if (this._tabs > 1)
108 if (this._resolvers.length) 107 this._browser.removeTab(tab);
109 this._resolvers.shift()(createTab(this._browser));
110 else 108 else
111 this._tabs--; 109 {
Wladimir Palant 2016/03/16 14:52:52 Now you have the problem that in the theoretical s
sergei 2016/03/16 15:47:00 Yes. However I think it should be addressed at ano
sergei 2016/04/11 15:20:22 I have addressed that issue. BTW, having an array
112 } 110 // navigate away from early opened URL
111 tab.linkedBrowser.loadURI('about:blank', null, null);
112 this._tabKeepingWindowAlive = tab;
113 }
114
115 this._tabs--;
116 if (this._resolvers.length && this._tabs < this._maxtabs)
117 {
118 this._resolvers.shift()(this._createTab());
119 }
120 },
113 }; 121 };
114 122
115 /** 123 /**
116 * Observes page loads in a particular tabbed browser. 124 * Observes page loads in a particular tabbed browser.
117 * 125 *
118 * @param {tabbrowser} browser 126 * @param {tabbrowser} browser
119 * The tabbed browser to be observed 127 * The tabbed browser to be observed
120 * @param {int} timeout 128 * @param {int} timeout
121 * Load timeout in milliseconds 129 * Load timeout in milliseconds
122 * @constructor 130 * @constructor
(...skipping 131 matching lines...) Expand 10 before | Expand all | Expand 10 after
254 resolve(); 262 resolve();
255 } 263 }
256 }; 264 };
257 FilterNotifier.addListener(onFiltersLoaded); 265 FilterNotifier.addListener(onFiltersLoaded);
258 }).then(() => crawl_urls(window, urls, timeout, maxtabs, targetURL, onDone)) 266 }).then(() => crawl_urls(window, urls, timeout, maxtabs, targetURL, onDone))
259 .catch(reportException); 267 .catch(reportException);
260 } 268 }
261 exports.run = run; 269 exports.run = run;
262 270
263 /** 271 /**
264 * Spawns a {Task} task to crawl each url from `urls` argument and calls 272 * Spawns a {Task} task to crawl each url from urls argument and calls
265 * `onDone` when all tasks are finished. 273 * onDone when all tasks are finished.
266 * @param {Window} window 274 * @param {Window} window
267 * The browser window we're operating in 275 * The browser window we're operating in
268 * @param {String[]} urls 276 * @param {String[]} urls
269 * URLs to be crawled 277 * URLs to be crawled
270 * @param {int} timeout 278 * @param {int} timeout
271 * Load timeout in milliseconds 279 * Load timeout in milliseconds
272 * @param {int} maxtabs 280 * @param {int} maxtabs
273 * Maximum number of tabs to be opened 281 * Maximum number of tabs to be opened
274 * @param {String} targetURL 282 * @param {String} targetURL
275 * URL that should receive the results 283 * URL that should receive the results
(...skipping 110 matching lines...) Expand 10 before | Expand all | Expand 10 after
386 394
387 function reportException(e) 395 function reportException(e)
388 { 396 {
389 let stack = ""; 397 let stack = "";
390 if (e && typeof e == "object" && "stack" in e) 398 if (e && typeof e == "object" && "stack" in e)
391 stack = e.stack + "\n"; 399 stack = e.stack + "\n";
392 400
393 Cu.reportError(e); 401 Cu.reportError(e);
394 dump(e + "\n" + stack + "\n"); 402 dump(e + "\n" + stack + "\n");
395 } 403 }
LEFTRIGHT
« no previous file | run.py » ('j') | Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Toggle Comments ('s')

Powered by Google App Engine
This is Rietveld