Closed Bug 701527 Opened 8 years ago Closed 8 years ago

Add support for IndexedDB

Categories

(Firefox for Android :: General, defect, P2)

x86
Linux
defect

Tracking

()

RESOLVED FIXED
Tracking Status
firefox11 --- fixed
fennec 11+ ---

People

(Reporter: mfinkle, Assigned: mbrubeck)

References

Details

Attachments

(3 files, 2 obsolete files)

We can mostly port the desktop Firefox helper object for this:
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#6475

Except we use NativeWindow.doorhanger

Since this is kinda async, like e10s Fennec, you might want to look at the XUL Fennec impl too:
http://mxr.mozilla.org/mozilla-central/source/mobile/chrome/content/bindings/browser.js#723

It might help make things a little cleaner. Also, use the BrowserApp.getXxx helpers instead of those built into the desktop OfflineApps
Assignee: nobody → mbrubeck
Just copies and pastes code from desktop's browser.js, with a find-and-replace to change "var" to "let".  Just separating this out to make reviews easier, so you can see which code is old and which is now.  I'll fold this into the other patch(es) before landing, since it doesn't work on its own.
Attached patch part 2: (WIP) (obsolete) — Splinter Review
Converts the code to use native doorhangers and BrowserApp methods.  This is more or less complete but it isn't working yet.  I'm debugging it now.
Priority: -- → P2
Attachment #573876 - Flags: review?(mark.finkle)
It turns out my tests were failing because my test case was using obsolete APIs.  This patch actually works fine.

I'll probably post a follow-up to make this use the doorhanger timeout/persistence features that just landed in bug 700913.
Attachment #573879 - Attachment is obsolete: true
Attachment #574461 - Flags: review?(mark.finkle)
Attachment #573876 - Flags: review?(mark.finkle) → review+
Comment on attachment 574461 [details] [diff] [review]
part 2: use NativeWindow.doorhanger and BrowserApp APIs

We might need a new doorhanger.hide(...) API
Attachment #574461 - Flags: review?(mark.finkle) → review+
This adds a doorhanger.hide method, so the quota-cancel method can do the same thing as XUL Fennec or desktop Firefox.  It adds a Tab.forceRemoveDoorHanger method that ignores persistence and timeout values, and also uses that to avoid creating duplicate doorhangers in addDoorHanger.
Attachment #574645 - Flags: review?(margaret.leibovic)
Comment on attachment 574645 [details] [diff] [review]
part 3: Hide the doorhanger on cancel

I'm planning on landing my patch for bug 702386 today, and it fixes persistence/timeout checks so that you won't need to make a forceRemoveDoorHanger method.

I'm sorry it didn't land before you wrote this, but hopefully it will make this simpler! (That patch also fixes the issue I see you found in addDoorHanger.)
Attachment #574645 - Flags: review?(margaret.leibovic) → review-
Now with LESS CODES.
Attachment #574645 - Attachment is obsolete: true
Attachment #574676 - Flags: review?(margaret.leibovic)
Comment on attachment 574676 [details] [diff] [review]
part 3 (v2): Hide the doorhanger on cancel

The indexexDB prompts are currently unique in that they really timeout, so we do need to allow them to be removed from the Java side. This timeout is not the same as the DoorHanger persistence/timeout.
Comment on attachment 574676 [details] [diff] [review]
part 3 (v2): Hide the doorhanger on cancel

>diff --git a/embedding/android/GeckoApp.java b/embedding/android/GeckoApp.java
>--- a/embedding/android/GeckoApp.java
>+++ b/embedding/android/GeckoApp.java

>+        mMainHandler.post(new Runnable() {
>+            public void run() {
>+                Tab tab = Tabs.getInstance().getTab(tabId);
>+                if (tab == null)
>+                    return;
>+                tab.removeDoorHanger(value);

This just removes the doorhanger from the set of doorhangers stored on the tab, so you'll also need a mDoorHangerPopup.updatePopupForTab(tab) here if you want to make sure the notification disappears from the UI (or mDoorHangerPopup.updatePopup() if bug 702778 lands).
Attachment #574676 - Flags: review?(margaret.leibovic) → review+
Blocks: 702796
(In reply to Margaret Leibovic [:margaret] from comment #9)
> This just removes the doorhanger from the set of doorhangers stored on the
> tab, so you'll also need a mDoorHangerPopup.updatePopupForTab(tab) here if
> you want to make sure the notification disappears from the UI (or
> mDoorHangerPopup.updatePopup() if bug 702778 lands).

Thanks.  I added updatePopupForTab, and I see you have already landed bug 702778 and changed it to updatePopup:

https://hg.mozilla.org/projects/birch/rev/5675736cb251 (parts 1 and 2)
https://hg.mozilla.org/projects/birch/rev/144775136472 (part 3)

I also filed follow-up bug 702796 to add the 5-minute timeout behavior from desktop Firefox.

No tests; do we have a good way to run unit tests for this code yet?
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
(In reply to Matt Brubeck (:mbrubeck) from comment #10)
> No tests; do we have a good way to run unit tests for this code yet?

I was asking around about that last week. I think once bug 701076 lands we will have a framework in place to let us test native UI components.
mobile/components/ContentPermissionPrompt.js also claims to handle IndexedDB requests. I gather from this bug that it actually doesn't. Maybe we should clean it up to prevent future confusion?

http://hg.mozilla.org/projects/birch/file/5352c48874e2/mobile/components/ContentPermissionPrompt.js#l76
Depends on: 705012
tracking-fennec: --- → 11+
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.