770821 - Flash 11.3 with protected mode enabled hangs Firefox when application started by mozrunner and page with flash objects is opened

Status: VERIFIED FIXED

(Testing :: Mozbase, defect)

Description

[Mihaela Velimiroviciu (:mihaelav)]

Firefox hangs when Mozmill test loads a page with embeded flash video with protected mode enabled.

Mozmill test: testFlash_SWFVideoEmbed (from endurance suite)
The tests fails either by running it individually (mozmill -t <path to test> <path to firefox build>) or by locally running the whole endurance suite.
Error: Disconnect Error: Application unexpectedly closed.

When the test page (http://mozqa.com/data/firefox/plugins/flash/test_swf_embed_nosound.html) is loaded manually, the browser does not hang and flash content is loaded and plays correctly.

Disabling flash protected mode (http://forums.adobe.com/thread/1018071) fixes the problem (running the mozmill test doesn't hang the browser and flash content is loaded, but it takes longer than loading the page manualy).

Comment 1

[Henrik Skupin (:whimboo) [⌚️UTC+2] (away 07/24 - 08/10)]

We will have to find steps how to reproduce this manually.

Comment 2

[u279076]

Mihaela, can you provide a link to a report so we can see which plug-ins are installed during the testrun?

Comment 3

[Henrik Skupin (:whimboo) [⌚️UTC+2] (away 07/24 - 08/10)]

Something Mihaela didn't mention is that when Flash 11.3 gets uninstalled or 11.2 installed, the hang doesn't occur. Same when the protected mode in Flash 11.3 gets disabled. So this is a new regression with the protected mode of Flash in 11.3.

Comment 4

[Henrik Skupin (:whimboo) [⌚️UTC+2] (away 07/24 - 08/10)]

The simplified test would look like:

function setupModule() {
  controller = mozmill.getBrowserController();
}

function testFlashViaEmbedTag() {
  controller.open("http://www.mozqa.com/data/firefox/plugins/flash/test_swf_embed_nosound.html");
  controller.waitForPageLoad();
}

Comment 5

[u279076]

Would it be possible to slow down the instantiation of each iteration so that we can see on which iteration this occurs? I suspect it's not happening on the first iteration as given by Mihaela's manual test.

Comment 6

[Henrik Skupin (:whimboo) [⌚️UTC+2] (away 07/24 - 08/10)]

Anthony, as comment 4 shows there is no iteration. This is the minimized testcase she can reproduce the issue with. I don't have a local Win7 or vista machine yet for testing. But I will try to do so tomorrow.

Comment 7

[Alex Keybl [:akeybl]]

If this is an in-product issue, I don't think we'll fix in time for FF14. Preemptively tracking for FF15, however.

Comment 8

[Henrik Skupin (:whimboo) [⌚️UTC+2] (away 07/24 - 08/10)]

I can see the same hang locally now with the Vista machine we use for Mozmill CI. I will investigate that. It's strange that this happens with such a simple test in Mozmill but can't be reproduced manually.

Comment 9

[Henrik Skupin (:whimboo) [⌚️UTC+2] (away 07/24 - 08/10)]

This is the code you will have to execute in the Error Console to get the hang:

var wm = Components.classes["@mozilla.org/appshell/window-mediator;1"].getService(Components.interfaces.nsIWindowMediator);
var win = wm.getMostRecentWindow("navigator:browser");
win.getBrowser().loadURI("http://www.mozqa.com/data/firefox/plugins/flash/test_swf_embed_nosound.html");

There is no Mozmill interaction necessary and can perfectly be reproduced manually by using .getBrowser().loadURI().

Comment 10

[Henrik Skupin (:whimboo) [⌚️UTC+2] (away 07/24 - 08/10)]

Actually this affects all supported versions of Firefox but only when the browser gets started via mozrunner. It doesn't matter if we create a new profile or use an existing one. Could be preferences we set or something else. I will dig into now.

Comment 11

[Henrik Skupin (:whimboo) [⌚️UTC+2] (away 07/24 - 08/10)]

So this is a somewhat strange interaction between the process manager of mozrunner and the Flash protected mode. The line of problem lays here:

https://github.com/mozilla/mozbase/blob/master/mozrunner/mozrunner/runner.py#L178

When I testing wise replace the use of 'self.process_handler' with subprocess.call(), Firefox gets started and does not hang when loading the page.

The same hang can be seen in Mozrunner 2.5.12 (which is used for Mozmill 1.5.13) and the latest version on the repository master.

Comment 12

[Henrik Skupin (:whimboo) [⌚️UTC+2] (away 07/24 - 08/10)]

Full steps to reproduce:

1. Download and start (execute run.cmd) the mozmill-env (http://people.mozilla.com/~hskupin/downloads/mozmill-env/1.5.13-windows.zip)
2. Start Firefox via: 'mozrunner %path_to_firefox%
3. Open the Error Console
4. Execute the following command:

var wm = Components.classes["@mozilla.org/appshell/window-mediator;1"].getService(Components.interfaces.nsIWindowMediator);
var win = wm.getMostRecentWindow("navigator:browser");
win.getBrowser().loadURI("http://www.mozqa.com/data/firefox/plugins/flash/test_swf_embed_nosound.html");


Jeff or Clint, does one of you have time to check what's wrong with the process manager of mozrunner? I'm not sure right now if this is our fault or if the Flash plugin behaves badly. So I will not update the product/component yet.

Comment 13

[Henrik Skupin (:whimboo) [⌚️UTC+2] (away 07/24 - 08/10)]

Please forget about step 4 in the last comment. It's enough to load the URL directly by c&p it into the location bar. Also this is based on all pages which contain flash objects.

Comment 14

[Henrik Skupin (:whimboo) [⌚️UTC+2] (away 07/24 - 08/10)]

We talked about that problem in yesterdays Automation Development meeting. Clint mentioned that he has a suspicion why it happens and wants to have a look at it.

Comment 15

[Henrik Skupin (:whimboo) [⌚️UTC+2] (away 07/24 - 08/10)]

It looks like that all the disconnects in our tests we only have on Windows Vista. I will check XP and Windows 7 now, so that we can probably exclude those os versions.

Comment 16

[Henrik Skupin (:whimboo) [⌚️UTC+2] (away 07/24 - 08/10)]

So this problem is happening for Windows Vista and 7, but not for Windows XP. Looks like a bad interaction between a new Windows feature in Vista with Flash 11.3 and the process handling of mozprocess.

Comment 17

[Henrik Skupin (:whimboo) [⌚️UTC+2] (away 07/24 - 08/10)]

Hm, looks like that other people also facing this problem. And those will most likely have not installed mozrunner.

See the appropriate Adobe forum post:
http://forums.adobe.com/message/4509105

I will check more of those posts on the web if I can find out some more information about this freeze.

Comment 18

[Henrik Skupin (:whimboo) [⌚️UTC+2] (away 07/24 - 08/10)]

Cheng, have you heard about something like that?

Comment 19

[Henrik Skupin (:whimboo) [⌚️UTC+2] (away 07/24 - 08/10)]

Given that it looks like a bad combination only with the Flash plugin lets move it into the right plugin component for now. I hope that some Adobe developers can help us too.

Comment 20

[Henrik Skupin (:whimboo) [⌚️UTC+2] (away 07/24 - 08/10)]

As Clint pointed out to me this is really related to mozprocess and how we handle child processes. He will give a more detailed comment soon, but for now lets move it into the mozbase component where it belongs to.

Comment 21

[Henrik Skupin (:whimboo) [⌚️UTC+2] (away 07/24 - 08/10)]

Also for now I have disabled the protected mode for our Vista machine which is affected by this bug.

Comment 22

[Henrik Skupin (:whimboo) [⌚️UTC+2] (away 07/24 - 08/10)]

We would need a fix for Mozmill 1.5.14 too. I hope the code doesn't have changed that much in that process handling area.

Comment 23

[Jeff Hammel]

(In reply to Henrik Skupin (:whimboo) from comment #22)
> We would need a fix for Mozmill 1.5.14 too. I hope the code doesn't have
> changed that much in that process handling area.

It has.  Quite a bit

Comment 24

[cmtalbert]

Attachment: Patch v1

So, the issue here is that we create job objects to manage our sub-processes. And we do not allow our subprocesses to create their own job objects (thus breaking away from our control).  For the exact same reason, adobe is making their own job object for their new low-rights process when running in protected mode (so they have total control over their sub-processes).  This patch will still manage our subprocesses and ensure they are properly killed but if one of our subprocesses requests the ability to make its own job object, that will be allowed. This allows protected mode flash to work, thus fixing the hang.

For more information, check out job objects:
http://msdn.microsoft.com/en-us/library/windows/desktop/ms684161%28v=vs.85%29.aspx

Comment 25

[cmtalbert]

Attachment: A test for this specific issue using mozmill.

The test

Comment 26

[Andrew Halberstadt [:ahal]]

Comment on attachment 641956 [details] [diff] [review]
Patch v1

Review of attachment 641956 [details] [diff] [review]:
-----------------------------------------------------------------

Awesome! Good call on the problem.
One comment, wouldn't it be better to use mozlog and use a DEBUG log level? I guess it might pollute other issues, but it doesn't seem like that many messages will be printed. I'm ok with checking in as is though.

Comment 27

[Andrew Halberstadt [:ahal]]

Comment on attachment 641958 [details] [diff] [review]
A test for this specific issue using mozmill.

Review of attachment 641958 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good! r+

::: mutt/mutt/tests/js/liveweb_tests/flash-protectedmode.js
@@ +14,5 @@
> +/*
> + * Test opening embeded flash content
> + */
> +function testFlashViaEmbedTag() {
> +  dump("Running Test Now!!!\n");

I think you forgot to remove this.

Comment 28

[cmtalbert]

(In reply to Andrew Halberstadt [:ahal] from comment #26)
> Comment on attachment 641956 [details] [diff] [review]
> Patch v1
> 
> Review of attachment 641956 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Awesome! Good call on the problem.
> One comment, wouldn't it be better to use mozlog and use a DEBUG log level?
> I guess it might pollute other issues, but it doesn't seem like that many
> messages will be printed. I'm ok with checking in as is though.


I tend to think of Mozlog being for end-users not for internal mozbase things.  I.e. the people building atop mozbase should use mozlog.  I don't see the benefit of then polluting mozlog messages with low-level stuff like this.  So, gonna leave as is for now.  Thanks for the review.

And I'll remove that dump statement before I check in.  Yeah, I forgot to delete it. Thanks!!!

Comment 29

[cmtalbert]

Landed: https://github.com/mozilla/mozbase/commit/30357c549293ee9e4f10955af32615cd31cb2b2d
and: https://github.com/mozautomation/mozmill/commit/0c7331af47124d7ba82b1ad181449f8d49117a13

--> FIXED

Comment 30

[Henrik Skupin (:whimboo) [⌚️UTC+2] (away 07/24 - 08/10)]

Clint, thanks for the fix! Given the code changes between Mozmill 1.5 and current mozbase, how we should proceed with a patch for Mozmill 1.5? Is it worth backporting or should we leave Flash 11.3 protected mode disabled until we upgrade to Mozmill 2.0 soon.

Comment 31

[Henrik Skupin (:whimboo) [⌚️UTC+2] (away 07/24 - 08/10)]

Tested and works fine with latest mozrunner and Mozilla/5.0 (Windows NT 6.0; rv:13.0) Gecko/20100101 Firefox/13.0.1 with Flash 11.3 installed and protected mode enabled. Lets talk about the backport later in the automation meeting.

Comment 32

[Henrik Skupin (:whimboo) [⌚️UTC+2] (away 07/24 - 08/10)]

We have discussed that in our yesterdays automation developer meeting and as consensus we want to fix this in Mozmill 1.5. It seems to be a simple copy and paste given that in this area the code really hasn't changed.

Comment 33

[cmtalbert]

Attachment: hotfix1.5 backport

Backport for 1.5 branch

Comment 34

[Henrik Skupin (:whimboo) [⌚️UTC+2] (away 07/24 - 08/10)]

Comment on attachment 642958 [details] [diff] [review]
hotfix1.5 backport

I think Ahal is a better reviewer for this code.

Comment 35

[Andrew Halberstadt [:ahal]]

Comment on attachment 642958 [details] [diff] [review]
hotfix1.5 backport

Review of attachment 642958 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, r+

Comment 36

[Henrik Skupin (:whimboo) [⌚️UTC+2] (away 07/24 - 08/10)]

Landed on hotfix-1.5:
https://github.com/mozautomation/mozmill/commit/a3093cc555af4e91d1f30ebadffba5a67f099962