1045209 - The OpenH264 path should be relative to the profile directory and include a version subdirectory

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Johannes Pfrang [:johnp]]

After moving my whole system to an SSD which previously only contained firefox in D:\Program Files\Nightly\ with the profile symlinked from %appdata%/... to D:\Program Files\Nightly\profile, the media.gmp-gmpopenh264.path config entry is now still at at D:\Program Files\Nightly\profile\gmp-gmpopenh264, altough firefox and it's profile are now in their default locations on C:.

Shouldn't that path be relative to the profile path and if not, isn't a hard-coded path were a .dll is loaded from and that can probably be changed by another application a security issue? Could the path be changed to point to a malicious .dll?

Comment 1

[Georg Fritzsche [:gfritzsche]]

Ok, that's a shortcoming in our solution - relative to profile dir sounds workable. But given that moving profiles around is not common it should not block.

I don't see a security bug here though - if you can change the pref and drop binaries on the disk, you already have access.
Apparently i can't unset the sec flag though.

Comment 2

[Johannes Pfrang [:johnp]]

(In reply to Georg Fritzsche [:gfritzsche] from comment #1)
Sounds reasonable. Just wanted to make sure with all those DLL hijacking issues over the course of the last years that this couldn't be used maliciously.

Comment 3

[Maire Reavy [:mreavy]]

Georg -- I think this is in the Add-ons manager.  Can you take this bug or find someone to take it?

Comment 4

[Georg Fritzsche [:gfritzsche]]

I put it on our Firefox backlog, so it should get picked up at some point. The question is whether we can live with it for now or not (and the prioritization over e.g. my other tasks).

Comment 5

[Maire Reavy [:mreavy]]

I think the priority depends on how likely it is for the average user to experience this problem.  Based on the bug description and comments, it seems pretty unlikely. So we can probably make this non-blocking.  Do you agree?  (Does anyone disagree?)

Comment 6

[Georg Fritzsche [:gfritzsche]]

(In reply to Maire Reavy [:mreavy] (Plz needinfo me) from comment #5)
> Based on the bug description and comments, it
> seems pretty unlikely. So we can probably make this non-blocking.  Do you
> agree?

I agree.

Comment 7

[Georg Fritzsche [:gfritzsche]]

Talking with Benjamin this is more likely then we assumed - e.g. enterprise setups with roaming profiles would be affected (e.g. when mounted to different drives).

Comment 8

[Georg Fritzsche [:gfritzsche]]

Marco, we need this in 33 - can we get this bumped up into the next iteration?

Comment 9

[Marco Mucci [:MarcoM]]

Forwarding 'needinfo' to Gavin for considering this bug in the upcoming Iteration 34.2

(In reply to Georg Fritzsche [:gfritzsche] from comment #8)
> Marco, we need this in 33 - can we get this bumped up into the next
> iteration?

Comment 10

[:Gavin Sharp [email: [e-mail]]]

Let's add to the 34.2 backlog.

Comment 11

[Marco Mucci [:MarcoM]]

Added to 34.2 Backlog.

(In reply to :Gavin Sharp [email: [e-mail]] from comment #10)
> Let's add to the 34.2 backlog.

Comment 12

[Marco Mucci [:MarcoM]]

Hi Georg, can you provide a point value for this bug.

Comment 13

[Georg Fritzsche [:gfritzsche]]

bbondy, is this something you could potentially look into?

Comment 14

[Georg Fritzsche [:gfritzsche]]

We would have to set a path relative to the profile here:
http://hg.mozilla.org/mozilla-central/annotate/71497ed2e0db/toolkit/modules/GMPInstallManager.jsm#l885

... and expand it from |Services.dirsvc.get('ProfD', Ci.nsIFile)| everywhere OPENH264_PREF_PATH gets used in OpenH264Provider.jsm.
http://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/internal/OpenH264Provider.jsm

Comment 15

[Maire Reavy [:mreavy]]

Adding uplift to the whiteboard based on Comment 7.

Comment 16

[Benjamin Smedberg]

I'm not convinced that we need to use a path for this at all. We already know that it's going to be in <profile>/..../<version> right? If so can we just store the current version in prefs?

i.e. s/media.gmp-gmpopenh264.path/media.gmp-gmpopenh264.currentVersion/

Comment 17

[Georg Fritzsche [:gfritzsche]]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #16)
> I'm not convinced that we need to use a path for this at all. We already
> know that it's going to be in <profile>/..../<version> right?

We already store the version ("media.gmp-gmpopenh264.version"), but use <profile>/<gmp-id>.
That sounds sensible though and we could easily switch to installing the GMP into a version subdir.

Comment 18

[Brian R. Bondy [:bbondy]]

I think we can just use  <profile>/<gmp-id> and assume if it exists it'll be the latest no?

Comment 19

[Georg Fritzsche [:gfritzsche]]

I think that we can actually run into "update while in use issues" without this, right?
This would leave the "remove older versions" issue, but we could look into it in a follow-up.

Comment 20

[Brian R. Bondy [:bbondy]]

Maybe on Windows, we could probably rename out of the way before upgrading though. And delete any old renamed file if one exists.  Not sure if that's more simple than the version thing though.

I'll probably just do the version dir thing and at the same time I'd check what the old version is and delete it if it exists.  I'll handle both in this bug.

Comment 21

[Brian R. Bondy [:bbondy]]

Oops, thanks for fixing.

Comment 22

[Georg Fritzsche [:gfritzsche]]

* We won't need the media.gmp-gmpopenh264.path preference anymore (affects GMPInstallManager & OpenH264Provider)
* OpenH264Provider should listen to changes to the media.gmp-gmpopenh264.version pref instead
* the plugin will be installed into <profile-dir>/<gmp-plugin-id>/<version>
* some tests using the path property will need fixup (probably toolkit/mozapps/extensions/test/browser/browser_openH264.js & .../xpcshell/test_openH264.js)

Helpful:
* OS.Path.join(OS.Constants.Path.profileDir, ...)
* install path after download: http://hg.mozilla.org/mozilla-central/annotate/71497ed2e0db/toolkit/modules/GMPInstallManager.jsm#l872

Comment 23

[Georg Fritzsche [:gfritzsche]]

[Tracking Requested - why for this release]:

Comment 24

[Sylvestre Ledru [:Sylvestre]]

Tracking but we won't block the release for this.

Comment 25

[Qeole]

Attachment: Changing_install_dir_and_remove_path-preference.patch

(In reply to Georg Fritzsche [:gfritzsche] from comment #22)
> * We won't need the media.gmp-gmpopenh264.path preference anymore (affects
> GMPInstallManager & OpenH264Provider)
Also affects GMPParent.cpp [1] and GMPChild.cpp [2], in which we rely on plugin path to extract plugin name.
> * OpenH264Provider should listen to changes to the
> media.gmp-gmpopenh264.version pref instead
> * the plugin will be installed into <profile-dir>/<gmp-plugin-id>/<version>
I've addressed the three points above in enclosed patch.
|media.gmp-gmpopenh264.path| preference is gone, path is <profile-dir>/<gmp-plugin-id>/<version>, and plugin still loads correctly.

> * some tests using the path property will need fixup (probably
> toolkit/mozapps/extensions/test/browser/browser_openH264.js &
> .../xpcshell/test_openH264.js)
Tests will be for next week, assuming feedback is positive.

[1] https://hg.mozilla.org/mozilla-central/annotate/6a7be7376caa/content/media/gmp/GMPParent.cpp#l100
[2] https://hg.mozilla.org/mozilla-central/annotate/6a7be7376caa/content/media/gmp/GMPChild.cpp#l66

Comment 26

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8472750 [details] [diff] [review]
Changing_install_dir_and_remove_path-preference.patch

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

Thanks, that looks good!
I wasn't aware of the GMPChild/GMPParent dependencies here - that seems unfortunate when they could just receive a path instead of re-implementing the same logic - i'll file a new bug on that.

::: toolkit/mozapps/extensions/internal/OpenH264Provider.jsm
@@ +218,5 @@
>    get pluginMimeTypes() { return []; },
>    get pluginLibraries() {
> +    if (this.isInstalled) {
> +      let path = this.version;
> +      return [path];

This will probably look a little awkward in about:plugins, but we're not exposing this in "normal" UI anywhere, so i don't think that's a real concern now.

@@ +300,5 @@
>      let wrapper = OpenH264Wrapper;
>  
>      AddonManagerPrivate.callAddonListeners("onUninstalling", wrapper, false);
>      if (this.gmpPath) {
> +      this._log.info("onPrefVersionChanged() - removing gmp directory " + this.gmpPath);

Can we change this to "unregistering gmp directory" while we're here?
Reading through this i realize that easy to misunderstand.

@@ +316,2 @@
>      if (this.gmpPath && wrapper.isActive) {
> +      this._log.info("onPrefVersionChanged() - adding gmp directory " + this.gmpPath);

"registering gmp directory"?

Comment 27

[Georg Fritzsche [:gfritzsche]]

(In reply to Georg Fritzsche [:gfritzsche] from comment #26)
> I wasn't aware of the GMPChild/GMPParent dependencies here - that seems
> unfortunate when they could just receive a path instead of re-implementing
> the same logic - i'll file a new bug on that.

I meant "receive the plugin id".

Comment 28

[Qeole]

(In reply to Georg Fritzsche [:gfritzsche] from comment #26)
> I wasn't aware of the GMPChild/GMPParent dependencies here - that seems
> unfortunate when they could just receive a path instead of re-implementing
> the same logic - i'll file a new bug on that.

I saw you filled bug 1053727. Patch from bug 1040905 (rebased and waiting for review) will modify same portion of code, should it be worth setting it blocking for bug 1053727?
 
> ::: toolkit/mozapps/extensions/internal/OpenH264Provider.jsm
> @@ +218,5 @@
> >    get pluginMimeTypes() { return []; },
> >    get pluginLibraries() {
> > +    if (this.isInstalled) {
> > +      let path = this.version;
> > +      return [path];
> 
> This will probably look a little awkward in about:plugins, but we're not
> exposing this in "normal" UI anywhere, so i don't think that's a real
> concern now.

What would you suggest instead?

> […]
> Can we change this to…

Will do.

Comment 29

[Georg Fritzsche [:gfritzsche]]

(In reply to qeole from comment #28)
> I saw you filled bug 1053727. Patch from bug 1040905 (rebased and waiting
> for review) will modify same portion of code, should it be worth setting it
> blocking for bug 1053727?

Right, done.

> > ::: toolkit/mozapps/extensions/internal/OpenH264Provider.jsm
> > @@ +218,5 @@
> > >    get pluginMimeTypes() { return []; },
> > >    get pluginLibraries() {
> > > +    if (this.isInstalled) {
> > > +      let path = this.version;
> > > +      return [path];
> > 
> > This will probably look a little awkward in about:plugins, but we're not
> > exposing this in "normal" UI anywhere, so i don't think that's a real
> > concern now.
> 
> What would you suggest instead?

We could do something like [0] here, but given that this only seen in about:plugins for OpenH264 let's leave it for now.

Comment 30

[Qeole]

Attachment: Changing_install_dir_and_remove_path-preference.patch

Updating patch with modified comments as suggested by Georg in comment #26.
Asking review for said patch.

Comment 31

[Qeole]

Attachment: Tests.patch

Here is a separate patch updating the tests (toolkit/mozapps/extensions/test/browser/browser_openH264.js and toolkit/mozapps/extensions/test/xpcshell/test_openh264.js).

* browser_openH264.js
I had to correct some wrong |Services.prefs.setBoolPref()| calls to |Services.prefs.setCharPref()| for setting version and last update time.

* test_openh264.js
There would be some test to add regarding version handling for plugin update once this is implemented in plugin management (currently we don't check that newly registered plugin has higher version than former one). See FIXME tag in the patch. I believe it could be relevant to make those tests part of bug 1053729.

Both test files passed successfully on my computer.

Comment 32

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8474490 [details] [diff] [review]
Changing_install_dir_and_remove_path-preference.patch

r?jesup for GMPChild.cpp, GMPParent.cpp
r?unfocused for GMPInstallManager.jsm, OpenH264Provider.jsm

Comment 33

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8474494 [details] [diff] [review]
Tests.patch

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

This looks fine, with the below comment addressed you should go forward and push to try and request review from Unfocused (Blair).

::: toolkit/mozapps/extensions/test/xpcshell/test_openh264.js
@@ +216,5 @@
>    Assert.equal(removedPath, null);
>  
>    // Changing the pref mid-session should cause unregistration and registration.
> +  // FIXME: Need a test to verify that nothing happens if registered version number
> +  // is higher than unregistered plugin version number. See also bug 1053729.

I don't think we need to assert this here and can just remove the FIXME comment.
If our update/install handling picks something, we should accept it and not worry about version ordering.
If we don't have something like this, we could add checks for GMPInstallManager or the providers update picking in another bug.

Comment 34

[Qeole]

Attachment: Tests.patch

Updating patch related to tests.

There were some tests we had forgotten, here is the list of files now affected by the patch:

dom/media/gmp-plugin/Makefile.in
testing/mochitest/runtests.py
toolkit/modules/tests/xpcshell/test_GMPInstallManager.js
toolkit/mozapps/extensions/test/browser/browser_openH264.js
toolkit/mozapps/extensions/test/xpcshell/test_openh264.js

I've just made a push to try:
https://tbpl.mozilla.org/?tree=Try&rev=5b764964f42a

Comment 35

[Qeole]

I'm having issues with my push to try (https://tbpl.mozilla.org/?tree=Try&rev=5b764964f42a).
There's an error message preceded by “INFO -  Could not find path to gmp-fake plugin!”.

I think this message comes from runtests.py file, but I can't understand why we have it. There may be a place where I forgot to update plugin path, or I may have done something wrong in runtests.py? Or something else?

jesup, do you have any idea about that, please? Or about a way I could obtain more info?

Comment 36

[Randell Jesup [:jesup] (needinfo me)]

ted did the runtests.py/gmp stuff - forwarding needinfo

Comment 37

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8474490 [details] [diff] [review]
Changing_install_dir_and_remove_path-preference.patch

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

runtests.py and some other stuff will need to change...  r+ for the two files; r- for not dealing with the test automation  (r+ with a test patch that ted r+'d)

Comment 38

[Qeole]

Attachment: Tests.patch

Thank you jesup for review and needinfo forwarding.

(In reply to Randell Jesup [:jesup] from comment #37)

> runtests.py and some other stuff will need to change...
What do you mean exactly?

The error causing (most of) those oranges was due to fake plugin install commands in testing/mochitest/Makefile.in, and I fixed it (but I can't clear the needinfo request for Ted).

I ran another push to try but still have an issue on test bc3 for Windows, I'm investigating.
https://tbpl.mozilla.org/?tree=Try&rev=16a55783d0c6

Comment 39

[Georg Fritzsche [:gfritzsche]]

(In reply to qeole from comment #38)
> (In reply to Randell Jesup [:jesup] from comment #37)
> 
> > runtests.py and some other stuff will need to change...
> What do you mean exactly?

I think jesup missed that the test changes are in a separate patch.

Comment 40

[Qeole]

I need some help with the oranges from last try push
(https://tbpl.mozilla.org/?tree=Try&rev=16a55783d0c6).

There's something happening for bc3 tests on Windows and I can't figure out what it is exactly.
There is probably something wrong happenig around [1].

It seems to me that there is a call to a logging function with "Too few arguments"; in particular, I added debug prints in [2], and could observe that |message| is a dictionary, and that it lacks a 'subtest' key entry (and associated value) for the line which seems to break the test. I'm not 100% sure it comes from there, but that's the impression I got. I have no idea how my patches could affect this 'subset' entry, though.

Would anyone have a hint here? jmaher, maybe?

[1] http://hg.mozilla.org/mozilla-central/file/4d94eeca89f3/toolkit/mozapps/extensions/test/browser/browser_openH264.js#l131
[2] http://hg.mozilla.org/mozilla-central/file/4d94eeca89f3/testing/mochitest/runtests.py#l1863

Comment 41

[Joel Maher ( :jmaher ) (UTC -8)]

Chris, there is an error https://tbpl.mozilla.org/?tree=Try&rev=16a55783d0c6 that looks like this:
20:17:31     INFO -  4981 INFO TEST-PASS | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/browser_openH264.js | Got add-on element. - [object XULElement] == true
20:17:31     INFO -  Exception in thread Thread-24:
20:17:31     INFO -  Traceback (most recent call last):
20:17:31     INFO -    File "c:\mozilla-build\python27\Lib\threading.py", line 551, in __bootstrap_inner
20:17:31     INFO -      self.run()
20:17:31     INFO -    File "c:\mozilla-build\python27\Lib\threading.py", line 504, in run
20:17:31     INFO -      self.__target(*self.__args, **self.__kwargs)
20:17:31     INFO -    File "C:\slave\test\build\venv\lib\site-packages\mozprocess\processhandler.py", line 775, in _processOutput
20:17:31     INFO -      self.processOutputLine(line.rstrip())
20:17:31     INFO -    File "C:\slave\test\build\venv\lib\site-packages\mozprocess\processhandler.py", line 720, in processOutputLine
20:17:31     INFO -      handler(line)
20:17:31     INFO -    File "C:\slave\test\build\tests\mochitest/runtests.py", line 1858, in processOutputLine
20:17:31     INFO -      self.harness.message_logger.process_message(message)
20:17:31     INFO -    File "C:\slave\test\build\tests\mochitest/runtests.py", line 178, in process_message
20:17:31     INFO -      self.dump_buffered(limit=True)
20:17:31     INFO -    File "C:\slave\test\build\tests\mochitest/runtests.py", line 202, in dump_buffered
20:17:31     INFO -      self.logger.log_raw(buf_msg)
20:17:31     INFO -    File "C:\slave\test\build\venv\lib\site-packages\mozlog\structured\structuredlog.py", line 142, in log_raw
20:17:31     INFO -      converted_data = convertor_registry[action].convert_known(**raw_data)
20:17:31     INFO -    File "C:\slave\test\build\venv\lib\site-packages\mozlog\structured\logtypes.py", line 96, in convert_known
20:17:31     INFO -      return self.convert(**known_kwargs)
20:17:31     INFO -    File "C:\slave\test\build\venv\lib\site-packages\mozlog\structured\logtypes.py", line 61, in convert
20:17:31     INFO -      raise TypeError("Too few arguments")
20:17:31     INFO -  TypeError: Too few arguments

Is there a chance that we now require a certain format for debug messages?

Comment 42

[Qeole]

Attachment: Tests.patch

Well, it works now.
Thank you jmaher, and sorry for the bother (and once again, I can't clear NR for Chris).

I fixed a minor error I had made (I had turned |Services.prefs.setCharPref()| for |OPENH264_PREF_LASTUPDATE| to |setCharPrefs()| instead of |setIntPref()|), but what seemed to fix the above problem was mostly a source update.

Anyway, here is a successful try push:
https://tbpl.mozilla.org/?tree=Try&rev=0e75ec00060f

I'm now waiting for patch from bug 1040905 to be checked in to ask review here, as I'll most probably have to rebase code for GMPChild.cpp.

Comment 43

[Joel Maher ( :jmaher ) (UTC -8)]

glad to see it is working now.

Comment 44

[Qeole]

Comment on attachment 8474490 [details] [diff] [review]
Changing_install_dir_and_remove_path-preference.patch

Following Georg's suggestion on IRC, I'm asking review now :)

First patch (path handling):
r?jesup       content/media/gmp/GMPChild.cpp
r?jesup       content/media/gmp/GMPParent.cpp
r?unfocused   toolkit/modules/GMPInstallManager.jsm
r?unfocused   toolkit/mozapps/extensions/internal/OpenH264Provider.jsm

Second patch (tests):
r?jesup       dom/media/gmp-plugin/Makefile.in
r?jesup       testing/mochitest/Makefile.in
r?jmaher      testing/mochitest/runtests.py
r?unfocused   toolkit/modules/tests/xpcshell/test_GMPInstallManager.js
r?unfocused   toolkit/mozapps/extensions/test/browser/browser_openH264.js
r?unfocused   toolkit/mozapps/extensions/test/xpcshell/test_openh264.js

Comment 45

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8475819 [details] [diff] [review]
Tests.patch

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

Forwarding build-system review portion to ted

Comment 46

[Qeole]

Attachment: Changing_install_dir_and_remove_path-preference.patch

Patch for bug 1040905 has just been checked-in.
Rebasing first patch because of GMPChild.cpp file.

Comment 47

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8475819 [details] [diff] [review]
Tests.patch

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

nothing looks concerning here, thanks for putting this together!

Comment 48

[Qeole]

Please note that I'm leaving today for two weeks or so. I won't be able to follow up or handle potential issues arising here before next uplift (September 1st).

Comment 49

[Benjamin Smedberg]

Qeole, thank you for your help! Georg and I along with the media team will look after this and any regressions.

Comment 50

[(not currently active) Ted Mielczarek]

Comment on attachment 8475819 [details] [diff] [review]
Tests.patch

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

::: testing/mochitest/runtests.py
@@ +1162,5 @@
>      return manifest
>  
>    def getGMPPluginPath(self, options):
>      # For local builds, gmp-fake will be under dist/bin.
> +    gmp_path = os.path.join(options.xrePath, 'gmp-fake', '1.0')

n.b.: this will conflict with bug 1057328, and you'll have to fix the code path added there as well.

Comment 51

[Marco Mucci [:MarcoM]]

Hi Florin, can a QA contact be assigned for this bug.

Comment 52

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 8476254 [details] [diff] [review]
Changing_install_dir_and_remove_path-preference.patch

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

::: toolkit/modules/GMPInstallManager.jsm
@@ +899,5 @@
>          // Success, set the prefs
>          let now = Math.round(Date.now() / 1000);
>          GMPPrefs.set(GMPPrefs.KEY_ADDON_LAST_UPDATE, now, gmpAddon.id);
> +        // Setting the version pref signals installation completion to consumers,
> +        // so set potential other information they use first.

Nit: This is a bit confusing now there isn't any "other information" we're setting. Clarify?

::: toolkit/mozapps/extensions/internal/OpenH264Provider.jsm
@@ +225,2 @@
>    },
>    get pluginFullpath() {

You're not regressing this.... but I just realised both pluginFullpath and pluginLibraries are wrong, as they only point to the directory, not the file. Could you file a separate bug for this? (only need one)

Comment 53

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 8475819 [details] [diff] [review]
Tests.patch

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

r+ with a couple of small fixes.

::: toolkit/modules/tests/xpcshell/test_GMPInstallManager.js
@@ -446,5 @@
>        // Make sure the prefs are set correctly
>        do_check_true(!!GMPPrefs.get(GMPPrefs.KEY_ADDON_LAST_UPDATE,
>                                     gmpAddon.id, ""));
> -      do_check_eq(GMPPrefs.get(GMPPrefs.KEY_ADDON_PATH, gmpAddon.id, ""),
> -                               extractedFile.parent.path);

By removing this, the test is no longer checking that the extracted file was placed where we expect it. Should add a check for extractedFile.path

@@ +447,5 @@
>        // Make sure it reports as being installed
>        do_check_true(gmpAddon.isInstalled);
>  
>        // Cleanup
>        extractedFile.parent.remove(true);

Need to update this too.

Comment 54

[Georg Fritzsche [:gfritzsche]]

(In reply to Blair McBride [:Unfocused] from comment #52)
> You're not regressing this.... but I just realised both pluginFullpath and
> pluginLibraries are wrong, as they only point to the directory, not the
> file. Could you file a separate bug for this? (only need one)

Right, i mentioned this above. I filed bug 1058042 now.

Comment 55

[Georg Fritzsche [:gfritzsche]]

https://hg.mozilla.org/integration/fx-team/rev/8058658b0a28
https://hg.mozilla.org/integration/fx-team/rev/7c725f22a307

Comment 56

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8058658b0a28
https://hg.mozilla.org/mozilla-central/rev/7c725f22a307

Comment 57

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Setting correct status flag.

Comment 58

[Georg Fritzsche [:gfritzsche]]

How is the verification going?
We want this uplifted, but should get it verified first.

Comment 59

[Ada [:adalucinet]]

Reproduced with Nightly 2014-07-30
Verified as fixed with latest Nightly (Build ID: 20140828030205) on Windows 7 x64, Mac OS X 10.9.4 and Ubuntu 12.04 x32: after moving the profile, the plugin is displayed as installed and a h264 call is successfully performed.
No new issues found after performing some additional exploratory testing - all the logs are correctly generated; after resetting media.gmp-manager.lastCheck, a new addon check is correctly performed; crash reports are correctly generated.

Comment 60

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8476254 [details] [diff] [review]
Changing_install_dir_and_remove_path-preference.patch

Approval Request Comment
[Feature/regressing bug #]: OpenH264 integration
[User impact if declined]: non-working OpenH264 after moving profile, e.g. in enterprise scenarios
[Describe test coverage new/current, TBPL]: we have mochitest & xpcshell coverage for the code involved, qa verified
[Risks and why]: lowish - it only touches path handling & pref handling in some well-understood locations and it is qa verified
[String/UUID change made/needed]: none

Comment 61

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8475819 [details] [diff] [review]
Tests.patch

Approval Request Comment
These are the required test changes for the above.

Comment 62

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/93fa2a38c5d5

Comment 63

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ec5f67c9d712

Comment 64

[Ada [:adalucinet]]

Verified as fixed with latest Aurora 33.0a2 (Build ID: 20140901004002) on Windows 7 64-bit, Mac OS X 10.9.4 and Ubuntu 14.04 32-bit.