1193903 - contentAreaUtils saveDocument/saveBrowser doesn't work with Print Preview documents

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

As reported in bug 1101100 comment #41, the changes to e10s-enable nsWebBrowserPersist broke saving Print Preview documents.

Specifically: the Print Preview document doesn't have a page descriptor, and the contentAreaUtils code for getting the postdata and cache key would catch and ignore the exception from that, but the C++ version added in bug 1101100 does not.  So even in the non-e10s case, passing a Print Preview document (or similar) to |saveDocument| would throw an exception when it reads the |cacheKey| or |postData| properties.

The e10s case is worse, because WebBrowserPersistDocumentChild reads all the attributes on actor creation to eagerly send them up to the parent (to avoid sync IPC in the getters), so startPersistence will fail and we don't even get to |saveDocument|.  And |saveBrowser| doesn't give an onError callback to startPersistence, because I thought there was nothing useful I could do there, which isn't quite right — there isn't too much useful information at that point, besides a too-generic nsresult, but throwing a Components.Exception would at least make it obvious that *something* failed.

Comment 1

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

Attachment: Part 1: Improve error reporting for out-of-process saveBrowser().

(These patches don't need to be in order, but this part is easier to test if there's still a bug to cause things to fail.)

Comment 2

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

Attachment: Part 2: Fix nsWebBrowserPersist.

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8647281 [details] [diff] [review]
Part 1: Improve error reporting for out-of-process saveBrowser().

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

::: toolkit/content/contentAreaUtils.js
@@ +141,5 @@
>    persistable.startPersistence({
>      onDocumentReady: function (document) {
>        saveDocument(document, aSkipPrompt);
> +    },
> +    onError: function (status) {

Ah heh - I think we're going to bitrot each other. :) If you beat me to landing, I'll just go with your approach here - seems more robust.

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8647282 [details] [diff] [review]
Part 2: Fix nsWebBrowserPersist.

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

Thanks jld!

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6c4e80bdc918
https://hg.mozilla.org/integration/mozilla-inbound/rev/4ac646590af1

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6c4e80bdc918
https://hg.mozilla.org/mozilla-central/rev/4ac646590af1

Comment 7

[dw-dev]

As promised, I have re-tested my Print Edit add-on with Nightly 43.0a1 (2015-08-15) (which includes this patch) in e10s and non-e10s windows.  Saving the print preview DOM works fine for both 'Web Page, complete' and 'Web Page, HTML only'.  Great!