Issue 6424 - Use internal symbol in ext namespace

Following up on our discussion on #6424, this is a list of changes to the code in the ext namespace to make the underscore properties "internal" using a symbol.

The symbol is defined in ext/common.js

Every object that wants to have a separate "internal" namespace referenced via this symbol must call the global defineNamespace function.

Underscore properties on class instances are now in their own internal namespace.

[Manish Jethani, 2018-02-28 15:23:27]

Patch Set 1

https://codereview.adblockplus.org/29711592/diff/29711593/composer.html
File composer.html (right):

https://codereview.adblockplus.org/29711592/diff/29711593/composer.html#newco...
composer.html:27: <script type="text/javascript" src="base.js"></script>
base.js is included everywhere we need defineNamespace, i.e. everywhere we're
including any ext/*.js files.

https://codereview.adblockplus.org/29711592/diff/29711593/ext/background.js
File ext/background.js (right):

https://codereview.adblockplus.org/29711592/diff/29711593/ext/background.js#n...
ext/background.js:18: /* global internal, defineNamespace */
We need to do this for now. Later we can include these globals in the ESLint
config.

https://codereview.adblockplus.org/29711592/diff/29711593/ext/background.js#n...
ext/background.js:27: defineNamespace(this, internal);
Call defineNamespace in the constructor.

https://codereview.adblockplus.org/29711592/diff/29711593/ext/background.js#n...
ext/background.js:60: function deletePage(pageMap, pageId)
ext.PageMap.prototype._delete is now simply deletePage. The first argument is
the ext.PageMap instance. The rest of the code is unchanged.

https://codereview.adblockplus.org/29711592/diff/29711593/ext/background.js#n...
ext/background.js:339: function queueBrowserActionChanges(browserAction)
Similarly the _queueChanges, _applyChanges, and _addChange methods in
BrowserAction.prototype and now simply regular functions in the outer scope
taking the actual BrowserAction instance as the first argument.

https://codereview.adblockplus.org/29711592/diff/29711593/ext/common.js
File ext/common.js (right):

https://codereview.adblockplus.org/29711592/diff/29711593/ext/common.js#newco...
ext/common.js:22: const internal = Symbol();
The symbol is defined in ext/common.js since it is common to the ext namespace.
It should remain inaccessible to code outside the ext namespace.

https://codereview.adblockplus.org/29711592/diff/29711593/ext/common.js#newco...
ext/common.js:46: ext[internal].dispatchEvent = (eventTarget, ...args) =>
EventTarget.prototype._dispatch is now ext[internal].dispatchEvent

https://codereview.adblockplus.org/29711592/diff/29711593/lib/requestBlocker.js
File lib/requestBlocker.js (left):

https://codereview.adblockplus.org/29711592/diff/29711593/lib/requestBlocker....
lib/requestBlocker.js:221: return ext.webRequest.onBeforeRequest._dispatch(
This is one instance of code outside the ext namespace "illegally" accessing its
internal members. Luckily we can just call our own code here, we don't have to
check if the request should be blocked elsewhere.

[Manish Jethani, 2018-02-28 15:27:41]

Patch Set 2: Add base.js

This new file defines a window.defineNamespace function that creates a
non-enumerable, non-writable, non-configurable property on the given object.
This is useful for creating a "namespace" on the object. The namespace is really
just an empty object with a null prototype.

[Manish Jethani, 2018-02-28 16:35:48]

Patch Set 3: Support internal methods

This change allows us to define an internal namespace on class prototypes as
well.

[Manish Jethani, 2018-03-01 12:04:14]

Patch Set 4: Switch to internal methods everywhere

https://codereview.adblockplus.org/29711592/diff/29712590/ext/background.js
File ext/background.js (right):

https://codereview.adblockplus.org/29711592/diff/29712590/ext/background.js#n...
ext/background.js:328: {
This is mostly just an indentation change. this._applyChanges,
this._queueChanges, and this._addChange are now this[internal].applyChanges,
this[internal].queueChanges, and this[internal].addChange and that's all.