499770 - um.activeUpdate is null in onStopRequest for some edgecases

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect)

Description

[Robert Strong (they/them - no direct email)]

* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "'[JavaScript Error: "um.activeUpdate is null" {file: "file:///c:/moz/_1_mozilla-central/ff-debug/dist/bin/components/nsUpdateService.js" line: 2734}]' when calling method: [nsIRequestObserver::onStopRequest]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "<unknown>"  data: yes]

Comment 1

[Robert Strong (they/them - no direct email)]

Attachment: patch rev1 with tests

Comment 2

[Robert Strong (they/them - no direct email)]

Comment on attachment 384468 [details] [diff] [review]
patch rev1 with tests

By directly calling xpcom-shutdown this tests this in every test

Comment 3

[Robert Strong (they/them - no direct email)]

Attachment: patch as checked in

Comment 4

[Robert Strong (they/them - no direct email)]

Pushed to mozilla-central
http://hg.mozilla.org/mozilla-central/rev/7a8502b70fdf

Comment 5

[Robert Strong (they/them - no direct email)]

Comment on attachment 384689 [details] [diff] [review]
patch as checked in

Drivers, this is a safe, sane, simple fix with tests for an edgecase where the update manager throws during onStopRequest for nsIIncrementalDownload. I'd like to get this for 1.9.1 if there is another RC

Comment 6

[Samuel Sidler (old account; do not CC)]

Won't block on this, but feel free to request approval1.9.1.1 on the patch (though we might punt until 1.9.1.2 given how small this release is intended to be).

Comment 7

[Robert Strong (they/them - no direct email)]

Comment on attachment 384689 [details] [diff] [review]
patch as checked in

Requesting 1.9.1.1 - drivers, this is a safe, sane, simple fix with tests for an edgecase where the update manager throws during onStopRequest for nsIIncrementalDownload. I'd like to get this for 1.9.1.1

Comment 8

[Robert Strong (they/them - no direct email)]

Comment on attachment 384689 [details] [diff] [review]
patch as checked in

Should also get this in for 1.9.0.13

Comment 9

[Daniel Veditz [:dveditz]]

Comment on attachment 384689 [details] [diff] [review]
patch as checked in

Approved for 1.9.0.13, a=dveditz for release-drivers

Comment 10

[Robert Strong (they/them - no direct email)]

Attachment: As checked in for 1.9.0.13

Comment 11

[Robert Strong (they/them - no direct email)]

Checked in for 1.9.0.13 / Firefox 3.0.13

Checking in toolkit/mozapps/update/src/nsUpdateService.js.in;
/cvsroot/mozilla/toolkit/mozapps/update/src/nsUpdateService.js.in,v  <--  nsUpda
teService.js.in
new revision: 1.155; previous revision: 1.154
done
Checking in toolkit/mozapps/update/test/unit/head_update.js;
/cvsroot/mozilla/toolkit/mozapps/update/test/unit/head_update.js,v  <--  head_up
date.js
new revision: 1.3; previous revision: 1.2
done
Checking in toolkit/mozapps/update/test/unit/tail_update.js;
/cvsroot/mozilla/toolkit/mozapps/update/test/unit/tail_update.js,v  <--  tail_up
date.js
new revision: 1.3; previous revision: 1.2
done
Checking in toolkit/mozapps/update/test/unit/test_0010_general.js;
/cvsroot/mozilla/toolkit/mozapps/update/test/unit/test_0010_general.js,v  <--  t
est_0010_general.js
new revision: 1.3; previous revision: 1.2
done
Checking in toolkit/mozapps/update/test/unit/test_0020_general.js;
/cvsroot/mozilla/toolkit/mozapps/update/test/unit/test_0020_general.js,v  <--  t
est_0020_general.js
new revision: 1.3; previous revision: 1.2
done
Checking in toolkit/mozapps/update/test/unit/test_0030_general.js;
/cvsroot/mozilla/toolkit/mozapps/update/test/unit/test_0030_general.js,v  <--  t
est_0030_general.js
new revision: 1.3; previous revision: 1.2
done
Checking in toolkit/mozapps/update/test/unit/test_0040_general.js.in;
/cvsroot/mozilla/toolkit/mozapps/update/test/unit/test_0040_general.js.in,v  <--
  test_0040_general.js.in
new revision: 1.4; previous revision: 1.3
done
Checking in toolkit/mozapps/update/test/unit/test_0060_manager.js;
/cvsroot/mozilla/toolkit/mozapps/update/test/unit/test_0060_manager.js,v  <--  t
est_0060_manager.js
new revision: 1.2; previous revision: 1.1
done
Checking in toolkit/mozapps/update/test/unit/test_0061_manager.js;
/cvsroot/mozilla/toolkit/mozapps/update/test/unit/test_0061_manager.js,v  <--  t
est_0061_manager.js
new revision: 1.2; previous revision: 1.1
done
Checking in toolkit/mozapps/update/test/unit/test_0062_manager.js;
/cvsroot/mozilla/toolkit/mozapps/update/test/unit/test_0062_manager.js,v  <--  t
est_0062_manager.js
new revision: 1.2; previous revision: 1.1
done
Checking in toolkit/mozapps/update/test/unit/test_0110_general.js;
/cvsroot/mozilla/toolkit/mozapps/update/test/unit/test_0110_general.js,v  <--  t
est_0110_general.js
new revision: 1.2; previous revision: 1.1
done
Checking in toolkit/mozapps/update/test/unit/test_0111_general.js;
/cvsroot/mozilla/toolkit/mozapps/update/test/unit/test_0111_general.js,v  <--  t
est_0111_general.js
new revision: 1.2; previous revision: 1.1
done

Comment 12

[Samuel Sidler (old account; do not CC)]

Comment on attachment 384689 [details] [diff] [review]
patch as checked in

Approved for 1.9.1.2. a=ss for release-drivers

Please land on mozilla-1.9.1 and use the ".2-fixed" option of the "status1.9.1" flag.

Comment 13

[Robert Strong (they/them - no direct email)]

Pushed to mozilla-1.9.1
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/8544fcde14a2

Comment 14

[Robert Strong (they/them - no direct email)]

Attachment: As pushed to mozilla-1.9.1

Comment 15

[juan becerra [:juanb]]

Robert, could you help us verify this for 3.5.2, or add a comment to that extent?

Comment 16

[Robert Strong (they/them - no direct email)]

There aren't steps where you can reproduce this but it does have a test. If you like I can show you the change via an xpcshell test with / without the patch but that seems a bit overkill for what the patch does which is add a null check

Comment 17

[Al Billings [:abillings - ex-MoCo]]

Verified for 1.9.0.14 based on tests passing.