1249642 - Enable environment selection in aboutNewTabService.js for remote URLs

Status: RESOLVED FIXED

(Firefox :: New Tab Page, defect)

Description

[Olivier Yiptong [:oyiptong]]

Given that we're locking down the remote URL for about:newtab, we still need to be able to change the URLs depending on "environments".

Use-cases:

1. development: where we want about:newtab to point to localhost.
2. test: we want to point to mochitest URL locations
3. staging: for CI to test pre-production resources
4. production: for all users

Given the update mechanisms, we also need to make the remote newtab versioning live in a pref. This allows for updates via addons, which will be needed after a signing key update or just an update of the page.

The environments are described in bug 1239118, so there should be some consolidation with the code there.

Note, this only affects remote newtab URLs and this codepath is only ever run when /browser's AboutRedirector is hit.

Comment 1

[Ricky Rosario [:rrosario, :r1cky]]

Attachment: MozReview Request: Bug 1249642 - Implement remote modes in aboutNewTabService.js r?oyiptong

Review commit: https://reviewboard.mozilla.org/r/37213/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/37213/

Comment 2

[Olivier Yiptong [:oyiptong]]

Comment on attachment 8724889 [details]
MozReview Request: Bug 1249642 - Implement remote modes in aboutNewTabService.js r?oyiptong

https://reviewboard.mozilla.org/r/37213/#review33761

Good stuff! r+ with a couple of nits

::: browser/components/newtab/NewTabRemoteResources.jsm:12
(Diff revision 1)
> +  "dev": {origin: "http://localhost"}

Maybe http://localhost:5000 or a port that isn't privileged might be more dev friendly

