Defer loading the newly created docShell after a preloaded newtab page has been swapped in

RESOLVED FIXED in Firefox 24

Status

()

Firefox
Tabbed Browser
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: ttaubert, Assigned: ttaubert)

Tracking

Trunk
Firefox 24
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

4 years ago
When a new tab is opened (and preloading enabled) we swap docShells of the newly opened tab and a preloaded one in the background. There is no need to actually start loading the newly created docShell until the tabopen animation has been finished. The animation is a lot smoother on my quite fast machine with this fixed.
(Assignee)

Comment 1

4 years ago
Created attachment 753495 [details] [diff] [review]
defer loading the newly created docShell after a preloaded newtab page has been swapped in
Attachment #753495 - Flags: review?(dao)
Comment on attachment 753495 [details] [diff] [review]
defer loading the newly created docShell after a preloaded newtab page has been swapped in

>             if (!uriIsAboutBlank) {
>               // pretend the user typed this so it'll be available till
>               // the document successfully loads
>               if (aURI && gInitialPages.indexOf(aURI) == -1)
>                 b.userTypedValue = aURI;
> 
>-              let flags = Ci.nsIWebNavigation.LOAD_FLAGS_NONE;
>-              if (aAllowThirdPartyFixup)
>-                flags |= Ci.nsIWebNavigation.LOAD_FLAGS_ALLOW_THIRD_PARTY_FIXUP;
>-              if (aFromExternal)
>-                flags |= Ci.nsIWebNavigation.LOAD_FLAGS_FROM_EXTERNAL;
>-              if (aIsUTF8)
>-                flags |= Ci.nsIWebNavigation.LOAD_FLAGS_URI_IS_UTF8;
>-              try {
>-                b.loadURIWithFlags(aURI, flags, aReferrerURI, aCharset, aPostData);
>-              } catch (ex) {
>-                Cu.reportError(ex);
>+              // If we didn't swap docShells with a preloaded browser
>+              // then let's just continue loading the page normally.
>+              if (!docShellsSwapped) {
>+                let flags = Ci.nsIWebNavigation.LOAD_FLAGS_NONE;
>+                if (aAllowThirdPartyFixup)
>+                  flags |= Ci.nsIWebNavigation.LOAD_FLAGS_ALLOW_THIRD_PARTY_FIXUP;
>+                if (aFromExternal)
>+                  flags |= Ci.nsIWebNavigation.LOAD_FLAGS_FROM_EXTERNAL;
>+                if (aIsUTF8)
>+                  flags |= Ci.nsIWebNavigation.LOAD_FLAGS_URI_IS_UTF8;
>+                try {
>+                  b.loadURIWithFlags(aURI, flags, aReferrerURI, aCharset, aPostData);
>+                } catch (ex) {
>+                  Cu.reportError(ex);
>+                }
>               }
>             }

Why not just check docShellsSwapped along with uriIsAboutBlank? "pretend the user typed this so it'll be available till the document successfully loads" doesn't seem to make sense for swapped docshells where nothing gets loaded.

(How) Does the timer code handle new tabs being opened in fast succession?
(Assignee)

Comment 3

4 years ago
(In reply to Dão Gottwald [:dao] from comment #2)
> Why not just check docShellsSwapped along with uriIsAboutBlank? "pretend the
> user typed this so it'll be available till the document successfully loads"
> doesn't seem to make sense for swapped docshells where nothing gets loaded.

Right, good point. Great, that makes the code a little less 'complex'.

> (How) Does the timer code handle new tabs being opened in fast succession?

If there's no preloaded page that is readyState=complete and has the right URL we will not swap docShells and therefore just continue loading as normal. The new tab will load as it currently does.
(Assignee)

Comment 4

4 years ago
Created attachment 753707 [details] [diff] [review]
defer loading the newly created docShell after a preloaded newtab page has been swapped in, v2
Attachment #753495 - Attachment is obsolete: true
Attachment #753495 - Flags: review?(dao)
Attachment #753707 - Flags: review?(dao)

Updated

4 years ago
Attachment #753707 - Flags: review?(dao) → review+
(Assignee)

Comment 5

4 years ago
Thanks!

https://hg.mozilla.org/integration/fx-team/rev/876d89111acb
(Assignee)

Comment 6

4 years ago
https://hg.mozilla.org/mozilla-central/rev/876d89111acb
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 24

Updated

4 years ago
Blocks: 842607
(Assignee)

Updated

4 years ago
Depends on: 886376
You need to log in before you can comment on or make changes to this bug.