Add support for IndexedDB

RESOLVED FIXED

Status

()

Firefox for Android
General
P2
normal
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: mfinkle, Assigned: mbrubeck)

Tracking

unspecified
x86
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox11 fixed, fennec11+)

Details

Attachments

(3 attachments, 2 obsolete attachments)

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
(Reporter)

Updated

6 years ago
Assignee: nobody → mbrubeck
(Assignee)

Comment 1

6 years ago
Created attachment 573876 [details] [diff] [review]
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.
(Assignee)

Comment 2

6 years ago
Created attachment 573879 [details] [diff] [review]
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.

Updated

6 years ago
Priority: -- → P2
(Assignee)

Updated

6 years ago
Attachment #573876 - Flags: review?(mark.finkle)
(Assignee)

Comment 3

6 years ago
Created attachment 574461 [details] [diff] [review]
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.
Attachment #573879 - Attachment is obsolete: true
Attachment #574461 - Flags: review?(mark.finkle)
(Reporter)

Updated

6 years ago
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+
(Assignee)

Comment 5

6 years ago
Created attachment 574645 [details] [diff] [review]
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.
Attachment #574645 - Flags: review?(margaret.leibovic)

Comment 6

6 years ago
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-
(Assignee)

Comment 7

6 years ago
Created attachment 574676 [details] [diff] [review]
part 3 (v2): Hide the doorhanger on cancel

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 9

6 years ago
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+
(Assignee)

Updated

6 years ago
Blocks: 702796
(Assignee)

Comment 10

6 years ago
(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
Last Resolved: 6 years ago
Flags: in-testsuite?
Resolution: --- → FIXED

Comment 11

6 years ago
(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
(Assignee)

Updated

6 years ago
Depends on: 705012
tracking-fennec: --- → 11+
status-firefox11: --- → fixed
(Assignee)

Updated

5 years ago
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.