780546 - Add simple test for opening named windows inside mozbrowser

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Justin Lebar (not reading bugmail)]

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)]

Attachment: Patch, v1

Comment 2

[Justin Lebar (not reading bugmail)]

https://hg.mozilla.org/try/rev/cc4749cd5a9f

Comment 3

[Justin Lebar (not reading bugmail)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/34a7d2909954

Comment 4

[Ed Morley [:emorley]]

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]]

https://hg.mozilla.org/mozilla-central/rev/34a7d2909954

Comment 6

[Justin Lebar (not reading bugmail)]

Except Ed backed this out...

Comment 7

[Ed Morley [:emorley]]

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

This looks like yet another race condition initializing the message manager during window.open.

Comment 9

[Justin Lebar (not reading bugmail)]

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)]

> Smaug, any pointers here would be appreciated.

I think I have a handle on this now...

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/6c7ed23db4b2