Issue 1706 - Move first-run page to adblockplusui repository

Most of the changes here is removing files that moved. Requiring Firefox 25 because of https://bugzilla.mozilla.org/show_bug.cgi?id=887781.

[Sebastian Noack, 2015-01-07 10:27:52]

http://codereview.adblockplus.org/5056848617013248/diff/5629499534213120/ext/...
File ext/common.js (right):

http://codereview.adblockplus.org/5056848617013248/diff/5629499534213120/ext/...
ext/common.js:22: Object.defineProperty(global, "Page", {
We only expose Page objects in the background page on Chrome/Opera/Safari. Any
reason you do so in ext/common.js here?

http://codereview.adblockplus.org/5056848617013248/diff/5629499534213120/ext/...
ext/common.js:125: function EventTarget(cancelable)
The argument is unused.

http://codereview.adblockplus.org/5056848617013248/diff/5629499534213120/ext/...
ext/common.js:153: {
What is this branch good for?

[Thomas Greiner, 2015-01-07 13:51:37]

LGTM with Sebastian's points addressed

[Wladimir Palant, 2015-01-07 16:36:49]

I addressed Sebastian's comments and also fixed the code removing observer.

http://codereview.adblockplus.org/5056848617013248/diff/5629499534213120/ext/...
File ext/common.js (right):

http://codereview.adblockplus.org/5056848617013248/diff/5629499534213120/ext/...
ext/common.js:22: Object.defineProperty(global, "Page", {
On 2015/01/07 10:27:52, Sebastian Noack wrote:
> We only expose Page objects in the background page on Chrome/Opera/Safari. Any
> reason you do so in ext/common.js here?

MessageProxy._handleRequest() below needs to use *something* as sender
parameter. And rather than checking "are we running in background" every time a
message is handled I'd prefer to have a dummy page constructor in content.
However, having the memoization on the global object is indeed misguided - it
should really be a local thing.

http://codereview.adblockplus.org/5056848617013248/diff/5629499534213120/ext/...
ext/common.js:153: {
On 2015/01/07 10:27:52, Sebastian Noack wrote:
> What is this branch good for?

This file is included either as CommonJS module (by the "background page", via
require("ext_common")) or a regular script. In the first scenario it needs to
export variables by added in them to the global exports variable, in the second
it needs to change global.ext.

[Sebastian Noack, 2015-01-07 17:00:39]

http://codereview.adblockplus.org/5056848617013248/diff/5629499534213120/ext/...
File ext/common.js (right):

http://codereview.adblockplus.org/5056848617013248/diff/5629499534213120/ext/...
ext/common.js:22: Object.defineProperty(global, "Page", {
On 2015/01/07 16:36:49, Wladimir Palant wrote:
> On 2015/01/07 10:27:52, Sebastian Noack wrote:
> > We only expose Page objects in the background page on Chrome/Opera/Safari.
Any
> > reason you do so in ext/common.js here?
> 
> MessageProxy._handleRequest() below needs to use *something* as sender
> parameter. And rather than checking "are we running in background" every time
a
> message is handled I'd prefer to have a dummy page constructor in content.
> However, having the memoization on the global object is indeed misguided - it
> should really be a local thing.

I'm not sure if I correctly understand the problem. But on Chrome/Opera/Safari
|sender.page| is undefined when the message is sent by the background page.
However, I didn't check explicitly whether we are in the background page, but
abstracted that code path in content.js/background.js.

http://codereview.adblockplus.org/5056848617013248/diff/5629499534213120/ext/...
ext/common.js:153: {
On 2015/01/07 16:36:49, Wladimir Palant wrote:
> On 2015/01/07 10:27:52, Sebastian Noack wrote:
> > What is this branch good for?
> 
> This file is included either as CommonJS module (by the "background page", via
> require("ext_common")) or a regular script. In the first scenario it needs to
> export variables by added in them to the global exports variable, in the
second
> it needs to change global.ext.

Why not |exports = ext|?

[Wladimir Palant, 2015-01-07 19:01:19]

http://codereview.adblockplus.org/5056848617013248/diff/5629499534213120/ext/...
File ext/common.js (right):

http://codereview.adblockplus.org/5056848617013248/diff/5629499534213120/ext/...
ext/common.js:22: Object.defineProperty(global, "Page", {
On 2015/01/07 17:00:39, Sebastian Noack wrote:
> I'm not sure if I correctly understand the problem. But on Chrome/Opera/Safari
> |sender.page| is undefined when the message is sent by the background page.
> However, I didn't check explicitly whether we are in the background page, but
> abstracted that code path in content.js/background.js.

Yes, the way this is implemented in Safari the sender isn't generated in the
message proxy but rather passed in from outside - that's because the message
proxy doesn't listen to the messages itself, it is rather being triggered by
message listeners defined elsewhere. I consider this a rather awkward solution,
related logic being spread out throughout the code. Here on the other hand the
message proxy registers its own listeners, so having different handling for
background and content isn't quite as straightforward.

http://codereview.adblockplus.org/5056848617013248/diff/5629499534213120/ext/...
ext/common.js:153: {
On 2015/01/07 17:00:39, Sebastian Noack wrote:
> Why not |exports = ext|?

Surprisingly enough, that actually works with our module loader. Let's go with
this for consistency, even though I'd rather have this export MessageProxy
rather than _MessageProxy.

[Thomas Greiner, 2015-01-08 09:59:11]

LGTM