Re: Blocking window.print() while loading resources for print

[Daniel Cheng]

Apologies, I was OOO at the time and missed this thread.

I don't really love the idea of deleting ScopedPagePauser. Because there's reentrancy involved in printing, it's really easy to end up in weird states that crash or do unexpected things. ScopedPagePauser was/is a sort of mitigation to this.

If you haven't already done the code archaeology, I can look into it a bit--but it will take awhile, because this is really old code and blaming across the Blink reformats is painful due to a bunch of renames. But I don't feel comfortable just deleting it without understanding its purpose.

FWIW, I'm pretty sure that we had attempted to work around this by deferring window.print() from popping a dialog until a page was loaded: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/frame/local_dom_window.cc;l=1343;drc=566dea16b3e9850406f3f9d70a92b0827ef2573a. Is that not happening here? Or are the print resources simply not blocking the load event?

Daniel

On Wed, 11 Sept 2024 at 14:09, Traian Captan <tca...@google.com> wrote:

CC+ platform-arc...@chromium.org as suggested by Dominic.

On Wed, Sep 11, 2024 at 1:49 PM Traian Captan <tca...@google.com> wrote:

+ Mason and Philip who were asking about this issue, and Scot as FYI

On Wed, Aug 28, 2024 at 3:15 PM Traian Captan <tca...@google.com> wrote:

Hi Daniel,

Dominic thought it might be good to reach out to you regarding a proposed solution for fixing a set of bugs caused by not blocking window.print() while waiting for print resources to load.

These bugs (like bug) came my way as a result of a recent CL I did: "Fix print-only object bitmap-image loading"

The basic assertion of the bugs is that if `window.print()` is called we should block its return until the print dialog is closed.

The way that resources are loaded for printing is via the `WillPrintSoon()` method which kicks off the async loading. This is called from `PrintRenderFrameHelper::RequestPrintPreview`, which checks if we need to wait for loading, and sets up a call back if waiting is necessary, then returns.

Since we return while waiting for the print resources, `window.print()` returns too and does not block.

One thing I tried to fix this issue is to create a new run loop, calling WillPrintSoon as a posted task and hoping that the response will be processed in the inner run loop which successfully blocks the window.print() call. However, while the WillPrintSoon task gets processed in the inner run loop the response does not get processed until we return to the main run loop after the print preview window closes.

This turned out to be because of ScopedPagePauser in chrome_client.cc

After commenting that out we achieve both blocking of the window.print() call and loading of print only resources. Does this seem like a reasonable approach?

If it's problematic, do you have any advice on how to block this window.print() return while still processing the print only resource loads?

Regards,

Traian

[Daniel Cheng]

Sadly, I have traced this to https://source.chromium.org/chromium/chromium/src/+/bc88a25e2d34af239f303872b06189e389e8956f (which introduced it for JS modal dialogs) and https://source.chromium.org/chromium/chromium/src/+/aa0d8720d4687f7efa0fcb543fef9db88849c1a0 (which mentions that print() should similarly defer load completion) and https://source.chromium.org/chromium/chromium/src/+/d989b6dccf16bff20dab49319b98ac345ce98654 (which introduces the guard but doesn't explain any background).

The most useful thing is probably the report in rdar... which I don't think any of us will be able to access...

Daniel

[Dave Tapuska]

You can't remove the ScopedPagePauser that is clear..

"The user agent may wait for the user to either accept or decline before returning; if so, the user agent must pause while the method is waiting"

"While a user agent has a paused task, the corresponding event loop must not run further tasks, and any script in the currently running task must block."

The HTML spec is pretty clear in this regard.

dave.

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architect...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAF3XrKo02_aa80w5D9Jzb_mW7RvND%2BfJn2x7tborTjCjKkRVsA%40mail.gmail.com.

[Dave Tapuska]

Dominic or Mason would know if you are allowed to reenter the loading phase. That is what I would think might be best

.. and then window.print wouldn't block but that probably has it own whole set of problems because I don't think you can do that. 

Dave

On Wed, Sep 11, 2024, 6:12 PM Traian Captan <tca...@google.com> wrote:

Thanks Dave!

I'll leave the `ScopedPagePauser` in.
Do you have any suggestions on how to allow loading of the print only resources with the `ScopedPagePauser` on?

Regards,

Traian

 

[David Baron]

I think there's a tradeoff between two different approaches to the underlying problem here.  We can either:

