Closed Bug 918880 Opened 12 years ago Closed 12 years ago

MaybeAllowOfflineAppByDefault must work on child processes

Categories

(Core :: Networking: Cache, defect)

Product:
Core
Component:
Networking: Cache
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla29
Project Flags:
blocking-b2g koi+
Tracking Flags:
Tracking Status
firefox27 --- wontfix
firefox28 --- fixed
firefox29 --- fixed
b2g-v1.2 --- fixed
b2g-v1.3 --- fixed

People

(Reporter: gwagner, Assigned: fabrice)

References

Details

Attachments

(3 files, 1 obsolete file)

v1
13.72 KB, patch
jduell.mcbugs
: review+
Details | Diff | Splinter Review
followup
7.53 KB, patch
mayhemer
: review+
Details | Diff | Splinter Review
avoid returning null from TabParent::AllocPOfflineCacheUpdateParent v1
2.67 KB, patch
Details | Diff | Splinter Review
When starting twitter on a b2g device I get: #0 NS_DebugBreak (aSeverity=1, aStr=0xb66446fa "Cannot perform action in content process!", aExpr=0xb64cbd8b "Error", aFile=0xb6644529 "../../../extensions/cookie/nsPermissionManager.cpp", aLine=610) at ../../../xpcom/base/nsDebugImpl.cpp:302 #1 0xb57512d0 in nsPermissionManager::AddFromPrincipal (this=0xb2efe780, aPrincipal=0xb2839250, aType=0xb657ba5f "offline-app", aPermission=1, aExpireType=0, aExpireTime=0) at ../../../extensions/cookie/nsPermissionManager.cpp:610 #2 0xb513f36e in nsContentUtils::MaybeAllowOfflineAppByDefault (aPrincipal=0xb2839250) at ../../../../content/base/src/nsContentUtils.cpp:1434 #3 0xb513aeec in nsContentSink::ProcessOfflineManifest (this=0xb3e32800, aManifestSpec=...) at ../../../../content/base/src/nsContentSink.cpp:1067 #4 0xb54a5b30 in nsHtml5SpeculativeLoad::Perform (this=0xb281ab3c, aExecutor=0xb3e32800) at ../../../parser/html/nsHtml5SpeculativeLoad.cpp:45 #5 0xb54bade8 in nsHtml5TreeOpExecutor::RunFlushLoop (this=0xb3e32800) at ../../../parser/html/nsHtml5TreeOpExecutor.cpp:507 #6 0xb54a6468 in nsHtml5ExecutorFlusher::Run (this=0xb288eac0) at ../../../parser/html/nsHtml5StreamParser.cpp:131 #7 0xb5b41b1a in nsThread::ProcessNextEvent (this=0xb3e02390, mayWait=<optimized out>, result=0xbe98dda7) at ../../../xpcom/threads/nsThread.cpp:622 #8 0xb5b10d0e in NS_ProcessNextEvent (thread=0xb3e02390, mayWait=<optimized out>) at ../../../xpcom/glue/nsThreadUtils.cpp:238 #9 0xb57e48de in mozilla::ipc::MessagePump::Run (this=0xb3e01c40, aDelegate=0xbe98df08) at ../../../ipc/glue/MessagePump.cpp:81 #10 0xb5b67336 in MessageLoop::RunInternal (this=0xbe98df08) at ../../../ipc/chromium/src/base/message_loop.cc:220 #11 0xb5b6734e in RunHandler (this=0xbe98df08) at ../../../ipc/chromium/src/base/message_loop.cc:213 #12 MessageLoop::Run (this=0xbe98df08) at ../../../ipc/chromium/src/base/message_loop.cc:187 #13 0xb577944e in nsBaseAppShell::Run (this=0xb31bd2e0) at ../../../widget/xpwidgets/nsBaseAppShell.cpp:161 #14 0xb4d84df2 in XRE_RunAppShell () at ../../../toolkit/xre/nsEmbedFunctions.cpp:679 #15 0xb57e49ca in mozilla::ipc::MessagePumpForChildProcess::Run (this=0xb3e01c40, aDelegate=0xbe98df08) at ../../../ipc/glue/MessagePump.cpp:201 #16 0xb5b67336 in MessageLoop::RunInternal (this=0xbe98df08) at ../../../ipc/chromium/src/base/message_loop.cc:220 #17 0xb5b6734e in RunHandler (this=0xbe98df08) at ../../../ipc/chromium/src/base/message_loop.cc:213 #18 MessageLoop::Run (this=0xbe98df08) at ../../../ipc/chromium/src/base/message_loop.cc:187 #19 0xb4d84d1e in XRE_InitChildProcess (aArgc=5, aArgv=<optimized out>, aProcess=<optimized out>) at ../../../toolkit/xre/nsEmbedFunctions.cpp:516 #20 0x0000876e in main (argc=6, argv=0xbe98e9f4) at ../../../ipc/app/MozillaRuntimeMain.cpp:85
We will have to send the permission setting request to the parent process. Is there already some API to do it? Would then there be any security impact?
I am not aware of any existing API to make such a request.
blocking-b2g: --- → koi?
(In reply to Honza Bambas (:mayhemer) from comment #1) > We will have to send the permission setting request to the parent process. > Is there already some API to do it? > > Would then there be any security impact? We could do it via PermissionSettings.js but then every app would need the 'permission' permission and we definitely don't want that. I think we need to add a remote function that deals with this special case.
Blocks: 767258
Blocks: 871323
(In reply to Gregor Wagner [:gwagner] from comment #3) > We could do it via PermissionSettings.js but then every app would need the > 'permission' permission and we definitely don't want that. I think we need > to add a remote function that deals with this special case. Thanks. That's the info I need! I'm on it.
Assignee: nobody → honzab.moz
Status: NEW → ASSIGNED
OK, probably not that simple... The check and set in nsContentSink is as: if (!nsContentUtils::OfflineAppAllowed(mDocument->NodePrincipal()) && !nsContentUtils::MaybeAllowOfflineAppByDefault(mDocument->NodePrincipal()) && !nsContentUtils::OfflineAppAllowed(mDocument->NodePrincipal())) { return; } Note there is the additional check call to OfflineAppAllowed. In some cases MaybeAllowOfflineAppByDefault returns true based on AddFromPrincipal has succeeded. But OfflineAppAllowed still returns false. Some tests were failing w/o this second after-set check. I'd like the IPC call to parent to set the permission be async (obviously). But here raises a problem: the second call to OfflineAppAllowed will check again with the permission manager - and fail. I moved the actual call to perm-manager (on parent process) or my new SendSetPermission (on child process) to OfflineCacheUpdateService. Here I have to simulate behavior of the permission manager to make OfflineAppAllowed work (at least for the time until the permission set operation it performed on the parent and propagated back to the child - if so). Permission manager is a black box (and should be) and I don't want to re-write it's functionality in OfflineCacheUpdateService because of this. Option is to make the SetPermission call be sync or even RPC. Or do you have any other ideas?
Attached patch WIP1, NOT EVEN COMPILABLE (obsolete) — — Splinter Review
Summary: Don't set permission in child process in MaybeAllowOfflineAppByDefault → MaybeAllowOfflineAppByDefault must work on child processes
Gregor, Please check on next steps.
Flags: needinfo?(anygregor)
blocking-b2g: koi? → koi+
No security risks. Not functionally visible to the user. Why would we block on this bug?
blocking-b2g: koi+ → koi?
Honza, does this have any impact on functionality?
Flags: needinfo?(anygregor) → needinfo?(honzab.moz)
(In reply to Gregor Wagner [:gwagner] from comment #9) > Honza, does this have any impact on functionality? I don't understand the question. |this| stands for what? and what |functionality| should be impacted?
(In reply to Honza Bambas (:mayhemer) from comment #10) > (In reply to Gregor Wagner [:gwagner] from comment #9) > > Honza, does this have any impact on functionality? > > I don't understand the question. |this| stands for what? and what > |functionality| should be impacted? I mean what is the impact if we don't fix this bug for 1.2? We are trying to set a permission and fail right?
Got it! The overall impact is that web content loaded on content processes will not be cached offline. This should be fixed, please see comment 5. I need someone familiar with the permission manager to find some nice and elegant way to set the permission on content process in a way it will be set on the parent asynchronously while synchronously available on the content process.
Flags: needinfo?(honzab.moz)
Is the permission of the app being impacted? Is the perf impacted on the phone?
Flags: needinfo?(honzab.moz)
Is comment 0 just seen in a log or does it have user impact like a delayed app launch? ISTM that nsContentUtils::MaybeAllowOfflineAppByDefault is new since 1.1 so I if there's user impact this is a regression, right?
(In reply to Preeti Raghunath(:Preeti) from comment #13) > Is the permission of the app being impacted? Is the perf impacted on the > phone? As we are using the principal to set the permission for, then an app (meaning having an appid), if I understand the question, yes. Not sure what you mean by 'on the phone'.
Flags: needinfo?(honzab.moz)
(In reply to Andrew Overholt [:overholt] from comment #14) > Is comment 0 just seen in a log or does it have user impact like a delayed > app launch? The permission is not set, so the content (web, app) is no cached offline. > ISTM that nsContentUtils::MaybeAllowOfflineAppByDefault is new since 1.1 so > I if there's user impact this is a regression, right? Looks like.
Lack of offline cache could impact startup performance and this is a regression so koi+. (In reply to Honza Bambas (:mayhemer) from comment #12) > This should be fixed, please see comment 5. I need someone familiar with > the permission manager to find some nice and elegant way to set the > permission on content process in a way it will be set on the parent > asynchronously while synchronously available on the content process. I think Fabrice may be able to help with permission manager questions.
blocking-b2g: koi? → koi+
Flags: needinfo?(fabrice)
I'm not familiar with the permission manager code, so I'll pass on this one.
Flags: needinfo?(fabrice)
I was talking to Jonas about this. Why do we try to add the permission if we have 'offline-apps.allow_by_default' set. Can we get rid of this and remove the check?
I.e. rather than adding the permission when the pref is set, instead we should not check the permission if the pref is set.
Looks like I have to explain this on and on....: When we assumed that all domains had the offline-app permission we would check ALL top level loads with appcache first. That would lead to a major performance problem. The offline-apps.allow_by_default = true causes the offline-app permission set when we hit the manifest only, so it prevents this huge performance regression.
Attached patch v1 — — Splinter Review
untested, since it's impossible to build a working b2g desktop build these days.. (win,linux).
Attachment #814839 - Attachment is obsolete: true
Attachment #829759 - Flags: review?(jonas)
(In reply to Honza Bambas (:mayhemer) from comment #22) > Created attachment 829759 [details] [diff] [review] > v1 > > untested, since it's impossible to build a working b2g desktop build these > days.. (win,linux). What issue are you facing? Many of us do desktop builds daily, at least on mac and linux.
(In reply to Fabrice Desré [:fabrice] from comment #23) > What issue are you facing? Many of us do desktop builds daily, at least on > mac and linux. Good to here that desktop builds are generally in a good shape! I don't build that often, so maybe I do something wrong, but I'm building according [1] and when I then run |/_obj/dist/bin/b2g -profile /path-to/gaia/profile/| I get nothing than a blank 0,0-sized window on Win and no window at all on Linux. Even no blank screen as usual when there is something wrong with the gaia version or the compiled profile. [1] https://developer.mozilla.org/en-US/docs/Mozilla/Firefox_OS/Platform/Gaia/Hacking
Comment on attachment 829759 [details] [diff] [review] v1 >+ // Wish there were a simple way to get the TabChild instance...... TabChild::GetFrom(aWindow)
(In reply to Honza Bambas (:mayhemer) from comment #21) > Looks like I have to explain this on and on....: Sorry, I did not know this. I don't know how I could have. > The offline-apps.allow_by_default = true causes the offline-app permission > set when we hit the manifest only, so it prevents this huge performance > regression. Ah, so sounds like you are using the permission database as a "This origin uses appcache" database. This is a bit of a hack and probably affects performance in B2G since we give offline permission at time of install to some apps, independent of if the app actually uses appcache or not. Looking at the patch now...
Comment on attachment 829759 [details] [diff] [review] v1 The general approach here seems fine if I understand things correctly. I.e. it appears that you are creating a hash table which indicates "uses offline" which lives in both child and parent processes. And then enables child processes to update that hash. One thing that does look like it's missing is that you don't broadcast to other child processes when you are adding something to that hash table. Or does the hash only live in the parent? Either way, I don't feel that I know this code well enough to review. If there really is no-one else then I can though.
Attachment #829759 - Flags: review?(jonas) → review?
(In reply to Jonas Sicking (:sicking) Please ni? if you want feedback from comment #26) > (In reply to Honza Bambas (:mayhemer) from comment #21) > > Looks like I have to explain this on and on....: > > Sorry, I did not know this. I don't know how I could have. No body knows, this caught my self ones. Problem is that this behavior/code path is very unexpected. > > > The offline-apps.allow_by_default = true causes the offline-app permission > > set when we hit the manifest only, so it prevents this huge performance > > regression. > > Ah, so sounds like you are using the permission database as a "This origin > uses appcache" database. This is a bit of a hack and probably affects > performance in B2G since we give offline permission at time of install to > some apps, independent of if the app actually uses appcache or not. > > Looking at the patch now... We were using this since ever. Before bug 892488 'offline-app' permission was at UNKNOWN for all sites. When a manifest was hit in the html tag, we raised the prompt to allow or disallow storing offline and based on the user's decision we had set the permission to ALLOW or REJECT respectively. Next time on the visit we knew the decision already. With bug 892488 this permission is just *given automatically* WHEN WE HIT a manifest in the html tag. First, users want an option to disable the offline caching, and probably per-site. Perm manager IMO is the right place, but I'm no expert to this to decide. A side-product is a protection against checking appcache sqlite database (currently on the main thread!) for EVERY top level load - we do it only for the allowed. Only few sites use appcache, so we would regress 98% of page load time. (In reply to Jonas Sicking (:sicking) Please ni? if you want feedback from comment #27) > Comment on attachment 829759 [details] [diff] [review] > v1 > > The general approach here seems fine if I understand things correctly. I.e. > it appears that you are creating a hash table which indicates "uses offline" > which lives in both child and parent processes. And then enables child > processes to update that hash. The hash table on the child is just simulating permission manager behavior. In a single process model the code looks like: - we hit manifest= attr at a site at foo.com/ - we have offline-apps.allow_by_default at true - if 'offline-app' permission is not set for foo.com (according perm manager), set it now (using perm manager) for foo.com - check again (with perm manager) that 'offline-app' permission had been set for foo.com correctly - if so, do the caching On the child process I cannot set the permission. So it's broadcasted to parent (only). And remembered at the hash on the child (only) since perm manager will still give me 'UNKNOWN' when asking for 'offline-app' perm for the foo.com domain. > > One thing that does look like it's missing is that you don't broadcast to > other child processes when you are adding something to that hash table. Really needed? I don't think it is. > > Or does the hash only live in the parent? It lives ONLY in child. > > Either way, I don't feel that I know this code well enough to review. If > there really is no-one else then I can though. jduell could take look instead, or maybe jdm. The major question here is your concern about the performance issue at b2g with permission manager. Sounds like you would rather change the way how we store 'offline-app' permission or, say, preference for a site and/or generally. I'm definitely open to suggestions. What we need is to have a mechanism that allows us REJECT the 'offline-app' permission per domain, since the prompts appearance is still configurable (via the pref, there is no or broken UI! - another bug!) and we want users leave the control (at least now). But depends... With temporary and persistent storage APIs we could remove the prompts all together and make this all decision code simpler. Querying the sqlite database for every top level load can definitely be optimized. As you might know I work on the new HTTP cache. Part of the first public deployment state is however not any change to appcache. Actually changing the appcache backend is not planned at all, but the new HTTP cache overhaul opens door to make it much simpler.
Blocks: 892488
Please, see comment 28.
Flags: needinfo?(jonas)
We need to land this asap, emailing to try to get traction again.
Comment on attachment 829759 [details] [diff] [review] v1 Review of attachment 829759 [details] [diff] [review]: ----------------------------------------------------------------- This isn't code in /netwerk but I know it passably well, and the code is simple enough, so I'm taking the liberty of marking this +r (with one nit to fix), unless a DOM peer objects (but AFAIK none of them know the code any better than I do). This is assuming the basic strategy of checking the permission manager is still OK, i.e. that comment 20 and comment 26 are future work if anything. That's my sense here. ::: uriloader/prefetch/nsOfflineCacheUpdateService.cpp @@ +757,5 @@ > +{ > + nsresult rv; > + > + if (GeckoProcessType_Default != XRE_GetProcessType()) { > + // Wish there were a simple way to get the TabChild instance...... As jdm mentions in comment 25, you can replace all this code with just TabChild::GetFrom(aWindow). Please do that.
Attachment #829759 - Flags: review? → review+
Thanks for reviewing Jason. I'm totally fine with fixing comment 26 as a followup only on mozilla-central. I definitely think that the current setup is affecting performance for many b2g apps. We have treated the "offline-app" permission as just a permission and so we set it for several apps, even if they aren't specifically using appcache. The fact that this causes us to do extra IO to look for appcached resources is a problem since it means that we spend time doing that for each app load. At the very least we should figure out a way to avoid this for packaged apps. But it'd be good to find a more generic solution. Though we should probably not spend too much time on it given how little use the appcache has. Anyway, could you file a followup on this?
Flags: needinfo?(jonas)
Actually, I thought of a very simple way of doing that. I filed bug 940254.
Thanks Phil, the patch was completely untested since I couldn't build b2g desktop locally...
OK, I cannot work on this bug, since my local B2G desktop build doesn't work (bug 937133). There is absolutely no progress on that bug, so I really don't know what to do. If anyone else can take the patch, test it and fix it on a working B2G build, it would be much appreciated.
Assignee: honzab.moz → nobody
Status: ASSIGNED → NEW
I found that we were throwing from shell.js during the failing tests - but I have no idea if that's the same issue that makes tbpl go bad since I don't get a crash locally. Anyway, try push with my shell.js fix: https://tbpl.mozilla.org/?tree=Try&rev=1c20ae9b98ad
NI on :fabrice to see if he can help here and own this ?
Flags: needinfo?(fabrice)
yes, I'm looking at that. For now I mostly need to reproduce the test failure locally.
Flags: needinfo?(fabrice)
Fabrice, thanks for taking this bug. You can also try whether e.g. forecast.io homepage gets offline-cached or not. Visit that page, that will trigger the path I've implemented. Then check the profile for OfflineCache dir and see if there is something (while no files where located there before visiting the page).
try run with a followup that let us not crash on forecast.io on an oop desktop build: https://tbpl.mozilla.org/?tree=Try&rev=e327f0b3131c
1.1 --- a/uriloader/prefetch/nsOfflineCacheUpdateService.cpp 1.2 +++ b/uriloader/prefetch/nsOfflineCacheUpdateService.cpp 1.3 @@ -760,29 +760,30 @@ nsOfflineCacheUpdateService::AllowOfflin 1.4 if (GeckoProcessType_Default != XRE_GetProcessType()) { 1.5 TabChild* child = TabChild::GetFrom(aWindow); 1.6 NS_ENSURE_TRUE(child, NS_ERROR_FAILURE); 1.7 1.8 if (!child->SendSetOfflinePermission(IPC::Principal(aPrincipal))) { 1.9 return NS_ERROR_FAILURE; 1.10 } 1.11 1.12 - nsAutoCString domain; 1.13 - rv = aPrincipal->GetBaseDomain(domain); 1.14 - NS_ENSURE_SUCCESS(rv, rv); 1.15 - 1.16 - nsOfflineCacheUpdateService::AllowedDomains()->PutEntry(domain); 1.17 } 1.18 else { 1.19 nsCOMPtr<nsIPermissionManager> permissionManager = 1.20 do_GetService(NS_PERMISSIONMANAGER_CONTRACTID); 1.21 if (!permissionManager) 1.22 return NS_ERROR_NOT_AVAILABLE; 1.23 1.24 rv = permissionManager->AddFromPrincipal( 1.25 aPrincipal, "offline-app", nsIPermissionManager::ALLOW_ACTION, 1.26 nsIPermissionManager::EXPIRE_NEVER, 0); 1.27 if (NS_FAILED(rv)) 1.28 return rv; 1.29 } 1.30 1.31 + nsAutoCString domain; 1.32 + rv = aPrincipal->GetBaseDomain(domain); 1.33 + NS_ENSURE_SUCCESS(rv, rv); 1.34 + 1.35 + nsOfflineCacheUpdateService::AllowedDomains()->PutEntry(domain); 1.36 + 1.37 return NS_OK; 1.38 } This is not good... actually it just expose more how the patch is not perfect. In single process model, in some cases setting the permission passes but its subsequent check doesn't pass, hence the calling code looks like: if (!nsContentUtils::OfflineAppAllowed(mDocument->NodePrincipal()) && !nsContentUtils::MaybeAllowOfflineAppByDefault(mDocument->NodePrincipal()) && !nsContentUtils::OfflineAppAllowed(mDocument->NodePrincipal())) { return; }
Attached patch followup — — Splinter Review
We were crashing in OOP because the permission was added from a principal (hence with a appId and isInBrowserElement flag) but the permission check was just an uri check. This patch uses principals to solve the issue. The alternative is to set the permission only using the uri, but I'm not sure this is correct since all the offline-cache storage using appId and the browser element flag.
Attachment #8337148 - Flags: review?(honzab.moz)
Comment on attachment 8337148 [details] [diff] [review] followup Review of attachment 8337148 [details] [diff] [review]: ----------------------------------------------------------------- r=honzab Fantastic! Thanks Fabrice for this work. ::: uriloader/prefetch/nsOfflineCacheUpdateService.cpp @@ +786,5 @@ > > rv = permissionManager->AddFromPrincipal( > aPrincipal, "offline-app", nsIPermissionManager::ALLOW_ACTION, > nsIPermissionManager::EXPIRE_NEVER, 0); > + NS_ENSURE_SUCCESS(rv, rv); if (MOZ_WARN_IF(NS_FAILED(rv)) return rv; ;)
Attachment #8337148 - Flags: review?(honzab.moz) → review+
Clarifying ownership
Assignee: nobody → fabrice
What's left to get done here to get this landed?
(In reply to Jason Smith [:jsmith] from comment #49) > What's left to get done here to get this landed? I still haven't figured out the crash in M3, and I never reproduced it locally :(. My next step is to get the minidump out of the try build to check what's going on.
Attachment #829759 - Flags: checkin+
(In reply to Fabrice Desré [:fabrice] from comment #50) > crash in M3 Which one? Can I somehow help with this bug (actually take it back?)
(In reply to Honza Bambas (:mayhemer) from comment #51) > (In reply to Fabrice Desré [:fabrice] from comment #50) > > crash in M3 > > Which one? Can I somehow help with this bug (actually take it back?) See https://tbpl.mozilla.org/?tree=Try&rev=3133b746cd1d
I got to reproduce the crash on a local emulator build, but the stack is not directly helpful: Program received signal SIGTERM, Terminated. write () at bionic/libc/arch-arm/syscalls/write.S:10 10 ldmfd sp!, {r4, r7} (gdb) bt #0 write () at bionic/libc/arch-arm/syscalls/write.S:10 Cannot access memory at address 0x40061468
Honza, here's a IPC log of the test that crashes: [time:1387230094740304][349][PNeckoChild] Sending Msg_PHttpChannelConstructor([TODO]) [time:1387230094742730][308][PNeckoParent] Received Msg_PHttpChannelConstructor([TODO]) [time:1387230094757804][308][PHttpChannelParent] Sending Msg_OnStatus([TODO]) Server Etag: apptype-cached-3 No Client Etag [time:1387230094759693][308][PHttpChannelParent] Sending Msg_OnStatus([TODO]) [time:1387230094770846][308][PHttpChannelParent] Sending Msg_OnStartRequest([TODO]) [time:1387230094773485][308][PHttpChannelParent] Sending Msg_OnTransportAndData([TODO]) [time:1387230094774812][308][PHttpChannelParent] Sending Msg_OnStopRequest([TODO]) [time:1387230094818990][349][PBrowserChild] Sending Msg_SetBackgroundColor([TODO]) [time:1387230094820819][308][PBrowserParent] Received Msg_SetBackgroundColor([TODO]) [time:1387230094822369][349][PBrowserChild] Sending Msg_SetBackgroundColor([TODO]) [time:1387230094823602][308][PBrowserParent] Received Msg_SetBackgroundColor([TODO]) [time:1387230094846627][349][PLayerTransactionChild] Sending Msg_Update([TODO]) [time:1387230094848312][308][PLayerTransactionParent] Received Msg_Update([TODO]) [time:1387230094849084][308][PLayerTransactionParent] Sending reply Reply_Update([TODO]) [time:1387230094850600][349][PLayerTransactionChild] Received reply Reply_Update([TODO]) [time:1387230094862101][349][PHttpChannelChild] Received Msg_OnStatus([TODO]) [time:1387230094863402][349][PHttpChannelChild] Received Msg_OnStatus([TODO]) [time:1387230094864232][349][PHttpChannelChild] Received Msg_OnStartRequest([TODO]) [time:1387230094903294][349][PStorageChild] Sending Msg_AsyncPreload([TODO]) [time:1387230094904812][308][PStorageParent] Received Msg_AsyncPreload([TODO]) [time:1387230094906290][308][PStorageParent] Sending Msg_LoadDone([TODO]) [time:1387230094907331][349][PStorageChild] Sending Msg_AsyncGetUsage([TODO]) [time:1387230094909095][308][PStorageParent] Received Msg_AsyncGetUsage([TODO]) [time:1387230094910188][308][PStorageParent] Sending Msg_LoadUsage([TODO]) [time:1387230094914451][349][PBrowserChild] Sending Msg_SetStatus([TODO]) [time:1387230094915675][308][PBrowserParent] Received Msg_SetStatus([TODO]) [time:1387230094932173][349][PBrowserChild] Sending Msg_AsyncMessage([TODO]) [time:1387230094935307][308][PBrowserParent] Received Msg_AsyncMessage([TODO]) [time:1387230094943136][349][PHttpChannelChild] Received Msg_OnTransportAndData([TODO]) [time:1387230094946002][349][PHttpChannelChild] Received Msg_OnStopRequest([TODO]) [time:1387230094947078][349][PHttpChannelChild] Sending Msg_DocumentChannelCleanup([TODO]) [time:1387230094948330][308][PHttpChannelParent] Received Msg_DocumentChannelCleanup([TODO]) [time:1387230094951188][349][PBrowserChild] Sending Msg_SetBackgroundColor([TODO]) [time:1387230094952304][308][PBrowserParent] Received Msg_SetBackgroundColor([TODO]) [time:1387230094953301][349][PBrowserChild] Sending Msg_SetBackgroundColor([TODO]) [time:1387230094953984][308][PBrowserParent] Received Msg_SetBackgroundColor([TODO]) [time:1387230094972077][349][PStorageChild] Received Msg_LoadDone([TODO]) [time:1387230094973087][349][PStorageChild] Received Msg_LoadUsage([TODO]) [time:1387230094979664][349][PBrowserChild] Sending Msg_SetOfflinePermission([TODO]) [time:1387230094981697][308][PBrowserParent] Received Msg_SetOfflinePermission([TODO]) [time:1387230094984050][308][PContentParent] Sending Msg_AddPermission([TODO]) [time:1387230094985605][308][PContentParent] Sending Msg_AddPermission([TODO]) [time:1387230094986479][353][PContentChild] Received Msg_AddPermission([TODO]) [time:1387230095024162][349][PBrowserChild] Sending Msg_SetBackgroundColor([TODO]) [time:1387230095025509][308][PBrowserParent] Received Msg_SetBackgroundColor([TODO]) [time:1387230095026426][349][PBrowserChild] Sending Msg_SetBackgroundColor([TODO]) [time:1387230095027099][308][PBrowserParent] Received Msg_SetBackgroundColor([TODO]) [time:1387230095060890][349][PContentChild] Received Msg_AddPermission([TODO]) [time:1387230095062953][349][PContentChild] Sending Msg_ScriptError([TODO]) [time:1387230095064422][308][PContentParent] Received Msg_ScriptError([TODO]) [time:1387230095077610][349][PBrowserChild] Sending Msg_POfflineCacheUpdateConstructor([TODO]) [time:1387230095079737][308][PBrowserParent] Received Msg_POfflineCacheUpdateConstructor([TODO]) [time:1387230095105680][308][PBrowserParent] Sending Msg_Destroy([TODO]) [time:1387230095160560][308][PContentParent] Sending Msg_AudioChannelNotify([TODO]) [time:1387230095161894][353][PContentChild] Received Msg_AudioChannelNotify([TODO]) [Parent 308] WARNING: waitpid failed pid:349 errno:10: file /home/fabrice/dev/b2g-inbound/ipc/chromium/src/base/process_util_posix.cc, line 254 [Parent 308] WARNING: waitpid failed pid:349 errno:10: file /home/fabrice/dev/b2g-inbound/ipc/chromium/src/base/process_util_posix.cc, line 254 [Parent 308] WARNING: Failed to deliver SIGKILL to 349!(3).: file /home/fabrice/dev/b2g-inbound/ipc/chromium/src/chrome/common/process_watcher_posix_sigchld.cc, line 118 [time:1387230098148384][308][PStorageParent] Sending Msg_Observe([TODO]) [time:1387230098150456][353][PStorageChild] Received Msg_Observe([TODO])
Triage believes this continues is a blocker, until Fabrice's tests prove otherwise.
Flags: needinfo?(fabrice)
Note - we're running out of time to land this. This needs to land by 12/20 or we're going to get pressure to punt this from 1.2.
try run with all three patches: https://tbpl.mozilla.org/?tree=Try&rev=1c9c10b564d2 The failure we had is not directly due to the offline cache code. We somehow get the wrong appid because the TabContext is not set for iframes inserted in test_app_update.html. I will file a bug on that.
Flags: needinfo?(fabrice)
Honza, any objections to land that now?
Yes, land this, it that is so import don't way for any feedback from me. I hope you didn't let this slip just because I was offline on Friday.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: