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]
:
Mentors:
Depends on: 886376
Blocks: 791670 842607
  Show dependency treegraph
 
Reported: 2013-05-23 14:36 PDT by Tim Taubert [:ttaubert]
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]
no flags Details | Diff | 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]
dao+bmo: review+
Details | Diff | Review

Description Tim Taubert [:ttaubert] 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] 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] 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] 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] 2013-05-24 03:13:25 PDT
Thanks!

https://hg.mozilla.org/integration/fx-team/rev/876d89111acb
Comment 6 Tim Taubert [:ttaubert] 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.