• try to load print-only resources before printing, so that no network loads are needed when printing (extra bandwidth, less complexity in printing code, maybe a little more complexity in loading code, faster response when the user prints)
• try to defer print-only resources until the user chooses to print (reverse of above tradeoffs)

It sounds like in many cases we follow the "try to load" approach.  (Is that correct?)  I know that Gecko did similar (e.g., for print-only stylesheets and images referenced from print-only stylesheets or print-only parts of stylesheets), but maybe not completely.

I don't think it's useful to follow a mix of the two approaches (particularly at the extreme of loading most but not all print-only resources) -- then we get both sets of complexity and most of the extra bandwidth usage.

So if the spec appears to mandate not loading resources in a particular case, maybe that's a spec bug, and we should try to stick to a consistent model of always loading the print-only resources (although hopefully at a low priority), and try to get the spec fixed?

-David

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAHgVhZW4H2vVq20uMvzwVrCb2Jd%2BOVHBupjJw_1%2BeF3LMzm%2BsA%40mail.gmail.com.

[Philip Rogers]

We cannot load resources for printing ahead of time because they depend on settings that change during printing. For example, media queries based on width will be different in portrait and landscape.

We've received bugs in this area, and new CSS support around @page is designed to enable this, so I don't think these are edge cases. If we can't load resources during print, I think we need to change the spec.

[Dave Tapuska]

