Closed Bug 401745 Opened 15 years ago Closed 15 years ago

Metrics extension compatibility with Google Toolbar

Categories

(Toolkit Graveyard :: Data Collection/Metrics, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: alex, Assigned: pete)

References

()

Details

(Whiteboard: [npotb?])

Attachments

(3 files, 1 obsolete file)

Google Toolbar user similar methods and preferences as the metrics extension that lives in trunk. It will have to be possible to run both side by side, so ensuring compatibility is a must.
Yes google toolbar is using the same prefs which is bad it really should be: "extensions.google.metrics.*"

I would recommend we change mozilla metrics prefs to something like:  "extensions.mozilla.metrics.*"

We might want to consider changing the uuid of the interfaces.

The contract id "@mozilla.org/metrics/service;1" is fine ... 
Assignee: nobody → pete
Status: NEW → ASSIGNED
Attached patch proposed patch (obsolete) — Splinter Review
proposed patch to avoid potential conflicts w/ google toolbar.

Jan, can you please review when you get a chance.
Ok, I just checked and googletoolbar is using the same contractID

  @mozilla.org/metrics/service;1

So maybe we should change the contractID to

  @mozilla.org/metrics/service;2

or maybe:

  @mozilla.org/extensions/metrics/service;1

Changing out the uuids and contactIDs is probably not necessary but I think prudent as to aviod potential conflicts w/ google toolbar extension ...
Attachment #290588 - Attachment is obsolete: true
Attachment #290704 - Flags: review+
checked in yesterday. Marking FIXED.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Google Toolbar for Firefox 3.0 has issue with changed interface ids for the same interface names. For some time we plan to distribute extension compatible with Firefox 2 and 3 (same .xpi bundle) so that our users won't need to update on Firefox upgrade. For this we need .xpt files for both metrics extensions to be loaded. We were unable to find a way to hide one .xpt file or another. We get messages complaining about same interface name registered with different iids and one of versions is unable to find interfaces. 

Reverting iids back saves the problem. We believe keeping iids is safe as there are no interface changes in metrics.
 
What created conflict in the previous version was toolbar including metrics code as is. So far we don't feel a need in the separate metrics implementation. It would be really useful for us to make metrics work for both products. For example, we can avoid modification of upload uri and let metrics extension collect data when it's set up. We can also check metrics presence and don't load our copy unless it's necessary.
We changed the contract ID's, interface ID's, and prefs so that there wont be any unanticipated collisions if users have the two implementations installed simultaneously. 

I was under the impression that the google implementation was a separate implementation that was *based* on metrics code.

If you are using a vanilla build of metrics, then can't you just spin a new build of the component and include it in your toolbar releases for FF3?

Reverting back the iids is fine by me if that solves the problem. But the current builds of metrics for FF3 is already released.


In order to have single .xpi that provides the metrics component compatible with both Firefox 2 and 3 we have to avoid conflict of .xpt's of different versions of metrics. Two ways are possible: make interfaces (names and iid's) identical or make them completely different. The second way looks more robust. The proposed patch renames the public interfaces nsIMetricsService, nsIMetricsCollector and nsIMetricsEventItem.
Comment on attachment 332544 [details] [diff] [review]
Rename public interfaces.

Pete, could you review this patch?
Attachment #332544 - Flags: review?(pete)
The iid's of the interfaces don't matter. The conflict arises only when there is another extension that wants to use the metrics interfaces w/ a separate implementation for those interfaces and that implementation is carrying the same named interfaces w/ different iid's.

I think the Google folks want to be able to do everything in a single xpi. 

I'm not sure how you are gong to have binary compatibility between FF2 and FF3 components ...

Since - "We can also check metrics presence and don't load our copy unless it's necessary" - then if you are not loading your copy, then where is the conflict?

Actually, how do you *not* load your components unless you have an XPI without them?

If reverting back to the old interface iid's will *really* solve the problem then perhaps that is what we should do ...

If Google folks are using the metrics implementation verbatim, maybe it might be easier to just roll their UI code in one XPI and then as a dependency, install the vanilla metrics xpi all as part of the same one click install. This way they are using the correct metrics version for the target distribution of Firefox ...



Sorry, Pete, I didn't realize that you haven't seen the thread of Alex Polvi and Igor Bazarny. In brief they agreed in the following.

1. To change interfaces names.
2. To make possible compile-time configuration of the metrics -- add "macros for contract id prefixes, prefs prefixes and move class ids to the separate header file, so we can replace everything in our build. File names (config, data), and about module name are also turned into macros, so we get independent clone."

Why 1. Igor wrote:
"our issue is changed interface ids for the same interface names. For some time we plan to distribute extension compatible with Firefox 2 and 3 (same .xpi bundle) so that our users won't need to update on Firefox upgrade. For this we need .xpt files for both metrics extensions to be loaded. We were unable to find a way to hide one file or another (for service implementations we have proxy loader registering version-specific library). We get messages complaining about same interface name registered with different iids and one of versions is unable to find interfaces."

Alex suggested to change interfaces names. I agree with him.
The reverting back the old interface id's would solve the problem if we could freeze the interfaces. On the other hand changing of names of interfaces shouldn't be too hard and will help to avoid the conflict if interfaces are modified.

Why 2.
At present Spectator and Toolbar will share preferences and conractID's if they are installed together. There is no good in any case and Firefox behavior is unpredictable.
Attachment #332544 - Flags: review+
Attachment #332544 - Flags: approval1.9.0.2?
Comment on attachment 332544 [details] [diff] [review]
Rename public interfaces.

I'm pretty sure the metrics extensions is NPOTB and doesn't need approval to land. Can someone confirm?
Attachment #332544 - Flags: review?(pete)
Whiteboard: [npotb?]
> I'm pretty sure the metrics extensions is NPOTB and doesn't need approval to
> land. Can someone confirm?


That's right, it just needs a review ...
Comment on attachment 332544 [details] [diff] [review]
Rename public interfaces.

Since approval is not needed the request for it is removed.
Attachment #332544 - Flags: approval1.9.0.2?
This patch doesn't interfere with the previous one.

Collect all cntractID's, classID's and preferences that could cause conflict between Spectator and Toolbar into one place -- public/nsMetricsModule.h
Attachment #333417 - Flags: review?(pete)
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.