299681 - delay onload without dumping the page image to disk

Status: RESOLVED FIXED

(Core :: Web Painting, enhancement)

Description

[Christian Persch (GNOME) (away; not receiving bug mail)]

With the patch from bug 285038, the env variable MOZ_FORCE_PAINT_AFTER_UNLOAD
acquired a double meaning: on the one hand it's used to wait with onload until
background image is loaded,
http://lxr.mozilla.org/seamonkey/source/layout/style/nsCSSValue.cpp#380 :
380   // If Paint Forcing is enabled, then force all background image loads to
381   // complete before firing onload for the document.  Otherwise, background
382   // image loads are special and don't block onload.
383   static PRBool bg_in_bg = !PR_GetEnv("MOZ_FORCE_PAINT_AFTER_ONLOAD");

and on the other hand it's used in layout/base/nsDocumentViewer.cpp to dump the
page as a PPM image to a filename derived from the content of that env variable.

In my application, I want to force onload to wait until bgimg has loaded, but I
*do*not* want to have the image dumped (I'm doing that myself, differently).

Can those 2 meanings please be separated? I'd prefer a pref for the onload part,
myself. If you guide me on the preferred way to resolve this, I can produce a patch.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

> I'd prefer a pref for the onload part, myself.

Sounds good to me. I'd love to see a patch for that :-)

Comment 2

[Christian Persch (GNOME) (away; not receiving bug mail)]

Attachment: proposal

Well, that was easy enough.
Note that nsContentUtils::GetBoolPref defaults to PR_FALSE if the pref doesn't
exist.

Things I'm unsure of:
- should I make it static PRBool, so that it's only checked once; is continued
pref checking going to be a perf problem here?
- should the env variable checking be kept?
- I'll change the pref name to whatever you want

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I would just take out the environment variable dependency here. And I'd make the
pref-getting conditional. Something like

PRInt32 loadFlag = (PRInt32)nsIRequest::LOAD_NORMAL;
if (aIsBGImage) {
  PRBool onloadAfterBackgroundImageLoads =
nsContentUtils::GetBoolPref("layout.onload_after_image_background_loads");
  if (!onloadAfterBackgroundImageLoads) {
    loadFlag = (PRInt32)nsIRequest::LOAD_BACKGROUND;
  }
}

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #2)
> Things I'm unsure of:
> - should I make it static PRBool, so that it's only checked once; is continued
> pref checking going to be a perf problem here?

Yeah, make it static

> - I'll change the pref name to whatever you want

Hmm. layout.fire_onload_after_image_background_loads?

Comment 5

[Christian Persch (GNOME) (away; not receiving bug mail)]

Attachment: implement pref for onload-after-bg-loads

Updated patch taking into account comment 3 and comment 4: Remove env var
checking, check the pref only once and only in the aIsBGImage case, and change
pref name.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 189044 [details] [diff] [review]
implement pref for onload-after-bg-loads

looks great

Comment 7

[Christian Persch (GNOME) (away; not receiving bug mail)]

Comment on attachment 189044 [details] [diff] [review]
implement pref for onload-after-bg-loads

Just check a pref instead of an env var; has no risk.

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

Checking in layout/style/nsCSSValue.cpp;
/cvsroot/mozilla/layout/style/nsCSSValue.cpp,v  <--  nsCSSValue.cpp
new revision: 1.32; previous revision: 1.31
done