Closed Bug 1247435 Opened 8 years ago Closed 8 years ago

Implement browser.runtime.onStartup

Categories

(WebExtensions :: General, defect, P3)

defect

Tracking

(firefox52 verified)

VERIFIED FIXED
mozilla52
Tracking Status
firefox52 --- verified

People

(Reporter: steve, Assigned: mattw)

References

Details

(Whiteboard: [runtime][triaged])

Attachments

(2 files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:46.0) Gecko/20100101 Firefox/46.0
Build ID: 20160210004006

Steps to reproduce:

Firefox Developer Edition 46.0a2 (2016-02-10)

My extension registers a function in a background page:
    chrome.runtime.onStartup.addListener(my_function);

Docs imply onStartup is supported https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/runtime#Chrome_incompatibilities


Actual results:

The browser console reports error
    TypeError: chrome.runtime.onStartup is undefined
when the add-on is installed.

onStartup does not appear when chrome.runtime is dumped to string:
{"PlatformOs":{"MAC":"mac","WIN":"win","ANDROID":"android","CROS":"cros","LINUX":"linux","OPENBSD":"openbsd"},"PlatformArch":{"ARM":"arm","X86-32":"x86-32","X86-64":"x86-64"},"RequestUpdateCheckStatus":{"THROTTLED":"throttled","NO_UPDATE":"no_update","UPDATE_AVAILABLE":"update_available"},"OnInstalledReason":{"INSTALL":"install","UPDATE":"update","CHROME_UPDATE":"chrome_update","SHARED_MODULE_UPDATE":"shared_module_update"},"OnRestartRequiredReason":{"APP_UPDATE":"app_update","OS_UPDATE":"os_update","PERIODIC":"periodic"},"onConnect":{},"onMessage":{}}


Expected results:

No console error, the listener should be registered.

It should appear in dump "onStartup":{}
OS: Unspecified → Mac OS X
Hardware: Unspecified → x86_64
Component: Untriaged → WebExtensions
Product: Firefox → Toolkit
OS: Mac OS X → All
Hardware: x86_64 → All
Summary: WebExtension background page, TypeError: chrome.runtime.onStartup is undefined → Implement browser.runtime.onStartup
Whiteboard: [runtime]
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking-webextensions-
Priority: -- → P3
Whiteboard: [runtime] → [runtime] triaged
Whiteboard: [runtime] triaged → [runtime][berlin] triaged
Assignee: nobody → aswan
Whiteboard: [runtime][berlin] triaged → [runtime][triaged]
Assignee: aswan → mwein
There is a runtime.onStartup implementation, but it is more like a DOMContentLoaded event than Chrome's onStartup event.

http://searchfox.org/mozilla-central/rev/65bed54efcce67cf04a890f7fe253ccdfa6befdc/toolkit/components/extensions/ext-backgroundPage.js#71
    // TODO: Right now we run onStartup after the background page
    // finishes. See if this is what Chrome does.

runtime.onStartup should be fired if 1) an extension is installed AND 2) the browser starts up for the first time.
It was implemented in Chrome at https://crbug.com/132328


There is another related signal that is currently not implemented in Firefox.
chrome.runtime.onInstalled should be fired when an extension is installed/updated or the browser is updated.
This event is the documented way to register rules for rule-based APIs (contextMenus, declarativeContent, declarativeWebRequest), where the rules are persisted until the extension is reloaded.

We don't support event pages in Firefox at the moment, so it is better to replace what we now call onStartup with onInstalled, and implement onStartup in the way that I described.
Component: WebExtensions: Untriaged → WebExtensions: General
Flags: blocking-webextensions-
Version: 46 Branch → unspecified
The magic actions for youtube is no longer functionallity in the settings page and dos not work with the current new firefox versions like nightly, developer edition, aurora, beta and the stable edition from firefox (I have checked all)!
I can't install this addon (The icon is not shown in the icon bar from firefox) onli in the addons page!
And no strings, booleans and integers are placed in the prefs.js file :(
Or is the addon blocked by the firefox developers?! I don't know :(
I hope the developers fron magic actions for youtube and the developers from mozilla firefox work together to fix the problem with the addon itself and mozilla firefox too.

I have contacted the developers too (For support) for their awesome addon.

ps:The magic actions for youtube is a great addon :)
Depends on: 1252871
Comment on attachment 8806546 [details]
Bug 1247435 - Add support for runtime.onStartup

https://reviewboard.mozilla.org/r/89940/#review89618

::: toolkit/components/extensions/ExtensionChild.jsm:164
(Diff revision 2)
>      this.viewType = viewType;
>      this.uri = uri || extension.baseURI;
>  
>      this.setContentWindow(contentWindow);
>  
> +    this.incognito = PrivateBrowsingUtils.isContentWindowPrivate(contentWindow);

setContentWindow should be responsible for setting incognito context.incognito, see 1309610

If all suggested steps in that bug are followed, then you don't need runtime_internal method (because context.incognito is then set both in the child and parent).

