1015621 - tab.getThumbnail() crashes the child process with e10s enabled

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect, P1)

Description

[Tomislav Jovanovic :zombie]

the proper fix for this is to implement an async API for thumbnails (bug 791098), so i'm thinking of just neutering the method when e10s is enabled, probably by returning a transparent PNG like the method already does on Fennec.

Comment 1

[Tomislav Jovanovic :zombie]

Attachment: Link to Github pull-request: https://github.com/mozilla/addon-sdk/pull/1496/files

and i did that. no new tests, as this is to fix existing ones when run with --e10s.

Comment 2

[Jordan Santell [:jsantell] [@jsantell]]

Comment on attachment 8428385 [details]
Link to Github pull-request: https://github.com/mozilla/addon-sdk/pull/1496/files

Some quick fixes and organization stuff in there -- I'd run by the e10s directory name with Gozala quick (I'm sure it's fine).

Agreed that the real fix for this is an async method that gets it. We should probably deprecate sync fetching of this as well in the future!

Comment 3

[Chris Peterson [:cpeterson]]

Zombie: can you still reproduce this child process crash?

Comment 4

[Tomislav Jovanovic :zombie]

yes, it still crashes reliably.. 

the exact thing that does it is accessing (reading) window.screen in an onReady event, where `window` is a CPOW to a contentWindow of a tab.

i can file a separate platform bug if you want, and try to create a minimal STR addon without the plethora of jetpack abstractions if that would be easier to debug?

Comment 5

[Chris Peterson [:cpeterson]]

Is there an existing Jetpack addon that reproduces the crash? Linking to or attaching it to this bug would help. As long as some addon can reliably crash the content process, I don't think you need to create a non-Jetpack addon.

Comment 6

[Tomislav Jovanovic :zombie]

the jetpack test that does it is:
https://github.com/mozilla/addon-sdk/blob/master/test/tabs/test-firefox-tabs.js#L207-L254

you can run it with this command line:

  cfx testpkgs -v -f tab:testTabPropertiesInSameWindow -b \path\to\nightly\firefox.exe

or alternatively, here is the reduced code that can be run in the (browser) scratchpad:

  let mm = Cc['@mozilla.org/appshell/window-mediator;1']
      .getService(Ci.nsIWindowMediator)
      .getMostRecentWindow('navigator:browser')
      .gBrowser.addTab('data:text/html,thumb')
      .linkedBrowser.messageManager;

  mm.addMessageListener(7, function({ target }) {
    target.contentWindow.screen;  // crash
  })
  mm.loadFrameScript('data:,sendAsyncMessage(7)', true);

Comment 7

[[github robot]]

Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/cf98a06dda09c630e0f873132604a8fa2792e8c0
bug 1015621 - return blank thumbnail in e10s

https://github.com/mozilla/addon-sdk/commit/58e3ca5a0120a60ffec3bd441714d29ed08310bb
Merge pull request #1496 from zombie/1015621-thumb-crash

bug 1015621 - getThumbnail crashes child process, r=@jsantell