153246 - need URL intercept hook

Status: RESOLVED INCOMPLETE

(Core Graveyard :: Embedding: APIs, enhancement)

Description

[Warren Harris]

All browsers to date have had a hook to allow external applications to intercept
certain URL loads. I.e. When a url is clicked on or entered in the location bar,
the hook is called to see if another module wants to handle the url before
passing it on to the browser window. Mozilla doesn't provide a hook for this,
and it should. Without a frozen API for doing this, any code that might
implement this feature threatens to break from release to release. 

Here's a message I recently sent on the subject, with a possible solution:

From: Warren Harris
To: [e-mail]
Cc: [e-mail] ; [e-mail] ; [e-mail] ;
[e-mail] ; [e-mail]
Sent: Thursday, June 20, 2002 5:23 PM
Subject: url intercept problem

Hi Darin,
 
Thanks for talking with me today about the url interception problem in the
browser. I've got some fairly simple code that I think will solve the problem,
and provide something that other developers can rely on for future releases.
Needless to say, I'd like to squeeze this (or something like it) into the
Netscape 7 release so that I don't have to resort to fragile hacks.
 
Background:

    * The problem - register a handler that has a shot at intercepting the url
load before passing it on to the browser. (In IE, this is the Browser Helper
Object mechanism, in Netscape 4.x, there was also COM support for this.) I think
Rick Potts filed bug 147741 about this.
    * Netscape 6.0/6.1 - Rick Potts helped me replace the URILoader with a
wrapper with methods (OpenURI and OpenURIVia) that would fail when we wanted to
intercept the load. This wrapper was a component whose RegisterSelf method would
grab the real URILoader and replace it. This worked, but broke in 6.2.
* Netscape 7 / Mozilla 1.0 - It was suggested that we try to do a similar
wrapper approach, but with nsIProtocolHandler, wrapping the http protocol
handler. The idea was to fail from the NewChannel method if we wanted to
intercept the load. This doesn't work though, because NewChannel isn't called
for http. Instead, nsIProxiedProtocolHandler::NewProxiedChannel is called, and
that's an unsupported interface. (See bug 152663)

My proposed solution is to allow users to register hooks that get called by the
nsIOService before calling NewChannel (or NewProxiedChannel). This can be done
without exposing nsIIOService by using nsIObserverService to register the hooks.
Here's some code that seems to work:
 
