Closed Bug 914294 Opened 6 years ago Closed 6 years ago

Macro END_RESULT_BATCH_AND_REFRESH_CONTENTS() introduces an exception compatibility issue

Categories

(Toolkit :: Places, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla27
Tracking Status
firefox25 --- unaffected
firefox26 + fixed
firefox27 + fixed

People

(Reporter: mak, Assigned: mak)

References

Details

(Keywords: regression)

Attachments

(1 file)

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.
...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.
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?
Version: unspecified → Trunk
The problem started with FF26, works fine with FF25.
(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.
Attached patch 914294.diffSplinter Review
For now this should do.
Attachment #813376 - Flags: review?(mano)
requesting tracking cause this is breaking quite used add-ons
https://hg.mozilla.org/integration/fx-team/rev/55e9f80315a6
Target Milestone: --- → mozilla27
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
Attachment #813376 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/55e9f80315a6
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Attachment #813376 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.