::: browser/components/newtab/aboutNewTabService.js:161
(Diff revision 1)
> +    if (!MODE_CHANNEL_MAP.hasOwnProperty(mode)) {

nit: please use the `in` operator:

```javascript
if (! mode in MODE_CHANNEL_MAP) {
  ...
}
```

Comment 3

[Ricky Rosario [:rrosario, :r1cky]]

https://reviewboard.mozilla.org/r/37213/#review33761

> nit: please use the `in` operator:
> 
> ```javascript
> if (! mode in MODE_CHANNEL_MAP) {
>   ...
> }
> ```

That's what I had originally and it never goes inside the if statement (`mode in MODE_CHANNEL_MAP` always returns true). I just tried again and the tests blow up becuase it ends up trying a non existing mode. I also tried `if (! mode in Object.keys(MODE_CHANNEL_MAP)) {`. Puzzling.

```
 0:00.62 LOG: Thread-1 INFO "CONSOLE_MESSAGE: (error) [JavaScript Error: "TypeError: MODE_CHANNEL_MAP[mode] is undefined: generateRemoteURL@file:///Users/rlr/dev/newtab-dev/obj-ff-dbg/dist/NightlyDebug.app/Contents/Resources/browser/components/aboutNewTabService.js:164:5
_updateRemoteMaybe@file:///Users/rlr/dev/newtab-dev/obj-ff-dbg/dist/NightlyDebug.app/Contents/Resources/browser/components/aboutNewTabService.js:192:15
EventEmitter_emit@resource://devtools/shared/event-emitter.js:147:11
observe@resource:///modules/NewTabPrefsProvider.jsm:43:13
Preferences._set@resource://gre/modules/Preferences.jsm:123:9
Preferences.set@resource://gre/modules/Preferences.jsm:109:3
test_updates@/Users/rlr/dev/newtab-dev/obj-ff-dbg/_tests/xpcshell/browser/components/newtab/tests/xpcshell/test_AboutNewTabService.js:141:3
TaskImpl_run@resource://gre/modules/Task.jsm:319:40
_do_main@/Users/rlr/dev/newtab-dev/testing/xpcshell/head.js:209:5
_execute_test@/Users/rlr/dev/newtab-dev/testing/xpcshell/head.js:533:5
@-e:1:1
" {file: "resource://devtools/shared/event-emitter.js" line: 152}]
EventEmitter_emit@resource://devtools/shared/event-emitter.js:152:11
observe@resource:///modules/NewTabPrefsProvider.jsm:43:13
Preferences._set@resource://gre/modules/Preferences.jsm:123:9
Preferences.set@resource://gre/modules/Preferences.jsm:109:3
test_updates@/Users/rlr/dev/newtab-dev/obj-ff-dbg/_tests/xpcshell/browser/components/newtab/tests/xpcshell/test_AboutNewTabService.js:141:3
TaskImpl_run@resource://gre/modules/Task.jsm:319:40
_do_main@/Users/rlr/dev/newtab-dev/testing/xpcshell/head.js:209:5
_execute_test@/Users/rlr/dev/newtab-dev/testing/xpcshell/head.js:533:5
@-e:1:1
```

Comment 4

[Ricky Rosario [:rrosario, :r1cky]]

https://reviewboard.mozilla.org/r/37213/#review33761

> That's what I had originally and it never goes inside the if statement (`mode in MODE_CHANNEL_MAP` always returns true). I just tried again and the tests blow up becuase it ends up trying a non existing mode. I also tried `if (! mode in Object.keys(MODE_CHANNEL_MAP)) {`. Puzzling.
> 
> ```
>  0:00.62 LOG: Thread-1 INFO "CONSOLE_MESSAGE: (error) [JavaScript Error: "TypeError: MODE_CHANNEL_MAP[mode] is undefined: generateRemoteURL@file:///Users/rlr/dev/newtab-dev/obj-ff-dbg/dist/NightlyDebug.app/Contents/Resources/browser/components/aboutNewTabService.js:164:5
> _updateRemoteMaybe@file:///Users/rlr/dev/newtab-dev/obj-ff-dbg/dist/NightlyDebug.app/Contents/Resources/browser/components/aboutNewTabService.js:192:15
> EventEmitter_emit@resource://devtools/shared/event-emitter.js:147:11
> observe@resource:///modules/NewTabPrefsProvider.jsm:43:13
> Preferences._set@resource://gre/modules/Preferences.jsm:123:9
> Preferences.set@resource://gre/modules/Preferences.jsm:109:3
> test_updates@/Users/rlr/dev/newtab-dev/obj-ff-dbg/_tests/xpcshell/browser/components/newtab/tests/xpcshell/test_AboutNewTabService.js:141:3
> TaskImpl_run@resource://gre/modules/Task.jsm:319:40
> _do_main@/Users/rlr/dev/newtab-dev/testing/xpcshell/head.js:209:5
> _execute_test@/Users/rlr/dev/newtab-dev/testing/xpcshell/head.js:533:5
> @-e:1:1
> " {file: "resource://devtools/shared/event-emitter.js" line: 152}]
> EventEmitter_emit@resource://devtools/shared/event-emitter.js:152:11
> observe@resource:///modules/NewTabPrefsProvider.jsm:43:13
> Preferences._set@resource://gre/modules/Preferences.jsm:123:9
> Preferences.set@resource://gre/modules/Preferences.jsm:109:3
> test_updates@/Users/rlr/dev/newtab-dev/obj-ff-dbg/_tests/xpcshell/browser/components/newtab/tests/xpcshell/test_AboutNewTabService.js:141:3
> TaskImpl_run@resource://gre/modules/Task.jsm:319:40
> _do_main@/Users/rlr/dev/newtab-dev/testing/xpcshell/head.js:209:5
> _execute_test@/Users/rlr/dev/newtab-dev/testing/xpcshell/head.js:533:5
> @-e:1:1
> ```

Oops, using `in` with an `Array` doesn't do what I expected. But still puzzling that the `! mode in MODE_CHANNEL_MAP` isn't working.

Comment 5

[Ricky Rosario [:rrosario, :r1cky]]

Comment on attachment 8724889 [details]
MozReview Request: Bug 1249642 - Implement remote modes in aboutNewTabService.js r?oyiptong

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/37213/diff/1-2/

Comment 6

[Ricky Rosario [:rrosario, :r1cky]]

Comment on attachment 8724889 [details]
MozReview Request: Bug 1249642 - Implement remote modes in aboutNewTabService.js r?oyiptong

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/37213/diff/2-3/

Comment 7

[Ricky Rosario [:rrosario, :r1cky]]

https://reviewboard.mozilla.org/r/37213/#review33761

> Oops, using `in` with an `Array` doesn't do what I expected. But still puzzling that the `! mode in MODE_CHANNEL_MAP` isn't working.

Ahhh .I was missing a parens... `if (!(mode in MODE_CHANNEL_MAP)) {` works.

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0010ab7a28fa

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0010ab7a28fa