Closed Bug 994734 Opened 6 years ago Closed 6 years ago

Centralize change notification for dataset cursors

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 31
Tracking Status
firefox30 --- fixed
firefox31 --- fixed

People

(Reporter: lucasr, Assigned: lucasr)

References

Details

Attachments

(6 files)

Right now this is done multiple times on each dynamic panel instance which is not ideal.
Comment on attachment 8410302 [details] [diff] [review]
Rename PanelManager to PanelInfoManager (r=margaret)

Be more explicit about the kind of data PanelManager deals with. I'm still not happy with the 'manager' in the name as it doesn't really manage anything :-)
Attachment #8410302 - Flags: review?(margaret.leibovic)
Comment on attachment 8410303 [details] [diff] [review]
Rename HomeConfigInvalidator to HomePanelsManager (r=margaret)

I'd like to avoid creating yet another app-wide entry point for handling home panels stuff. So I'm re-purposing HomeConfigInvalidator with a more general name here.
Attachment #8410303 - Flags: review?(margaret.leibovic)
Comment on attachment 8410304 [details] [diff] [review]
Pass dataset id as query argument in HomeProvider (r=margaret)

We need access to the dataset ID in order to consistently set a notification URI in the cursors returned by HomeProvider.
Attachment #8410304 - Flags: review?(margaret.leibovic)
Comment on attachment 8410305 [details] [diff] [review]
Set cursor notification URI in HomeProvider (r=margaret)

Enforce consistent notification URI on all cursors returned by HomeProvider instead of relying on callers to set it.
Attachment #8410305 - Flags: review?(margaret.leibovic)
Comment on attachment 8410306 [details] [diff] [review]
Handle dataset refreshes in HomePanelsManager (r=margaret)

Move dataset refresh handling from DynamicPanel to HomePanelsManager.
Attachment #8410306 - Flags: review?(margaret.leibovic)
Attachment #8410302 - Flags: review?(margaret.leibovic) → review+
Attachment #8410303 - Flags: review?(margaret.leibovic) → review+
Attachment #8410304 - Flags: review?(margaret.leibovic) → review+
Comment on attachment 8410305 [details] [diff] [review]
Set cursor notification URI in HomeProvider (r=margaret)

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

Nice.
Attachment #8410305 - Flags: review?(margaret.leibovic) → review+
Comment on attachment 8410306 [details] [diff] [review]
Handle dataset refreshes in HomePanelsManager (r=margaret)

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

Nice, this looks like a much better place for this logic.
Attachment #8410306 - Flags: review?(margaret.leibovic) → review+
Attached patch patch for auroraSplinter Review
Note to release managers: I'm requesting uplift for a series of Firefox Hub bugs. The main fixes we need are in bugs at the end of the series, but trying to rebase those patches proved difficult and risky, so I think we should just uplift the dependecies.

Note to sheriffs: I have a local patch series that I can land on aurora when these bugs all get approval.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): dependency for initial Firefox Hub release (promoted feed add-ons in fx30)
User impact if declined: panels won't update when data changes
Testing completed (on m-c, etc.): baked on m-c
Risk to taking this patch (and alternatives if risky): only affects dynamic panels, need an add-on to trigger this
String or IDL/UUID changes made by this patch: none
Attachment #8412223 - Flags: approval-mozilla-aurora?
Attachment #8412223 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.