Closed
Bug 756645
Opened 13 years ago
Closed 12 years ago
Implement "indexedDB jars" for apps
Categories
(Core :: Storage: IndexedDB, defect, P1)
Core
Storage: IndexedDB
Tracking
()
RESOLVED
FIXED
mozilla18
People
(Reporter: sicking, Assigned: mounir)
References
Details
(Whiteboard: [qa-])
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
bent.mozilla
:
review+
|
Details | Diff | Splinter Review |
I think this should be relatively easy to do now that bug 754141 is fixed. We just need to add the app-key to the origin.
Reporter | ||
Updated•13 years ago
|
No longer blocks: app-data-jars
Reporter | ||
Updated•13 years ago
|
Blocks: app-data-jars
Updated•12 years ago
|
blocking-basecamp: --- → +
blocking-kilimanjaro: --- → +
Comment 1•12 years ago
|
||
Mounir and Ben can fight over who gets this. I'll assign it to Mounir for now :)
Assignee: nobody → mounir
Reporter | ||
Updated•12 years ago
|
Assignee: mounir → jonas
Assignee | ||
Updated•12 years ago
|
Assignee: jonas → mounir
OS: Mac OS X → All
Hardware: x86 → All
Assignee | ||
Comment 2•12 years ago
|
||
That's quite simple actually: instead of getting the origin from nsContentUtils::GetASCIIOrigin, we just get it from nsIPrincipal::GetExtendedOrigin. This is also returning a puny-encoded origin. I haven't renamed all the variables and stuff though. I also removed a call to GetASCIIOriginFromWindow() that was obviously not used.
Attachment #644541 -
Flags: review?(bent.mozilla)
Assignee | ||
Updated•12 years ago
|
Status: NEW → ASSIGNED
Updated•12 years ago
|
Attachment #644541 -
Flags: review?(bent.mozilla) → review+
Assignee | ||
Comment 3•12 years ago
|
||
Jonas, could you review the tests. Ben reviewed the code.
Attachment #644541 -
Attachment is obsolete: true
Attachment #644805 -
Flags: review?(jonas)
Assignee | ||
Comment 4•12 years ago
|
||
Those ones should pass try.
Attachment #644805 -
Attachment is obsolete: true
Attachment #644805 -
Flags: review?(jonas)
Attachment #645465 -
Flags: review?(bent.mozilla)
Comment on attachment 645465 [details] [diff] [review] Patch + Tests Review of attachment 645465 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/indexedDB/test/file_app_isolation.js @@ +96,5 @@ > + if (data.app || data.browser) { > + iframe.addEventListener('mozbrowsershowmodalprompt', function(e) { > + is(e.detail.message, 'success', 'test number ' + i); > + > +// document.getElementById('content').removeChild(iframe); Comment here that you're waiting for the other bug to be fixed.
Attachment #645465 -
Flags: review?(bent.mozilla) → review+
Assignee | ||
Comment 6•12 years ago
|
||
So tests are green on try except that the OOP one is leaking. It's not because of the code change but very likely the browser element and less likely IndexedDB code. IOW, something is leaking and that test is making that obvious. Given that we are going to ship B2G with OOP mozapps/browsers using IndexedDB, we will need to fix that leak. However, should we block that patch until the leak is fixed or should we push the patch with the OOP test disabled on debug so we can still test the behaviour but don't get failures for the leak? Try results: https://tbpl.mozilla.org/?tree=Try&rev=190a90c10a4b
Comment 7•12 years ago
|
||
> However, should we block that patch until the leak is fixed or should we push the patch
> with the OOP test disabled on debug so we can still test the behaviour but don't get
> failures for the leak?
Mounir cc'ed me, so presumably he would like my opinion? I think we should leave this up to bent and co, but my feeling is that we should at last try to debug the leak, because otherwise, there's a distinct possibility we won't fix it.
Updated•12 years ago
|
Priority: -- → P1
Assignee | ||
Comment 9•12 years ago
|
||
The leak isn't happening any more \o/ https://hg.mozilla.org/mozilla-central/rev/4bb90f8c6909
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Comment 10•12 years ago
|
||
This was backed out due to Android M3 timeouts in test_advance.html. https://hg.mozilla.org/mozilla-central/rev/29ca472bf2d2 One example: https://tbpl.mozilla.org/php/getParsedLog.php?id=14655303&tree=Firefox
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Updated•12 years ago
|
Flags: in-testsuite+
Target Milestone: mozilla17 → ---
Assignee | ||
Updated•12 years ago
|
Flags: in-testsuite+
Target Milestone: --- → mozilla18
Comment 11•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d192fe240461 https://hg.mozilla.org/mozilla-central/rev/4496025f9d35
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Comment 12•12 years ago
|
||
This looks testable from an end-user perspective, but I need clarification on what exactly is being tested here. Looks like the testing would be similar to what was tested on desktop for the origin rules for indexedDB access for apps. Is that correct?
QA Contact: jsmith
Whiteboard: [qa?]
Assignee | ||
Comment 13•12 years ago
|
||
It is indeed testable from an end-user perspective but we have automated tests for that so I think it's not worth doing manual testings.
Comment 14•12 years ago
|
||
(In reply to Mounir Lamouri (:mounir) from comment #13) > It is indeed testable from an end-user perspective but we have automated > tests for that so I think it's not worth doing manual testings. Okay sounds good.
Whiteboard: [qa?] → [qa-]
You need to log in
before you can comment on or make changes to this bug.
Description
•