914294 - Macro END_RESULT_BATCH_AND_REFRESH_CONTENTS() introduces an exception compatibility issue

Status: RESOLVED FIXED

(Toolkit :: Places, defect)

Description

[Marco Bonardo [:mak]]

END_RESULT_BATCH_AND_REFRESH_CONTENTS() macro checks for result validity and may throw, this could make sense ideally, since we are requesting information for a zombie (not live-updating) node.

Though this causes incompatibility with some add-ons that are working with a result and caching contents (like XMarks).

I don't have a perfect solution to make everyone happy, but I think at least we can modify END_RESULT_BATCH_AND_REFRESH_CONTENTS() so that it doesn't throw, as before. It may just sometimes return cached content instead of last second up-to-date content.

Comment 1

[Marco Bonardo [:mak]]

...and then you may be walking the "frozen" container contents, and try to query stuff for nonexisting ids and that will throw (but this could happen already so consumers are catching those failures).
It's a somehow complex problem, cause we know the consumer wants up-to-date content, but since he has an handle to frozen content we can't return it.
This is definitely something to evaluate for a results rewrite. Or we may calculate the cost to never batch in the result (e.g. never re-query, always live update) and only batch in the view.

Comment 2

[alanjstr]

Xmarks does not work on Firefox Nightly but does work on Firefox 23.0.1.  Have we been able to narrow the regression window at all?

Comment 3

[Anatoly]

The problem started with FF26, works fine with FF25.

Comment 4

[Marco Bonardo [:mak]]

(In reply to alanjstr from comment #2)
> Xmarks does not work on Firefox Nightly but does work on Firefox 23.0.1. 
> Have we been able to narrow the regression window at all?

there's already a blocked bug, no need to find a further reg window.

Comment 5

[Marco Bonardo [:mak]]

Attachment: 914294.diff

For now this should do.

Comment 6

[Marco Bonardo [:mak]]

requesting tracking cause this is breaking quite used add-ons

Comment 7

[Marco Bonardo [:mak]]

https://hg.mozilla.org/integration/fx-team/rev/55e9f80315a6

Comment 8

[Marco Bonardo [:mak]]

Comment on attachment 813376 [details] [diff] [review]
914294.diff

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 894331
User impact if declined: unexpected add-ons breakage
Testing completed (on m-c, etc.): local
Risk to taking this patch (and alternatives if risky): popular add-ons like Xmarks will break
String or IDL/UUID changes made by this patch: none

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/55e9f80315a6

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/2718af9506b3