843910 - window observer notifications

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect, P2)

Description

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

At the moment nsIWindowWatcher can be used to observe windows being opened /
closed via "domwindowopened" / "domwindowclosed" notifications. 

https://developer.mozilla.org/en-US/docs/XPCOM_Interface_Reference/nsIWindowWatcher#registerNotification%28%29

This is used by everything in SDK to register further event handlers for things that happen in those windows. It would be much better if those notifications were just dispatched on observer service itself.

Comment 1

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

As a matter of fact it looks like "domwindowopened" and "domwindowclosed" notifications are actually dispatched.

Comment 2

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

I have to say that we still need something for focus events.

Comment 3

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

So we need observer notifications for:

- focus
- DOMContentLoaded
- load

For all this events we currently just register event listeners.

Comment 4

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Attachment: bug-843910.patch

Comment 5

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Blake, it turned out that oranges in try were caused by my inexperience with HG.
My last try push is pretty green: https://tbpl.mozilla.org/?tree=Try&rev=58e485f3d9e7
only few known intermediate oranges showed up.

Comment 6

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Attachment: Implement tab.browser

Comment 7

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Comment on attachment 8341270 [details] [diff] [review]
Implement tab.browser

Oops wrong bug.

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 8339180 [details] [diff] [review]
bug-843910.patch

Review of attachment 8339180 [details] [diff] [review]:
-----------------------------------------------------------------

I'm fine with this for now. We should probably file a followup for figuring out how to do this correctly for and e10s world, though.

::: content/base/src/nsDocument.cpp
@@ +4653,5 @@
>    }
>  
> +  // Dispatch observer notification to notify observers document is interactive.
> +  nsCOMPtr<nsIObserverService> os = mozilla::services::GetObserverService();
> +  nsCOMPtr<nsIPrincipal> principal = GetPrincipal();

Here and below: no need for a nsCOMPtr, you can just do:

nsIPrincipal *principal = GetPrincipal();

Comment 9

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Landed & submitted Bug 946971 to figure out how to e10s-ify this.

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c0a30d9af35c