701527 - Add support for IndexedDB

Status: RESOLVED FIXED

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

Description

[Mark Finkle (:mfinkle) (use needinfo?)]

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

Comment 1

[Matt Brubeck (:mbrubeck)]

Attachment: part 1: transplant code from /browser

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.

Comment 2

[Matt Brubeck (:mbrubeck)]

Attachment: part 2: (WIP)

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.

Comment 3

[Matt Brubeck (:mbrubeck)]

Attachment: part 2: use NativeWindow.doorhanger and BrowserApp APIs

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.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 574461 [details] [diff] [review]
part 2: use NativeWindow.doorhanger and BrowserApp APIs

We might need a new doorhanger.hide(...) API

Comment 5

[Matt Brubeck (:mbrubeck)]

Attachment: part 3: Hide the doorhanger on cancel

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.

Comment 6

[: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.)

Comment 7

[Matt Brubeck (:mbrubeck)]

Attachment: part 3 (v2): Hide the doorhanger on cancel

Now with LESS CODES.

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

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 9

[:Margaret Leibovic]

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).

Comment 10

[Matt Brubeck (:mbrubeck)]

(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?

Comment 11

[:Margaret Leibovic]

(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.

Comment 12

[Gian-Carlo Pascutto [:gcp]]

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