You cannot run other things that would cause scripts to run (because you are paused in the execution of a current callgraph. Running arbitrary tasks can end up doing all sorts of things. Would it be best to re-visit window.print() in a new API that wouldn't block?

dave.

[David Baron]

OK, I just retested and my memory of this was wrong -- during initial page load, neither Chrome nor Firefox load images referenced from stylesheets that are currently excluded by media queries.  So my conjecture that we were loading most things but just excluding some edge cases was wrong.

(However, both Chrome and Firefox do load *stylesheets* that are excluded by media queries, which was probably the thing I was remembering.)

-David

On Thu, Sep 12, 2024 at 9:52 AM Philip Rogers <p...@chromium.org> wrote:

[Stefan Zager]

Even if we add a new API, we can't deprecate the old one (usage is too high). So we at least need a coherent solution for the existing problem.

Is there any precedent for suppressing JS load handlers? IIUC, the problem is not with doing the actual load but rather invoking JS.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAHgVhZWEfdjwM7ExonPv2VwX1TmkgKuK8UOtLEKbgv%2BJyvreZQ%40mail.gmail.com.

[Stefan Zager]

I'm sort of imagining a solution where we teach blink::MainThreadSchedulerImpl to service only loading tasks, use a nested RunLoop, and suppress JS load handlers. WDYT?

[Daniel Cheng]

I think the only reliable way to suppress JS load handlers is to disable JS execution altogether. Otherwise, we're just going to get bugs where "x, y and z are blocked but w isn't". Unfortunately, attempting to execute a script when script is disabled often just ends up CHECKing and crashing... which is better than a security bug, but still disruptive for the user.

The other problem is that JS reentrancy is certainly problematic, but C++ reentrancy is also problematic as well. We already have reentrancy in window.print(). "Just" running loading tasks still means C++ reentrancy and dealing with those crashes. Often there's no actual good way to handle the resulting reentrancy, because it other means:

- relying on state that doesn't exist yet

- or reordering events that weren't meant to be reordered

In fact, there's a longstanding unfixed crash where swapping frames happens in the middle of window.print.

Daniel

[Dave Tapuska]

Ya you could try a ScriptForbiddenScope somehow, but yes generally that leads to crashes here when the context is entered (as Daniel said). I'd be most curious what other vendors are doing, has anyone investigated that? 

The loaders should be frame based task runners so it should be possible to mark them as to execute while paused. I do think this is questionable territory though as I've got no idea what any loaders are doing and wouldn't be surprised they call back into some piece of javascript via some prototype chain.

dave.

[Stefan Zager]

Hmm... sorry if I'm going in circles or reopening settled issues because I've forgotten the resolution, but...

According to MDN doc for window.open:

"Opens the print dialog to print the current document.

"If the document is still loading when this function is called, then the document will finish loading before opening the print dialog.

"This method will block while the print dialog is open."

If I'm reading this correctly, then window.open() called during document load will return immediately, and then the print dialog will pop up as a subsequent blocking task. Can we just do the same thing if we need to load print-only resources (which we should be able to determine via synchronous style resolution)?

[Stefan Zager]

Sorry, I meant to say:

If I'm reading this correctly, then window.print() called during document load...

                                                                 ^^^^^

[Daniel Cheng]

Yes, that's correct; I linked to the logic that implements that above.

We could do that, but there are also pages that depend on window.print() to synchronously block, because they immediately call window.close() after that.

Really, it's all a giant mess. :)

Daniel

[Stefan Zager]

I guess the problem is that doing this would be a behavioral change, even if it hews more closely to (a new interpretation of) the spec.

[Stefan Zager]

All things considered, as of this moment my favored solution is:

- Make window.open() return a Promise

- Schedule the print dialog for a subsequent task if we need to load more stuff

- Add a feature flag, reverse origin trial, enterprise policy, etc.

- Make an announcement, ship it, and weather the storm

[Philip Rogers]

On Thu, Sep 12, 2024 at 11:05 AM Stefan Zager <sza...@chromium.org> wrote:

All things considered, as of this moment my favored solution is:

- Make window.open() return a Promise

- Schedule the print dialog for a subsequent task if we need to load more stuff

We can't determine this via a synchronous style resolution because the styles can change dynamically based on the print dialog. For example, you can have css rules that load different images when printing envelopes vs A1.

Would it be feasible to not block at all? Pages do rely on blocking behavior today, but they could be migrated to use the afterprint event.

[Daniel Cheng]

Yes, I think a promise-based API would be ideal. Hopefully the fact that it would be less randomly busted would encourage people to use it.

Can we really change the existing API though, or would we need to add a new one?

Daniel

[Stefan Zager]

Current API has no return value, so I think it would be OK to add one (I'm pretty sure I've seen this done before)

[Daniel Cheng]

"Fixing the crashes" means writing fixes for situations that should never happen. There will not be viable fixes for many of these types of issues, because you either end up:

- just dropping things, which might break other things that were expecting those tasks to not be dropped

- reordering things, which again, breaks things

I do not think removing the page pauser and forbidding script is a viable approach.

Daniel

On Wed, 18 Sept 2024 at 14:03, Lei Zhang <the...@google.com> wrote:

It's been a while, but a few years ago I tried to change the printing code to remove that ScopedPagePauser and have PrintRenderFrameHelper take control, but that lead to https://crbug.com/40146790 and I had to revert. Not sure how much has changed since then, and if letting PrintRenderFrameHelper control pausing helps with the issue at hand.

On Wed, Sep 18, 2024 at 1:43 PM Traian Captan <tca...@google.com> wrote:

Hi All,

Regarding what other vendors are doing, Safari blocks the `window.print()` call but does not load print only resources in time.

Firefox is managing to load print only resources while blocking the current JS API `window.print()` call. This is most likely because they clone documents for printing.

Currently Chrome loads print only resources but does not block `window.print()` if loading is occurring.

I think it would be great if we could achieve something similar to Firefox for the current API, where we load these print only resources and block the `window.print()` at the same time.

Instead of removing the `ScopedPagePauser` in `ChromeClient::Print`, how about we switch it to a `ScriptForbiddenScope` and fix the crashes in `BeforeCallEnteredCallback`?

Regards,

Traian

On Thu, Sep 12, 2024 at 7:13 PM Traian Captan <tca...@google.com> wrote:

If we add a new promise-based API, what should we do for the old one? Not load print only resources in order to block, trading off rendering correctness?

[Dave Tapuska]

I agree with Daniel here. I think this is going to be a challenging uphill battle. I think the best case is to design an API that works and experiment with it. Do we have a major partner that is complaining of usage? Or is it just end users? It may be simple for developers to switch if they were executing the code in an async event listener already.

dave.

[Morten Stenshorne]

Traian Captan <tca...@google.com> writes:

> Firefox is managing to load print only resources while blocking the
> current JS API `window.print()` call. This is most likely because they
> clone documents for printing.

Aside: Cloning documents for printing is on my birthday present wishlist
every year. :)

Is this something we're considering doing?

--
Morten Stenshorne, Software developer,
Blink/Layout, Google, Oslo, Norway

[Traian Captan]

Hi Morten,

On Thu, Sep 19, 2024 at 1:05 AM Morten Stenshorne <mste...@chromium.org> wrote:

Aside: Cloning documents for printing is on my birthday present wishlist
every year. :)

Is this something we're considering doing?

 As far as I know, we're not considering this at the moment.

Regards,

Traian