Last Comment Bug 875509 - Defer loading the newly created docShell after a preloaded newtab page has been swapped in
: Defer loading the newly created docShell after a preloaded newtab page has be...
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Tabbed Browser (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 24
Assigned To: Tim Taubert [:ttaubert] (on PTO, back Aug 29th)
:
Mentors:
Depends on: 886376
Blocks: 791670 842607
  Show dependency treegraph
 
Reported: 2013-05-23 14:36 PDT by Tim Taubert [:ttaubert] (on PTO, back Aug 29th)
Modified: 2013-06-24 10:20 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
defer loading the newly created docShell after a preloaded newtab page has been swapped in (7.42 KB, patch)
2013-05-23 14:50 PDT, Tim Taubert [:ttaubert] (on PTO, back Aug 29th)
no flags Details | Diff | Splinter Review
defer loading the newly created docShell after a preloaded newtab page has been swapped in, v2 (6.54 KB, patch)
2013-05-24 02:53 PDT, Tim Taubert [:ttaubert] (on PTO, back Aug 29th)
dao+bmo: review+
Details | Diff | Splinter Review

Description Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2013-05-23 14:36:35 PDT
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.
Comment 1 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2013-05-23 14:50:46 PDT
Created attachment 753495 [details] [diff] [review]
defer loading the newly created docShell after a preloaded newtab page has been swapped in
Comment 2 Dão Gottwald [:dao] 2013-05-24 02:48:59 PDT
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?
Comment 3 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2013-05-24 02:50:47 PDT
(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.
Comment 4 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2013-05-24 02:53:39 PDT
Created attachment 753707 [details] [diff] [review]
defer loading the newly created docShell after a preloaded newtab page has been swapped in, v2
Comment 5 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2013-05-24 03:13:25 PDT
Thanks!

https://hg.mozilla.org/integration/fx-team/rev/876d89111acb
Comment 6 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2013-05-24 13:21:27 PDT
https://hg.mozilla.org/mozilla-central/rev/876d89111acb

Note You need to log in before you can comment on or make changes to this bug.