430050 - xul:browser changes break Firebug

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Olli Pettay [:smaug][[e-mail]]]

Attachment: cancel frameloader initialization when needed.

Comment 1

[Mike Shaver (:shaver emeritus)]

Gotta block, and we need some test coverage here, clearly. :/

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

yup, I'm writing tests.

Comment 3

[John J. Barton]

I don't want to discourage your work on this problem, but it need not block 411814, firebug no longer uses loadURI in its own load sequence.

Comment 4

[Mike Shaver (:shaver emeritus)]

I would be extremely surprised if this didn't also affect other extensions, but yeah, thanks a ton for that parallel fix.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Attachment: a test

This is not a perfect test. I should still figure out how to test the
behavior Firebug does. But useful test anyway.

Comment 6

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 316804 [details] [diff] [review]
cancel frameloader initialization when needed.

>+  // Don't use a temporary array for mInitializableFrameLoaders, because
>+  // loading a frame may cause some other frameloader to be removed from the
>+  // array. But be careful to keep the loader alive when starting the load!
>+  while (mInitializableFrameLoaders.Length()) {
>+    nsRefPtr<nsFrameLoader> loader = mInitializableFrameLoaders[0];
>+    mInitializableFrameLoaders.RemoveElementAt(0);
>+    if (loader) {
>+      loader->ReallyStartLoading();

Is the nullcheck really needed?

>@@ -6808,16 +6808,28 @@ nsDocShell::InternalLoad(nsIURI * aURI,
>         return NS_ERROR_FAILURE;
>     }
> 
>     rv = CheckLoadingPermissions();
>     if (NS_FAILED(rv)) {
>         return rv;
>     }
> 
>+    // If this docshell is owned by a frameloader, make sure to cancel
>+    // possible frameloader initialization before loading a new page.
>+    nsCOMPtr<nsIDocShellTreeItem> parent;
>+    GetParent(getter_AddRefs(parent));
>+    if (parent) {
>+      nsCOMPtr<nsIDOMDocument> domDoc = do_GetInterface(parent);
>+      nsCOMPtr<nsIDocument> doc = do_QueryInterface(domDoc);

Silly that docshells don't GetInterface to nsIDocument :(

r=me

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 316804 [details] [diff] [review]
cancel frameloader initialization when needed.

nullcheck is not needed.
Doesn't cause any harm though.

Comment 8

[Damon Sicore (:damons)]

Comment on attachment 316804 [details] [diff] [review]
cancel frameloader initialization when needed.

a1.9+=damons

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Attachment: without null check