Spun off from bug 786295, we need to fix our code so that browsers within apps report the correct appId and browser flag. Currently IndexedDB sees all browsers within apps as having appId 0.

This is the beginning of what we've got so far. Bug 786295 comment 33 has not yet been addressed.

This is breaking our security model so this is definitely a blocker. Justin: Do you think you could finish this one up? This stuff feels complex enough that it needs your help.

It's going to take time away from memshrink bugs if I work on this... Is Ben giving up, or does he have something more important than memshrink to be doing?

Basically it seemed like a poor use of resources given the amount of guidance he needed from you.

(In reply to Jonas Sicking (:sicking) from comment #4) > Basically it seemed like a poor use of resources given the amount of > guidance he needed from you. Can we be explicit about the trade-off here? What is Ben going to work on instead? Can he pick up MemShrink bugs? It's obviously helpful for a capable programmer like Ben to learn this code.

Even if we agreed that memshrink was the absolute top priority project right now that doesn't mean that we can only do memshrink in order to ship B2G. So wasting resources is a bad idea no matter what. Bent is going to be working on testing our IPC code since we have very little tests for it and B2G is basically the first time that use it to the extent that we are. My impression was that we're seeing a lot of system-wide issues due to low memory right now, even though in theory we should be able to kill child processes on OOM and move on unaffected.

I guess I don't buy the idea that teaching Ben about this code is somehow a waste of resources. But it sounds like he doesn't want to work on it anymore, in which case of course continuing to teach him about it would be a waste. That's OK. But unfortunately it means that we're going to have to trade off between two high priorities, because I'm already working well above capacity.

Well, the question was if it was actually saving you any work given the amount of time needed to be spent on reviews. If it was then I can try to find someone else to finish this up.

If I have to get someone else up to the place where Ben was, that's probably not going to save me any work.

Ben, can you please give me a precise STR for the behavior you need fixed here? If you've written any automated testcases, or if testcases you've written can be easily modified to test this behavior, that would also be useful.

(In reply to Justin Lebar [:jlebar] from comment #10) > Ben, can you please give me a precise STR for the behavior you need fixed > here? Ben, I could really use some help here, so I know how to test the changes I'm making in this bug.

Sorry, I've been distracted trying to finish/land bug 786295. So here's the deal. I have a test in bug 786295 that seems to work as expected. Basically I have this setup: <!-- test_webapp_clearBrowserData.html --> <iframe mozbrowser mozapp="http://example.org/manifest.webapp" remote="true" src="webapp_clearBrowserData_appFrame.html"/> <!-- process boundary --> <!-- webapp_clearBrowserData_appFrame.html --> <iframe mozbrowser src="webapp_clearBrowserData_browserFrame.html"/> <!-- webapp_clearBrowserData_browserFrame.html --> <!-- stuff that uses IndexedDB --> In this test I get indexedDB data for the inner-most browser frame with origin "1+t+http+++example.org", which is correct. To verify you can pause the test before the clearing operation and look in the directory <profile>/indexedDB/. So whatever I'm doing in this test shows that our current code works as expected. However, on B2G, I see brokenness. I don't know of any automated tests that expose this problem, but the STR are simple: 1. Open The browser application on B2G (I use desktop b2g builds, but I'd assume the same thing would happen on the device too). 2. Enter "robnyman.github.com/html5demos/indexeddb" in the URL bar and go. 3. Look in the directory: <profile>/indexedDB/. 4. Observe "19+f+app+++browser.gaiamobile.org" and "0+t+http+++robnyman.github.com". I guess the browser appId could be different on your machine, not sure how that works. But in any case, the robnyman origin should have the same appId as the browser app, with a "t" instead of "f". I consistently see 0 as the appId for this origin.

> So here's the deal. This is perfect (for now, anyway). Thanks a ton.

Note that this almost certainly also affect where cookies and appcached content goes. The cookie and appcache code uses nsILoadContext (implemented by docshell) to figure out which datajar to store their data in. Unfortunately I don't have any steps to reproduce which are as simple. One way to reproduce it would be to remove the hack at [1] and then check that pressing the "clear private data" button in the browser UI actually clears cookies. For example, it should log you out of bugzilla. [1] http://hg.mozilla.org/mozilla-central/file/5c82f5a5e90d/dom/apps/src/Webapps.jsm#l1742

I have patches which pass bent's testcase. I need to clean them up, do some more testing, and see if I can write an automated test here.

It's a long name, but at least it's clear. (We originally considered GetSameTypeParentIgnoreBrowserFrame and rejected it as too long. But I figure as long as I'm changing this for clarity, we might as well go all the way.)

This patch doesn't cover the entire rename, but it gets most of them.

Again, this doesn't encompass the changes to all users of this interface, but it gets most of them.

This changes the interface of nsIDocShell to be more clear about the app/browser distinction. The biggest change is to how you mark a docshell as being a browser or part of an app -- now you do either SetIsApp(appId) or SetIsBrowserInsideApp(appId) . To create a browser outside an app (something we don't do in B2G), you pass NO_APP_ID to SetIsBrowserInsideApp.

This changes nsFrameLoader to use the modified nsIDocShell and TabParent interfaces, and also clarifies the browser/app distinction within the frameloader code.

This actually fixes bug 802366, passing the right parameters to the docshell so that browser frames get the app id of their containing apps. The order of arguments to many functions is changed from (is-browser, app) to (app, is-browser), for consistency with other modules. "App" is replaced with "OwnOrContainingApp" where appropriate to emphasize that we may have inherited this app from our parent process.

This is a lot of mostly un-interesting changes; sorry, Boris and Chris. Hopefully it won't be a problem to take such a large change on Aurora. Thankfully most of the non-trivial changes here are to B2G-only code. Basically, I felt like the main bug here was that in half of our interfaces, we used "is browser frame/element" to mean "browser or app", and in the other half, we used it to mean "is browser not app". I've fixed that, so now it's clear what's going on. Once I fixed the interfaces, the functional bug wasn't hard to resolve. I also modified Tab{Parent,Child} and nsFrameLoader to call "app" "ownOrContainingApp", to emphasize that we might have inherited the app from a parent process. I left nsIDocShell::appId alone, because changing that would have necessitated changing nsILoadGroup and a /lot/ of users in Necko. I also don't feel like nsILoadGroup::appId is as confusing as TabParent::appId, because you always want the loadgroup's own or containing appId, but you sometimes care that the tabparent/frameloader's appid is inherited. I'll look into cooking up an automated test for this, but I wanted to get these patches out in the meantime.

Comment on attachment 675740 [details] [diff] [review] Bug 802366 - Part 3c, the main event: dom/ipc changes. Much better, thanks!

Comment on attachment 675728 [details] [diff] [review] Prelude, part 1: Rename in-process-browser-frame-shown to in-process-browser-or-app-frame-shown. r=me

Comment on attachment 675730 [details] [diff] [review] Prelude, part 2: Rename GetParentIgnoreBrowserFrame to GetSameTypeParentIgnoreBrowserAndAppBoundaries. Please rev the docshell iid. r=me with that.

Comment on attachment 675731 [details] [diff] [review] Bug 802366 - Prelude, part 3: Make some methods on nsIPrincipal infallible, and improve documentation on other methods. Can you not nix more C++ blocks there? In any case, r=me. Much better comments!

Comment on attachment 675732 [details] [diff] [review] Bug 802366 - Prelude, part 4: Use and simplify nsScriptSecurityManager::GetDocShellCodebasePrincipal. r=me

Comment on attachment 675733 [details] [diff] [review] Bug 802366 - Prelude, part 5: Improve comments in nsIDocShellTreeItem.idl and nsILoadContext.idl. s/This is call/This getter/ and r=me.

Comment on attachment 675736 [details] [diff] [review] Bug 802366 - Part 2 (fold into main part): Changes to nsIMozBrowserFrame (ReallyIsBrowser --> ReallyIsBrowserOrApp) and users of the interface. r=me

Comment on attachment 675734 [details] [diff] [review] Bug 802366 - Part 1 (fold into main part): s/GetIsContentBoundary/GetIsBrowserOrApp/ s/GetIsBelowContentBoundary/GetIsInBrowserOrApp/. r=me

Comment on attachment 675738 [details] [diff] [review] Bug 802366 - Part 3a, the main event: docshell changes. r=me

Comment on attachment 675739 [details] [diff] [review] Bug 802366 - Part 3b, the main event: frameloader changes. r=me. Thank you for breaking this up into sane-sized chunks!

> Can you not nix more C++ blocks there? You know, I didn't even notice those. I think I can nix them. Strange that it compiled at all! > Please rev the docshell iid. For those who didn't read the patch queue: I do this in a later patch (sorry!). I'll figure out some order to land the patches in so that I rev the IID the first time I touch the interface. > Thank you for breaking this up into sane-sized chunks! I'm happy it was useful! Thanks a lot for the quick reviews (cjones too!).

Try run for 5acfc98200e7 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=5acfc98200e7 Results (out of 67 total builds): success: 51 warnings: 6 failure: 10 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jlebar@mozilla.com-5acfc98200e7

bent, with respect to writing a test for this: It seems that the mochitest you added in bug 786295 should work as a test for this bug if we changed it so that the app frame was loaded in-process and the browser frame was loaded oop. (Right now, it's the opposite.) But when I make the app frame in-process, I get the following exception: JavaScript error: http://example.org/tests/dom/indexedDB/test/webapp_clearBrowserData_appFrame.html, line 37: indexedDB is null This is a clean build of trunk; all I've done is change setAttribute("remote", "true") to setAttribute("remote", "false"). Do you have any idea what might be going on?

Oh, I see. I filed bug 806127.

Comment on attachment 675922 [details] [diff] [review] Epilogue: Remove workaround for bug 802366 in Webapps.jsm. Stealing this one

By the way, when landing this, we should send an email to dev-b2g saying that it's expected that this will cause all in-browser cookies to get lost. I.e. you'll automatically get logged out from all your websites and lose any localStorage/indexedDB data that any website has created. It's just a one-shot loss though, so not something that's expected to affect anyone but dogfooders.

Sounds good. I can land once I get a review in bug 806168. (I'd like to land with test coverage.)

Mail sent to dev-b2g (although, is that the right list for dogfooders?) and landed this, bug 806127, and bug 806168. Hopefully this sticks. :) https://hg.mozilla.org/integration/mozilla-inbound/rev/86d8067fa9e5 https://hg.mozilla.org/integration/mozilla-inbound/rev/e710acd6ef11 https://hg.mozilla.org/integration/mozilla-inbound/rev/212e1383eb6e https://hg.mozilla.org/integration/mozilla-inbound/rev/37357ffceb05 https://hg.mozilla.org/integration/mozilla-inbound/rev/8048f22347eb https://hg.mozilla.org/integration/mozilla-inbound/rev/a6a847452dbf https://hg.mozilla.org/integration/mozilla-inbound/rev/9b533d393792 https://hg.mozilla.org/integration/mozilla-inbound/rev/7fa2f2eff658 https://hg.mozilla.org/integration/mozilla-inbound/rev/a844765fb5d9 https://hg.mozilla.org/integration/mozilla-inbound/rev/fcf7daa94373 https://hg.mozilla.org/integration/mozilla-inbound/rev/4eba57a286d7

a6a847452dbf had the wrong bug number in the commit message, so I backed out and relanded that one. https://hg.mozilla.org/integration/mozilla-inbound/rev/f9a9450a82af https://hg.mozilla.org/integration/mozilla-inbound/rev/d718d87d4bb5

And backed out for bustage on Linux-32 opt Cipc/Ripc orange. https://tbpl.mozilla.org/php/getParsedLog.php?id=16604366&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=16604791&tree=Mozilla-Inbound https://hg.mozilla.org/integration/mozilla-inbound/rev/22890a4e431f https://hg.mozilla.org/integration/mozilla-inbound/rev/8c25f2a7a2f0 https://hg.mozilla.org/integration/mozilla-inbound/rev/2633b018c0a0 https://hg.mozilla.org/integration/mozilla-inbound/rev/df6cbf360b25 https://hg.mozilla.org/integration/mozilla-inbound/rev/1e04d8f5e601 https://hg.mozilla.org/integration/mozilla-inbound/rev/9c357f7836b4 https://hg.mozilla.org/integration/mozilla-inbound/rev/ea0327916114 https://hg.mozilla.org/integration/mozilla-inbound/rev/039573154dcf https://hg.mozilla.org/integration/mozilla-inbound/rev/2729e28870be https://hg.mozilla.org/integration/mozilla-inbound/rev/b103561f6a8b https://hg.mozilla.org/integration/mozilla-inbound/rev/3bc04269c76a https://hg.mozilla.org/integration/mozilla-inbound/rev/a2afde356192

Wait, this got backed out...

Unfortunately I can't reproduce the test failure locally. Chris, do you have any insight as to why I'd get [Child 2198] ###!!! ABORT: creating CanvasLayer back buffer failed!: file ../../../gfx/layers/basic/BasicCanvasLayer.cpp, line 443 ?

That will happen if we OOM or if the IPC channel has been closed.

Well, the next line is [Parent 2179] WARNING: pipe error (40): Connection reset by peer: file ../../../ipc/chromium/src/chrome/common/ipc_channel_posix.cc, line 421 so I'd guess the child is dying before the parent? But it's strange that we'd cause OOM here, so maybe something else is going on.

That means the ipc socket was closed unexpectedly, which would tend to suggest that the parent wasn't in the middle of shutting down. But I agree, OOM would be odd.

(i.e. the unexpected close was most likely caused by the child aborting.)

I took a look at the reftest failure, which seems more tractable (in particular, since I can reproduce it). What's happening is that layout/reftests/reftest-sanity/invalidation.html is hanging. It's hanging because MozReftestInvalidate is never fired. Perhaps a hint to what's going on is the fact that when I run with my patches, I get [Child process] WARNING: "Flushing retained layers!" from nsLayoutUtils::PaintFrame, when it's called from CanvasRenderingContext2D::DrawWindow. I don't get that warning without my patches. I'm trying to figure out how any of this code should be affected by these changes and not coming up with much. Any ideas would be appreciated.

Aha! Okay, the problem (at least for reftest) is in TabChild::IsRootContentDocument(). Here's what it originally did (complete with the confusing variable names): bool TabChild::IsRootContentDocument() { // (mIsBrowserElement means browser-or-app, I think.) if (mIsBrowserElement || mAppId == nsIScriptSecurityManager::NO_APP_ID) { // The child side of a browser or app element always behaves like a root // content document. return true; } // Otherwise, we're the child side of an <html:app remote=true> // embedded in an outer <html:app>. These don't behave like root // content documents in nested contexts. Because of bug 761935, // <html:browser remote> and <html:app remote> can't nest, so we // assume this isn't the root. When that bug is fixed, we need to // revisit that assumption. return false; } That comment is referring to something which currently can't happen. I even say it can't happen elsewhere in the patches, in ContentParent::CreateBrowserOrApp: if (aIsBrowserElement || !aOwnOrContainingApp) { // ... return; } // If we got here, we have an app and we're not a browser element. In this // case, we assume the app is our own app, not a containing one. That is, // if you're a remote iframe inside an app, you must either be a browser or // a new app; you can't be a non-browser non-app iframe. Chris, can I return true unconditionally for TabChild::IsRootContentDocument(), with a similar comment?

Seems fine to me.

> Chris, can I return true unconditionally for TabChild::IsRootContentDocument(), with a > similar comment? >> Seems fine to me. We established on IRC that this is not in fact fine; we need to return false for <mozapp> inside <mozapp>, because we want the inner app's iframe to be transparent. I designed these patches around the idea that we'd never treat <mozapp> inside <mozapp> different from a stand-alone mozapp -- indeed, that was a basic assumption underlying the idea that the (appid, isbrowser) tuple was all we needed to attach to a frame. So fixing this in a sane way requires basically rewriting most of these patches so we always carry the full information -- now (appid, containing-app-id, is-browser). We could maybe hack around this, but I think this bug demonstrates the harm of this sort of hack. Also, this bug isn't the last time this code will be touched (see bug 808206). I think I know basically what needs to be done here; I just need some time. Jason and I will coordinate to make sure that my new solution works for his purposes.

I need to clean up these patches and finish the automated test, but I think it's working. https://tbpl.mozilla.org/?tree=Try&rev=c45bbb2827e9

Try run for bbdb857a544b is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=bbdb857a544b Results (out of 2 total builds): exception: 2 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jlebar@mozilla.com-bbdb857a544b

Try run for c45bbb2827e9 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=c45bbb2827e9 Results (out of 4 total builds): failure: 4 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jlebar@mozilla.com-c45bbb2827e9

Try run for 02ce04d6ea55 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=02ce04d6ea55 Results (out of 57 total builds): success: 47 warnings: 10 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jlebar@mozilla.com-02ce04d6ea55

Try run for 02ce04d6ea55 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=02ce04d6ea55 Results (out of 60 total builds): success: 49 warnings: 11 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jlebar@mozilla.com-02ce04d6ea55

Try run for 02ce04d6ea55 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=02ce04d6ea55 Results (out of 61 total builds): success: 49 warnings: 11 failure: 1 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jlebar@mozilla.com-02ce04d6ea55

Try run for bbdb857a544b is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=bbdb857a544b Results (out of 3 total builds): exception: 2 failure: 1 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jlebar@mozilla.com-bbdb857a544b

Try run for c45bbb2827e9 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=c45bbb2827e9 Results (out of 5 total builds): failure: 5 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jlebar@mozilla.com-c45bbb2827e9

This lets us make the correct decision in TabChild::IsRootContentDocument. It's also nicer in general, I think, because TabChild, TabParent, and TabContext all expose the same interface.

This is a change in behavior, but I think it's more correct than the old behavior.

I'd really like to have a test for this app-in-app behavior, but it's hard to write, and we need to fix this bug. I'll see if I can get to it sometime soon, but I'm not going to lose sleep over it. I have other bugs to lose sleep over. :)

Comment on attachment 679067 [details] [diff] [review] Volume II, Part 2: Use TabContext to make IDB handling more correct in TabParent. Review of attachment 679067 [details] [diff] [review]: ----------------------------------------------------------------- Yeah, this looks right. Thanks!

Comment on attachment 679066 [details] [diff] [review] Volume II part 1: Add and use TabContext, which holds app-in-app data. >diff --git a/dom/ipc/PContent.ipdl b/dom/ipc/PContent.ipdl >+// This is equivalent to AppFrameIPCTabContext with all fields set to NO_APP_ID. >+struct VanillaFrameIPCTabContext Not too enthusiastic about this name, but I don't have a better idea, so OK! >diff --git a/dom/ipc/TabChild.cpp b/dom/ipc/TabChild.cpp >-bool > TabChild::IsRootContentDocument() > { >+ // (It would probably make sense if <html:iframe remote=true> or >+ // <xul:browser remote=true> inside an app was also not a root content >+ // document. But that breaks our tests, and it's not a configuration we >+ // ship.) > Well, <app> within <app> is supposed to behave like <iframe> within a content document wrt painting. The confusing part is that we're using "is root content document" to compute a graphics trait, which sort of doesn't make sense here. >diff --git a/dom/ipc/TabContext.cpp b/dom/ipc/TabContext.cpp >+TabContext::TabContext() >+ : mInitiailized(false) mInitialized >+ TabContext *context; >+ if (ipcContext.openerParent()) { >+ context = static_cast<TabParent*>(ipcContext.openerParent()); >+ if (context->IsBrowserElement() && !ipcContext.isBrowserElement()) { >+ // If the TabParent corresponds to a browser element, then it can only >+ // open other browser elements, for security reasons. We should have >+ // checked this before calling the TabContext constructor, so this is >+ // a fatal error. >+ MOZ_CRASH(); MOZ_NOT_REACHED() has the same effect and may be clearer, but I don't particularly care which you use. >+bool >+TabContext::SetTabContext(const TabContext& aContext) >+{ >+ NS_ENSURE_FALSE(mInitiailized, false); >+ Any reason why we shouldn't assert !mInitialized? dom/ipc doesn't do control-flow affecting macros. Explicit return stmts please. >+IPCTabContext >+TabContext::AsIPCTabContext() const >+{ >+ if (mIsBrowser) { >+ BrowserFrameIPCTabContext context; >+ context.browserFrameOwnerAppId() = mContainingAppId; >+ return IPCTabContext(context); |return BrowserFrameIPCTabContext(mContainingAppId)| is equivalent and much cleaner. >+ AppFrameIPCTabContext context; >+ context.ownAppId() = mOwnAppId; >+ context.appFrameOwnerAppId() = mContainingAppId; >+ return IPCTabContext(context); Similarly, |return AppFrameIPCTabContext(mOwnAppId, mContainingAppId)|. >diff --git a/dom/ipc/TabContext.h b/dom/ipc/TabContext.h >+ bool mInitiailized; >+ bool mIsBrowser; Reorder these after the uint32_t members. OK, this looks really really good. r=me with comments above addressed.

> Well, <app> within <app> is supposed to behave like <iframe> within a > content document wrt painting. The confusing part is that we're using > "is root content document" to compute a graphics trait, which sort of > doesn't make sense here. Thanks; I updated the comment to try to make it clearer. >diff --git a/dom/ipc/TabContext.cpp b/dom/ipc/TabContext.cpp > MOZ_NOT_REACHED() has the same effect and may be clearer, but I don't > particularly care which you use. I'd considered that, but I rejected it based on comments in MFBT. MOZ_NOT_REACHED() expands to |MOZ_ASSERT(false); MOZ_NOT_REACHED_MARKER();|, and the comment on MOZ_NOT_REACHED_MARKER() says: "[I]t is undefined behavior for execution to reach this point. No guarantees are made about what will happen if this is reached at runtime." I guess I see the purpose of MOZ_NOT_REACHED() existing as described in the comments (particularly since it indicates noreturn), but ISTM that an assertion of the sort you describe ought to exist as well. >+bool >+TabContext::SetTabContext(const TabContext& aContext) >+{ >+ NS_ENSURE_FALSE(mInitiailized, false); >+ > dom/ipc doesn't do control-flow affecting macros. Explicit return > stmts please. Heh, I think you may have lost this battle some time ago: $ git grep NS_ENSURE dom/ipc | cut -d ':' -f 1 | uniq -c | sort -rn 31 dom/ipc/Blob.cpp 17 dom/ipc/ContentParent.cpp 15 dom/ipc/ContentChild.cpp 13 dom/ipc/TabChild.cpp 11 dom/ipc/TabParent.cpp 9 dom/ipc/TabContext.cpp 5 dom/ipc/ProcessPriorityManager.cpp 2 dom/ipc/TabMessageUtils.cpp 2 dom/ipc/PermissionMessageUtils.cpp Is it OK if I leave these in? >diff --git a/dom/ipc/TabContext.h b/dom/ipc/TabContext.h >>+ bool mInitiailized; >>+ bool mIsBrowser; > > Reorder these after the uint32_t members. I moved mInitialized to the beginning, because I think that makes more sense than having it at the end of the members.

(In reply to Justin Lebar [:jlebar] from comment #74) > >+bool > >+TabContext::SetTabContext(const TabContext& aContext) > >+{ > >+ NS_ENSURE_FALSE(mInitiailized, false); > >+ > > > dom/ipc doesn't do control-flow affecting macros. Explicit return > > stmts please. > > Heh, I think you may have lost this battle some time ago: > > Is it OK if I leave these in? > Grumble. 10 review demerits ;).

I needed to make the IDB handling do the same sort of "allow non-browsers to read browsers, but not vice versa" thing that we do for popups. Hopefully this is the last patch here; this bug is becoming unweildy!

Fingers crossed... https://hg.mozilla.org/integration/mozilla-inbound/rev/bd00833b94d1 https://hg.mozilla.org/integration/mozilla-inbound/rev/b5a1e7b624c1 https://hg.mozilla.org/integration/mozilla-inbound/rev/69f25313dbb0 https://hg.mozilla.org/integration/mozilla-inbound/rev/7f0d506a10e9 https://hg.mozilla.org/integration/mozilla-inbound/rev/d1ed361b4806 https://hg.mozilla.org/integration/mozilla-inbound/rev/f479bbbe4b36 https://hg.mozilla.org/integration/mozilla-inbound/rev/065dde531bf9 https://hg.mozilla.org/integration/mozilla-inbound/rev/5b7551ba16ba https://hg.mozilla.org/integration/mozilla-inbound/rev/69bbc60a4bad https://hg.mozilla.org/integration/mozilla-inbound/rev/df92dc67212e https://hg.mozilla.org/integration/mozilla-inbound/rev/2c1151f4ad8b https://hg.mozilla.org/integration/mozilla-inbound/rev/5eb2472d835d

http://mozillamemes.tumblr.com/post/19498220636/try-server-takes-the-beatings-so-mozilla-inbound Backed out for Windows bustage. https://hg.mozilla.org/integration/mozilla-inbound/rev/5dc1c0530b00 https://tbpl.mozilla.org/php/getParsedLog.php?id=16912797&tree=Mozilla-Inbound e:/builds/moz2_slave/m-in-w32/build/dom/indexedDB/IndexedDatabaseManager.cpp(621) : error C4430: missing type specifier - int assumed. Note: C++ does not support default-int e:/builds/moz2_slave/m-in-w32/build/dom/indexedDB/IndexedDatabaseManager.cpp(621) : error C2143: syntax error : missing ',' before '&' e:/builds/moz2_slave/m-in-w32/build/dom/indexedDB/IndexedDatabaseManager.cpp(623) : error C2511: 'bool mozilla::dom::indexedDB::IndexedDatabaseManager::TabContextMayAccessOrigin(const int)' : overloaded member function not found in 'mozilla::dom::indexedDB::IndexedDatabaseManager' e:\builds\moz2_slave\m-in-w32\build\dom\indexeddb\IndexedDatabaseManager.h(47) : see declaration of 'mozilla::dom::indexedDB::IndexedDatabaseManager'

(In reply to Justin Lebar [:jlebar] from comment #78) > https://hg.mozilla.org/integration/mozilla-inbound/rev/69bbc60a4bad > https://hg.mozilla.org/integration/mozilla-inbound/rev/df92dc67212e > https://hg.mozilla.org/integration/mozilla-inbound/rev/2c1151f4ad8b > https://hg.mozilla.org/integration/mozilla-inbound/rev/5eb2472d835d I believe these were posted to the wrong bug.

This was causing Crashtest-IPC timeouts too. https://tbpl.mozilla.org/php/getParsedLog.php?id=16913805&tree=Mozilla-Inbound TEST-UNEXPECTED-FAIL | file:///home/cltbld/talos-slave/test/build/reftest/tests/dom/indexedDB/crashtests/726376-1.html | application timed out after 330 seconds with no output PROCESS-CRASH | file:///home/cltbld/talos-slave/test/build/reftest/tests/dom/indexedDB/crashtests/726376-1.html | application crashed (minidump found)

> I believe these were posted to the wrong bug. It's basically become one giant patch queue at this point, since patches from this bug have to land before and after patches from other bugs, in order to have any hope of all csets compiling. Which, to be fair, I haven't done a good job of by any measure. Sorry. :(

We are really too clever for our own good with these ascii origins. In particular, the ascii origin for (no-browser, no-app [i.e., app 0], mozilla.org) is not the same as "mozilla.org". :-/ Anywho, this is a simple fix, I think. https://tbpl.mozilla.org/?tree=Try&rev=f6e00ea3e664

> It's basically become one giant patch queue at this point, since patches from this bug > have to land before and after patches from other bugs Actually, that's not true at all. This bug regains a modicum of sanity. And a -Wunused -Werror fix for macos. https://tbpl.mozilla.org/?tree=Try&rev=07ee76cc1f02

And again with working trychooser params. (Sorry for the spam.) https://tbpl.mozilla.org/?tree=Try&rev=61ff8164a3f9

Once more, with feeling. https://hg.mozilla.org/integration/mozilla-inbound/rev/757ae1ab716a https://hg.mozilla.org/integration/mozilla-inbound/rev/9b31589a1590 https://hg.mozilla.org/integration/mozilla-inbound/rev/a93adbd3b608 https://hg.mozilla.org/integration/mozilla-inbound/rev/5a06f3d7dc81 https://hg.mozilla.org/integration/mozilla-inbound/rev/927c8e35b4d0 https://hg.mozilla.org/integration/mozilla-inbound/rev/f69c06692358 https://hg.mozilla.org/integration/mozilla-inbound/rev/b8616abaacf1 https://hg.mozilla.org/integration/mozilla-inbound/rev/5bdd08dcb8ad

https://hg.mozilla.org/mozilla-central/rev/757ae1ab716a https://hg.mozilla.org/mozilla-central/rev/9b31589a1590 https://hg.mozilla.org/mozilla-central/rev/a93adbd3b608 https://hg.mozilla.org/mozilla-central/rev/5a06f3d7dc81 https://hg.mozilla.org/mozilla-central/rev/927c8e35b4d0 https://hg.mozilla.org/mozilla-central/rev/f69c06692358 https://hg.mozilla.org/mozilla-central/rev/b8616abaacf1 https://hg.mozilla.org/mozilla-central/rev/5bdd08dcb8ad

https://hg.mozilla.org/releases/mozilla-aurora/rev/b35bd88a4f75 https://hg.mozilla.org/releases/mozilla-aurora/rev/5104d55cfabc https://hg.mozilla.org/releases/mozilla-aurora/rev/47a94e631253 https://hg.mozilla.org/releases/mozilla-aurora/rev/dc3a2243fef9 https://hg.mozilla.org/releases/mozilla-aurora/rev/684f0b8beb12 https://hg.mozilla.org/releases/mozilla-aurora/rev/20ded27ba114 https://hg.mozilla.org/releases/mozilla-aurora/rev/bb4b31442abc https://hg.mozilla.org/releases/mozilla-aurora/rev/d54f23b093c4