Do you think that you can submit a separate patch for that bug (if Kris doesn't have time to fix it)?
Comment on attachment 8806546 [details]
Bug 1247435 - Add support for runtime.onStartup

https://reviewboard.mozilla.org/r/89940/#review89930

::: toolkit/components/extensions/ext-c-runtime.js:20
(Diff revision 2)
> +      onStartup: new SingletonEventManager(context, "runtime.onStartup", fire => {
> +        let listener = () => {
> +          if (!context.incognito) {
> +            fire();
> +          }
> +        };
> +        context.childManager.getParentEvent("runtime_internal.onStartup").addListener(listener);
> +        return () => {
> +          context.childManager.getParentEvent("runtime_internal.onStartup").removeListener(listener);
> +        };
> +      }).api(),

There shouldn't be any particular need to implement this in the child process. Just initialize the proxy context in the parent process with the incognito property from the child.

::: toolkit/components/extensions/ext-c-runtime.js:22
(Diff revision 2)
>  
>        onMessage: context.messenger.onMessage("runtime.onMessage"),
>  
> +      onStartup: new SingletonEventManager(context, "runtime.onStartup", fire => {
> +        let listener = () => {
> +          if (!context.incognito) {

No need to even add the listener in this case.

::: toolkit/components/extensions/ext-runtime.js:116
(Diff revision 2)
>      },
> +    runtime_internal: {

No need for a `runtime_internal` namespace. Also, please add a blank line.

::: toolkit/components/extensions/ext-runtime.js:120
(Diff revision 2)
>        },
>      },
> +    runtime_internal: {
> +      onStartup: new SingletonEventManager(context, "runtime_internal.onStartup", fire => {
> +        let listener = () => {
> +          if (extension.startupReason == "APP_STARTUP") {

Nit: s/==/===/

::: toolkit/components/extensions/schemas/runtime.json:594
(Diff revision 2)
>          ]
>        }
>      ]
> +  },
> +  {
> +    "namespace": "runtime_internal",

This is unnecessary.

::: toolkit/components/extensions/test/xpcshell/test_ext_runtime_onInstalled_and_onStartup.js:48
(Diff revision 2)
> +  extension.sendMessage("get-on-installed-details");
> +  let details = yield extension.awaitMessage("on-installed-details");
> +  if (onInstalledReason) {
> +    equal(details.reason, onInstalledReason, "runtime.onInstalled fired with the correct reason");
> +  } else {
> +    equal(details, onInstalledReason, "runtime.onInstalled should not have fired");

I don't understand this assertion...

::: toolkit/components/extensions/test/xpcshell/test_ext_runtime_onInstalled_and_onStartup.js:53
(Diff revision 2)
> +  if (onStartupFired) {
> +    ok(fired, "runtime.onStartup should have fired");
> +  } else {
> +    ok(!fired, "runtime.onStartup should not have fired");
> +  }

`equal(fired, onStartupFired, ...)`

::: toolkit/components/extensions/test/xpcshell/test_ext_runtime_onInstalled_and_onStartup.js:100
(Diff revision 2)
> +        if (message == "get-on-installed-details") {
> +          browser.test.sendMessage("on-installed-details", onInstalledDetails);
> +        } else if (message == "did-on-startup-fire") {

s/==/===/

Same for below.
Attachment #8806546 - Flags: review?(kmaglione+bmo)
Comment on attachment 8806546 [details]
Bug 1247435 - Add support for runtime.onStartup

https://reviewboard.mozilla.org/r/89940/#review89618

> setContentWindow should be responsible for setting incognito context.incognito, see 1309610
> 
> If all suggested steps in that bug are followed, then you don't need runtime_internal method (because context.incognito is then set both in the child and parent).
> 
> Do you think that you can submit a separate patch for that bug (if Kris doesn't have time to fix it)?

Kris suggested I fix context.incognito as part of this bug and I just uploaded a new patch which I think fixes it. Could we maybe repurpose bug 1309610 to just handle adding tests for the context.incognito property?
Comment on attachment 8807547 [details]
Bug 1247435 - Fix context.incognito (this completes part of bug 1309610)

https://reviewboard.mozilla.org/r/90694/#review90636
Attachment #8807547 - Flags: review?(kmaglione+bmo) → review+
Comment on attachment 8806546 [details]
Bug 1247435 - Add support for runtime.onStartup

https://reviewboard.mozilla.org/r/89940/#review90638

::: toolkit/components/extensions/ext-runtime.js:26
(Diff revision 5)
>    let {extension} = context;
>    return {
>      runtime: {
> -      onStartup: ignoreEvent(context, "runtime.onStartup"),
> +      onStartup: new SingletonEventManager(context, "runtime.onStartup", fire => {
> +        let listener = () => {
> +          if (extension.startupReason === "APP_STARTUP" && !context.incognito) {

Just check `context.incognito` before adding the "startup" listener. There's no point in adding it when we know it won't be needed.
Attachment #8806546 - Flags: review?(kmaglione+bmo) → review+
Keywords: checkin-needed
Blocks: 1309610
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/7ac1ed5691cf
Fix context.incognito (this completes part of bug 1309610) r=kmag
https://hg.mozilla.org/integration/autoland/rev/c5d86a74509d
Add support for runtime.onStartup r=kmag
Keywords: checkin-needed
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/f0c05b86175b
Add support for runtime.onStartup: Add it to API list. r=bustage-fix on a CLOSED TREE
I was able to reproduce the initial issue on Firefox 51.0a2 (2016-11-08) under Windows 10.

Verified fixed on Firefox 52.0a1 (2016-11-08) under Windows 10 64-bit and Ubuntu 16.04 32-bit. 
Tested using several webextensions and “TypeError: chrome.runtime.onStartup is undefined” is no longer thrown in Browser Console.
Status: RESOLVED → VERIFIED
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.