Last Comment Bug 780546 - Add simple test for opening named windows inside mozbrowser
: Add simple test for opening named windows inside mozbrowser
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla17
Assigned To: Justin Lebar (not reading bugmail)
:
Mentors:
Depends on: 780761 819320
Blocks: browser-api
  Show dependency treegraph
 
Reported: 2012-08-05 20:41 PDT by Justin Lebar (not reading bugmail)
Modified: 2013-04-04 13:53 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch, v1 (6.46 KB, patch)
2012-08-05 20:42 PDT, Justin Lebar (not reading bugmail)
mounir: review+
Details | Diff | Splinter Review

Description Justin Lebar (not reading bugmail) 2012-08-05 20:41:21 PDT
I've had this test sitting around for a while, but it was failing intermittently for mysterious reasons.  But I think when I fixed the other mozbrowser oranges, I fixed this one too.  We should check it in.

Patch in a moment.
Comment 1 Justin Lebar (not reading bugmail) 2012-08-05 20:42:08 PDT
Created attachment 649168 [details] [diff] [review]
Patch, v1
Comment 2 Justin Lebar (not reading bugmail) 2012-08-05 20:42:23 PDT
https://hg.mozilla.org/try/rev/cc4749cd5a9f
Comment 3 Justin Lebar (not reading bugmail) 2012-08-06 08:17:17 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/34a7d2909954
Comment 4 Ed Morley [:emorley] 2012-08-06 16:04:45 PDT
Backed out for causing bug 780761 one push after this landed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2ecf7d9b7580
Comment 5 Ryan VanderMeulen [:RyanVM] 2012-08-06 17:11:22 PDT
https://hg.mozilla.org/mozilla-central/rev/34a7d2909954
Comment 6 Justin Lebar (not reading bugmail) 2012-08-06 17:13:45 PDT
Except Ed backed this out...
Comment 7 Ed Morley [:emorley] 2012-08-07 06:40:54 PDT
(In reply to Ed Morley [:edmorley] from comment #4)
> Backed out for causing bug 780761 one push after this landed:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/2ecf7d9b7580

https://hg.mozilla.org/mozilla-central/rev/2ecf7d9b7580
Comment 8 Justin Lebar (not reading bugmail) 2012-08-08 13:40:47 PDT
This looks like yet another race condition initializing the message manager during window.open.
Comment 9 Justin Lebar (not reading bugmail) 2012-08-08 13:54:38 PDT
I don't see how this can be happening.

1) We must be hitting nsFrameMessageManager::SendAsyncMessageInternal's check

  if (mAsyncCallback) {
    NS_ENSURE_TRUE(mCallbackData, NS_ERROR_NOT_INITIALIZED);

2) If we're running code in BrowserElementParent.js for an OOP frame, that means nsFrameLoader::ShowRemoteFrame has run and tickled the remote-browser-frame-shown observer:

    mRemoteBrowser->Show(size);
    mRemoteBrowserShown = true;

    EnsureMessageManager();

    nsCOMPtr<nsIObserverService> os = services::GetObserverService();
    if (OwnerIsBrowserFrame() && os && !mRemoteBrowserInitialized) {
      os->NotifyObservers(NS_ISUPPORTS_CAST(nsIFrameLoader*, this),
                          "remote-browser-frame-shown", NULL);
      mRemoteBrowserInitialized = true;
    }

3) So EnsureMessageManager() has been called with mRemoteBrowserShown == true.

4) ... which gives us very few reasons why the message manager's callback data should be null:

nsresult
nsFrameLoader::EnsureMessageManager()
{
  NS_ENSURE_STATE(mOwnerContent);

  nsresult rv = MaybeCreateDocShell();
  if (NS_FAILED(rv)) {
    return rv;
  }

  if (!mIsTopLevelContent && !OwnerIsBrowserFrame() && !mRemoteFrame) {
    return NS_OK;
  }

  if (mMessageManager) {
    if (ShouldUseRemoteProcess()) {
      mMessageManager->SetCallbackData(mRemoteBrowserShown ? this : nullptr);
    }
    return NS_OK;
  }

5) ...unless this message manager is not the message manager which is failing?

Smaug, any pointers here would be appreciated.
Comment 10 Justin Lebar (not reading bugmail) 2012-08-08 15:12:59 PDT
> Smaug, any pointers here would be appreciated.

I think I have a handle on this now...
Comment 11 Ed Morley [:emorley] 2012-08-14 06:01:23 PDT
https://hg.mozilla.org/mozilla-central/rev/6c7ed23db4b2

Note You need to log in before you can comment on or make changes to this bug.