Index: xpcom/ds/nsObserverService.cpp
===================================================================
RCS file: /cvsroot/mozilla/xpcom/ds/nsObserverService.cpp,v
retrieving revision 3.25.12.1
diff -u -r3.25.12.1 nsObserverService.cpp
--- xpcom/ds/nsObserverService.cpp 10 Apr 2002 03:36:43 -0000 3.25.12.1
+++ xpcom/ds/nsObserverService.cpp 20 Jun 2002 23:45:15 -0000
@@ -207,8 +207,10 @@
     {
         observers->GetNext(getter_AddRefs(observerRef));
         nsCOMPtr<nsIObserver> observer = do_QueryInterface(observerRef);
-        if ( observer )
-            observer->Observe( aSubject, aTopic, someData );
+        if ( observer ) {
+            rv = observer->Observe( aSubject, aTopic, someData );
+            if (NS_FAILED(rv)) return rv;
+        }
 #ifdef NS_WEAK_OBSERVERS
         else
         {  // check for weak reference.
Index: netwerk/base/src/nsIOService.cpp
===================================================================
RCS file: /cvsroot/mozilla/netwerk/base/src/nsIOService.cpp,v
retrieving revision 1.142.2.1
diff -u -r1.142.2.1 nsIOService.cpp
--- netwerk/base/src/nsIOService.cpp 10 Apr 2002 03:13:39 -0000 1.142.2.1
+++ netwerk/base/src/nsIOService.cpp 20 Jun 2002 23:45:15 -0000
@@ -143,6 +143,8 @@
 static const char kProfileChangeNetTeardownTopic[] = "profile-change-net-teardown";
 static const char kProfileChangeNetRestoreTopic[] = "profile-change-net-restore";
 
+static const char kURLInterceptTopic[] = "url-intercept";
+
 ////////////////////////////////////////////////////////////////////////////////
 
 nsIOService::nsIOService()
@@ -748,6 +750,14 @@
     nsresult rv;
     NS_ENSURE_ARG_POINTER(aURI);
     NS_TIMELINE_MARK_URI("nsIOService::NewChannelFromURI(%s)", aURI);
+
+    nsCOMPtr<nsIObserverService> observerService =
+        do_GetService("@mozilla.org/observer-service;1");
+    NS_ASSERTION(observerService, "Failed to get observer service");
+    if (observerService) {
+        rv = observerService->NotifyObservers(aURI, kURLInterceptTopic, nsnull);
+        if (NS_FAILED(rv)) return rv;   // cancels the load
+    }
 
     nsCAutoString scheme;
     rv = aURI->GetScheme(scheme);
Issues with this approach that you might want to think about:

    * One potential problem is that NotifyObservers doesn't stop its iteration
if one of the observers fails. This is standard practice, but given that it
wasn't done before, fixing nsObserverService to do this constitutes a semantic
change, and someone might be relying on the old behavior (i.e. that other
observers will still be called even if one of them fails). In my limited testing
of the browser, this didn't seem to be a problem, but you know how these things
go. Someone should either look over all the observer implementations, or we
should figure out another way to do this (e.g. a custom observer service for
this purpose).
    * We might want to move this hook up to a point even before the nsIURI
object is created, and just pass the string to the observer for inspection. I
don't feel strongly about this, but it might be nice to not require the user to
deal with nsIURI and all the XPCOM mechanism that goes along with it. Another
reason might be because the construction of the nsIURI object is
protocol-dependent, and the interception process probably should be
protocol-independent.
    * This code looks up the nsIObserverService for each url load, and that
might be a slight performance hit. We could introduce an instance variable to
speed this up.
* Finally, IE lets you register these hooks in the registry, and so does
Netscape 4.x (under the key HKEY_CURRENT_USER, "Software\\Netscape\\Netscape
Navigator\\Automation Protocols"). I don't know if we want to provide a similar
mechanism for Netscape 7 since this is what programmers are used to. As it
stands, my hooks, above, require an XPCOM module to be implemented with a
RegisterSelf method that plugs in the observer under the "url-intercept" topic.

Well, that's my thinking on this. What do you think about the issues involved,
and whether we can get this into the 7.0 release?
 
Thanks,
 
Warren

Comment 1

[Warren Harris]

Rick Potts filed bug 147741 about nsIContentPolicy being a possible mechanism to
solve this.

Dougt filed bug 152663 about the fact that nsIProxiedProtocolHandler isn't
public, and would need to be if you wanted to solve this by overriding the
protocol handler. I would argue that this is the wrong way to go though for 2
reasons:

  - nsIProxyInfo needs to continue to evolve to solve other proxy-related problems
  - overriding a protocol handler is too complicated and error prone for what I
want to accomplish here

Comment 2

[Warren Harris]

Attachment: my proposed patch from the message I sent

Enclosing patch from the message I sent, above. This patch allows observers to
nix the url load process.

Comment 3

[Warren Harris]

Doug Turner wrote:

> I am okay with the basic idea of the patch.  However, I don't want to trust the
> listeners of the observation.  Their return values should remain ignored.  I know
> that you want to get back status information, so instead of returning a result of
> notify, have the observeration pass some kind of "cancel" parameter.  Maybe a 
> boolean that can be set.  We have nsISupport primitive types which will allow you
> to pass a boolean as a nsISupports.

But (a) there's no "out" parameter on the observe method, and (b) you need to
stop the iteration once one of the observers wants to handle the url load. So
maybe I'll work on a special observer service and observer for this, and post
that patch.

BTW, can one of you add the right keyword for Netscape 7.0 Final consideration?
Thanks,

Comment 4

[Bradley Baetz (:bbaetz)]

For this case you want to stop the url load, but if I just want to tell a set of
people something, I don't want a random number of them to be told, and the rest
not to, just because some random class couldn't handle what I was telling them.
theres not even a guarantee on the order the observer service fires stuff, is there?

You could argue that the observerservice should return an error if any of the
things it was notifying failed, but then you'd have to change other code to
ignore that particular error message, and since the observer service is frozen,
we can't do that.

The place you've added the hook is going to pick up lots of noise, too
(chrome/mailnews uris, for example), and if you're going to do this on every
channel load, you should definately cache the observer service.

Comment 5

[Bradley Baetz (:bbaetz)]

Also, see bug 71270. (Mind the flamewars....)

Comment 6

[Warren Harris]

Well guys, it just keeps getting uglier. I've been trying to work on a
work-around in case you can't get the url-intercept hook into nsIOService before
7.0 ships... But I just discovered that between 6.1 and 7.0, the nsIURI
interface was changed to require the use of nsAString parameters for its
accessor routines (they used to be just char**s). That means I have to link with
xpcom.dll just for this one class! I used to be able to do everything through
virtual methods originating from the arguments passed to NSGetModule.

What really bites is that now I have to ship a copy of the xpcom.dll, because my
dll won't load without it because of the link dependency. That means that if I
just want to use my (browser helper object) dll with IE or Netscape 4.x, I still
need xpcom.dll!

I guess I can yank the code out of xpcom.dll for nsAString and statically link
it with my stuff, but I'd really rather not. Needless to say, I'm hoping that we
can get this url-intercept hook in place so that this won't be necessary. 

Comment 7

[Warren Harris]

Bradley: FYI, I didn't see chrome urls go through my hook.

I agree with your comments about the nsIObserverService. Note that my code
didn't change the frozen interface, but I assume that if an interface is frozen,
that implies its semantics are frozen too (i.e. you can fix bugs in the
implementation, but not make a change such as my code suggests). 

Comment 8

[Bradley Baetz (:bbaetz)]

Right. You could, however, return a scucessful code (ie something for which
NS_SUCCEEDED is true), which means 'worked, but some observer reported an error'

since testing with NS_OK directly has always been forbidden, you wouldn't be
changing the semantics in an incompatable way. You'd have to convinve other
people of this, though.

However, since you wouldn't be able to know what error it was, that probably
wouldn't be useful to you.

I'm surprised you don't get chrome calls. I didn't think that we special cased
them until lower down. You may not get mailnews, either, because they do there
own thing. Although you should get mailto: triggered from teh browser.

Comment 9

[Doug Turner (:dougt)]

warren, use the subject parameter to pass a URL Intercept class which contains
the URL and a "out" flag.  

Comment 10

[Darin Fisher]

so i talked a bit with rick yesterday afternoon (after getting off the phone
with warren) about this, and he told me that there's already a similar mechanism
partially in place that provides an external hook to block certain URLs.  it
also has the advantage of conveying some contextual information to the external
hook (e.g., this is an inline image URL, would you like to block it?). 
currently, this hook isn't queried for toplevel URLs, but rick thought it'd be
trivial to make that happen.  LXR for nsIContentPolicy.

Comment 11

[Marek Z. Jeziorek]

batch: adding topembed per Gecko2 document
http://rocknroll.mcom.com/users/marek/publish/Gecko/Gecko2Tasks.html

Comment 12

[Darin Fisher]

well, 1.0.1 has come and gone.  we need to decide what we're going to do with
this bug now.  as for the ACString bit, we're working on providing a lightweight
implementation for embedders / component writers.  the xpcom glue library will
hopefully provide this shortly.

going to label this an enhancement bug for now.

iirc, there was some discussion about the fact that necko might not be the right
place for an URL intercept hook since there is little context about the type of
load (e.g., image load vs. link click).

Comment 13

[Michael Buckland]

Bulk adding topembed keyword.  Gecko/embedding needed.

Comment 14

[saari (gone)]

topembed+

Comment 15

[Darin Fisher]

back to "Embedding APIs" component to decide what to do with this.

Comment 16

[Adam Lock]

Is this embedding APIs or a netwerk thing?

Comment 17

[Darin Fisher]

adam: it sounds to me like we need to provide some context to the URL intercept
hook (e.g., why this URL is being loaded... is it an image, css, js, etc.). 
networking doesn't have enough context about why an URL is being loaded, so i
think we need to look at this from an embedding APIs level.

Comment 18

[Mike Shaver (:shaver emeritus)]

The API you want for network loads with context is nsIContentPolicy, I think. 
(ShouldLoad and ShouldProcess)

If you want a net-nanny hook (which cares only about URL string matching, and
not why the load is happening), I think dougt had something suitable.

Comment 19

[Adam Lock]

Doug, do have something suitable as mentioned in the last comment?

Comment 20

[Juan José Mata]

5/5 EDT triage: minusing topembed+ status.  Dropping this from the radar to
better focus on existing working set.  Changing target milestone as well.

Danm recently did some work for an internal embedding customer that might be of
interest here.  I'll try to dig up that number and add it to this report.

Comment 21

[Benjamin Smedberg]

Marking a bunch of bugs in the "Embedding: APIs" component INCOMPLETE in preparation to archive that component. If I have done this incorrectly, please reopen the bugs and move them to a more correct component as we don't have "embedding" APIs any more.