Build Identifier: http://hg.mozilla.org/integration/mozilla-inbound/rev/96d374c8f833 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.0) Gecko/20130723 Firefox/25.0 ID:20130723004349 Steps To Reproduce: 1. Create New Profile and Start Nightly with the profile 2. Open http://rml.lri.fr/papers/Mandel-These.pdf and wait until completion of the drawing of the page 3. Exit Nightly(Alt >File > Exit) and Restart Nightly 4. Wait 5-10 sec --- observe download toolbar button 5. Repeat step 3-4 if you want Actual Results: Unanticipated downloading will be started Expected Results: Unexpected downloading should not be started Regression window(m-i) Good: http://hg.mozilla.org/integration/mozilla-inbound/rev/599fe516bed5 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.0) Gecko/20130723 Firefox/25.0 ID:20130723002346 Bad: http://hg.mozilla.org/integration/mozilla-inbound/rev/96d374c8f833 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.0) Gecko/20130723 Firefox/25.0 ID:20130723004349 Pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=599fe516bed5&tochange=96d374c8f833 Regressed by: Bug 870100

This also happens on ubuntu12.04 http://hg.mozilla.org/mozilla-central/rev/d0edf8086809 Mozilla/5.0 (X11; Linux i686; rv:25.0) Gecko/20100101 Firefox/25.0 ID:20130803030205

This probably occurs because addons are quite unsupport in remote processes right now, so pdf.js doesn't get to handle the content and we decide to download it.

I haven't tried the STR yet, but it seems strange to me that bug 870100 would be involved if about:newtab isn't one of the steps needed to repro. Either way, I guess it probably makes sense for the service to decline to capture anything that isn't text/html (and possibly a few others make sense too) - OTOH though, these items probably shouldn't be on about:newtab in the first place (and if they weren't, the b/g thumbnail service wouldn't be attempting to grab them)

Oh yeah, this bug makes me crazy since a week! When the New Tab Page displays a direct link to a PDF, Nightly downloads this PDF each time I open a new tab.

As per conversation on IRC I think implementing an nsIContentPolicy that uses a whitelist for .shouldProcess() might be the best way to fix this. I'll file a follow-up bug to investigate why these types of links show up on about:newtab at all. We probably need to filter those responses somehow from the links that the Places DB gives us. I think we should not block those types of links in general on about:newtab, we should just not automically add them. Users can drag all kinds of links onto the newtab page and I wouldn't want to restrict that.

Something worth trying: setting the thumbnail browser's docShell's parentContentListener to an nsIURIContentListener whose onStartURIOpen returns false for everything. I.e. docShell.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIURIContentListener).parentContentListener = {onStartURIOpen: () => false};

(In reply to Tim Taubert [:ttaubert] from comment #5) > As per conversation on IRC I think implementing an nsIContentPolicy that > uses a whitelist for .shouldProcess() might be the best way to fix this. I chatted with Mark about this - I don't think that will work, because while the content policy lets you block loads, it doesn't let you reliably determine how they will be handled before doing it, and so it would be tricky to determine whether a load should be blocked (consider content-disposition:attachment). (I'm not sure whether my suggested solution from comment 6 handles that case either, though.)

Alice, are there some other steps or context that are missing from your STR? I can't reproduce comment 0. Like Mark says, the background service shouldn't be involved at all if about:newtab is not involved. After I restart and open about:newtab, there's a thumbnail for the pdf. (But the image shows only pdf.js's chrome; the actual "paper" document is missing.) However, I can reproduce this way: 1. Open about:newtab 2. Open this bug in a new browser window 3. Drag the pdf link in comment 0 from the new window to a tile on the about:newtab window Then the download starts.

(In reply to Drew Willcoxon :adw from comment #8) > Alice, are there some other steps or context that are missing from your STR? Just omitted step in the STR: "Close dialog box if "Default Browser" dialog pop up" 100% reproducible with the STR in comment#0

Hmm, thanks, Alice. I wonder what the difference is. (In reply to Drew Willcoxon :adw from comment #8) > 3. Drag the pdf link in comment 0 from the new window to a tile on the > about:newtab window What should happen when you do this? The background service shouldn't start a download that the user clearly did not directly trigger (e.g., comment 0), but what about when the user drags and drops some downloadable file? I'm looking at about:newtab's code, and before it was updated to use the background service, it looks like it would create a tile for the file but have no image to show for it. I guess keeping that status quo would be OK.

In my case, I need to pin a direct link to a PDF in the new tab page to repro the bug each time I start the browser or I open a new tab. Ex: see my screenshot http://i.imgur.com/wH7httn.jpg where Nightly has downloaded 26 times the same PDF at each new tab ^^

(In reply to Drew Willcoxon :adw from comment #10) > Hmm, thanks, Alice. I wonder what the difference is. > > (In reply to Drew Willcoxon :adw from comment #8) > > 3. Drag the pdf link in comment 0 from the new window to a tile on the > > about:newtab window > > What should happen when you do this? Download starts

(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #6) > docShell.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci. > nsIURIContentListener).parentContentListener = {onStartURIOpen: () => false}; This is crashing the child process for some reason. I haven't attached gdb, but printfs show me that it's crashing here on the addref line: http://mxr.mozilla.org/mozilla-central/source/docshell/base/nsDSURIContentListener.cpp#235 Something screwy is up with mParentContentListener. The pointer is nonnull but apparently invalid, since the crash happens when it's dereferenced. mParentContentListener is the JS object assigned to parentContentListener above. I feel like I'm missing some obvious XPConnect or something problem. In debugging this, I also noticed that after restarting with the only open tab being about:home, a background capture is triggered for the pdf, which I had dragged and dropped onto about:newtab in a previous session (the first session after creating the new profile). I dumped the call stacks, and ultimately the gPage.init() call in newTab.js is getting called, which triggers a bunch of things resulting in a background capture. The only thing that includes newTab.js is newTab.xul, and sure enough, about:memory lists a newTab.xul window, but I never opened about:newtab during the session.

(In reply to Drew Willcoxon :adw from comment #13) > Something screwy is up with mParentContentListener. The pointer is nonnull > but apparently invalid, since the crash happens when it's dereferenced. Actually it's the NS_IF_ADDREF. Dereferencing alone doesn't crash. NS_IF_ADDREF is some AddRef call, but I can't find where it's implemented. I thought it might be nsXPCWrappedJS::AddRef, but that doesn't seem to be it. > In debugging this, I also noticed that after restarting with the only open > tab being about:home, a background capture is triggered for the pdf Mark points out this may be due to preloading about:newtab.

Seems strange. If you make your nsIURIContentListener implementation implement a QueryInterface that returns nsISupportsWeakReference, does it avoid the crash? The only other JS setter of parentContentListener seems to do that, though I'm not sure offhand why it would matter. http://hg.mozilla.org/mozilla-central/annotate/e33c2011643e/toolkit/content/widgets/editor.xml#l34

more debugging shows that the code that hands the download off is at http://mxr.mozilla.org/mozilla-central/source/uriloader/base/nsURILoader.cpp#517, and an nsIURIContentListener could indeed prevent that. So I think this is the route we should try and nail. FTR, this attachment does prevent many errors in the console regarding plugins and the block-list service - which sounds worthwhile, but doesn't prevent the download happening.

Not worth tracking, but please don't hesitate to nominate for uplift if a fix is found. Motivation for not tracking: * Most session tabs do not auto-restore for most people on launch * If the PDF is downloading now, it's downloaded before (doesn't cause an undesirable to download)

(In reply to Alex Keybl [:akeybl] from comment #17) > Not worth tracking, but please don't hesitate to nominate for uplift if a > fix is found. Motivation for not tracking: > > * Most session tabs do not auto-restore for most people on launch > * If the PDF is downloading now, it's downloaded before (doesn't cause an > undesirable to download) Please read STR in comment #0 carefully! No, I do not carry out downloading operation at all.

Thanks, nsISupportsWeakReference prevents the crash. However, onStartURIOpen gets called, but it's called for every load, not only for files we want to block, and it's passed only a URI, so it can't discriminate. There are other nsIURIContentListener methods that look useful, but I can't get anything to call them by setting parentContentListener. onStartURIOpen is called at [1], and it seems the content listener for that call is different from the listeners used elsewhere in nsURILoader (m_contentListener, mURILoader->m_listeners). So I stuck a listener in mURILoader->m_listeners, and that works. The patch turns out to be pretty simple. canHandleContent returns true if the content type is not "text/html", and doContent returns true to say "I'll take it from here (... and drop the request, sucka)." Am I missing any obvious problems? I need a little bit more though to convince myself that everything is properly cleaned up after each capture for all possible paths (e.g., no page load listeners sticking around, loads stopped or not as necessary), and some refactoring, so I'm calling this a WIP. [1] http://mxr.mozilla.org/mozilla-central/source/uriloader/base/nsURILoader.cpp#826

(In reply to Drew Willcoxon :adw from comment #19) > Created attachment 788480 [details] [diff] [review] > WIP: block non-text/html > > Thanks, nsISupportsWeakReference prevents the crash. However, > onStartURIOpen gets called, but it's called for every load, not only for > files we want to block, and it's passed only a URI, so it can't > discriminate. There are other nsIURIContentListener methods that look > useful, but I can't get anything to call them by setting > parentContentListener. onStartURIOpen is called at [1], and it seems the > content listener for that call is different from the listeners used > elsewhere in nsURILoader (m_contentListener, mURILoader->m_listeners). > > So I stuck a listener in mURILoader->m_listeners, and that works. That makes sense to me - those listeners are only called if the parent listener can't handle it. So it might be that onStartURIOpen can still be used now the listener added to those "other" listeners? I do wonder what other listeners are typically added though - it's almost as though we want to ensure our new listener is added at the *start* of m_listeners, but RegisterContentListener() adds them to the end. But - huh - it looks like there are no callers of this function in the tree! > The patch > turns out to be pretty simple. canHandleContent returns true if the content > type is not "text/html", and doContent returns true to say "I'll take it > from here (... and drop the request, sucka)." Am I missing any obvious > problems? Hrm - my testing shows that your canHandleContent isn't ever actually called with text/html - I'm testing with a few HTML pages, an image, a .py and .pdf on about:home, and I only ever see that handler called for the .py and .pdf. So it looks to me like you can reject everything? Also, IIRC, it was the "primary" content handler that does the thing with plugins etc - assuming you haven't applied that other patch I added, can you please verify that you still see errors relating to plugins and the blocklist with your patch? If so, I think we will still want mine too. Looks good though, and solves the problem for me!

One extra consideration is that we will probably want some way to prevent us attempting the thumbnail on each about:newtab. This is somewhat similar to bug 897880, although in this case we will never have a thumbnail to "touch". It's almost as as though we will want some kind of "pseudo" thumbnail file which indicates no thumbnail actually exists. Or something.

Comment on attachment 788013 [details] [diff] [review] Have nsWebNavigationInfo avoid looking for plugins if the docShell isn't going to use it anyway Can you put this patch in a new bug, that blocks bug 897031? Seems like really a completely separate issue.

The background service needs to not cause downloads to occur if we want to ship it, so tracking25+ (but we'll likely address by disabling the background service).

(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #22) > Can you put this patch in a new bug, that blocks bug 897031? Seems like > really a completely separate issue. Done - bug 903965, which I haven't marked as blocking this bug as the only impact seems to be error messages on the console in debug builds.

(In reply to Mark Hammond (:markh) from comment #20) > So it might be that onStartURIOpen can still be used now the listener > added to those "other" listeners? I was confused about this part so I asked about it: > markh: adw: so - I'm saying that now the listener is being added via > registerContentListener, we want to just say "please don't do this" > at the earliest opportunity and without regard to the URL or > content-type etc. I was thinking onStartURIOpen might be that > earliest opportunity. But I may well be wrong about that :) > adw: markh: i see, let me try that. you mean the onStartURIOpen on the > listener added via registerContentListener, not a listener attached > via parentContentListener, right? > markh: yes onStartURIOpen is never called on the listener added via registerContentListener. In summary, when the listener is the parentContentListener, onStartURIOpen is called for all loads, even text/html, but none of the other methods are called. When the listener is added with registerContentListener, onStartURIOpen is not called, but the other methods are, but only for non-HTMLy documents. > Hrm - my testing shows that your canHandleContent isn't ever actually called > with text/html You're right! I just realized a giant potential problem with this patch. It's adding a content listener to a global loader service. There's no association with the thumbnail browser. So if the listener gets veto power over the thumbnail browser, doesn't it get veto power over other browsers, other loads? It looks like the answer is no, but I can't see why, maybe a side effect of using a remote browser.

Another thing wrong with that patch is "is the document HTMLy" is too crude a metric. If an image is one of your top sites, or you drag an image to about:newtab, the background service should be able to handle it. What we really want is, does the browser handle this object by displaying it? If so, then load and capture it. Does anyone know whether there's any programmatic way to tell? (With pdf.js, PDFs fall into this category, so I wonder why passing a PDF causes it to be downloaded. Because pdf.js isn't used in the content process?)

I spoke to Drew about this yesterday - rather than trying to focus on detecting "loads that will be handled externally", I think we should just add a flag on the docShell that's something like "dontHandleExternally", and then just have all of the points where we handle things externally check for it before doing their thing. I think that means just preventing us from reaching: http://hg.mozilla.org/mozilla-central/annotate/4930fdea3efa/uriloader/base/nsURILoader.cpp#l517 It might also mean skipping some of the other things that DispatchContent() does, but I'm not sure if any of those could end up being considered "external" (I think not, given that they are done only if !forceExternalHandling).

(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #27) > I spoke to Drew about this yesterday - rather than trying to focus on > detecting "loads that will be handled externally", I think we should just > add a flag on the docShell that's something like "dontHandleExternally", and > then just have all of the points where we handle things externally check for > it before doing their thing. One problem with this approach (I think) is that it will still prevent onload firing, causing us to timeout. Drew's original approach solved this by cancelling the wait as we rejected the content. So if we do go this route, we would probably also need an additional notification from the docShell to tell us it acted on this flag and therefore no load event is going to fire (or use some existing event/notification I'm not aware of)

(In reply to Mark Hammond (:markh) from comment #28) > One problem with this approach (I think) is that it will still prevent > onload firing, causing us to timeout. Drew's original approach solved this > by cancelling the wait as we rejected the content. True, that is sub-optimal, but it doesn't seem so bad. To fix this without needing to add a custom notification, we could probably use a WebProgressListener to watch for a top-level STATE_STOP notification with a failed status code, or something.

Good point. What I would like to do is modify the paths in nsURILoader that don't consult the parent content listener to consult it. Then we could use Gavin's approach in comment 6 and it would work and be simple. It seems weird to avoid the parent when calling some nsIURIContentListener methods but not for others. (I talked about this with Gavin this week.) But in bug 334703, bz vetoes that idea. And it does seem dangerous to modify such important paths that have been working like they work for a long time. So the next best thing might be to hook into nsDocShell's nsIURIContentListener implementation, the one it returns when you call GetInterface(nsIURIContentListener) on it. That's the first listener that nsDocumentOpenInfo::DispatchContent consults (where it's called m_contentListener). What I'm thinking is an nsIDocShell property of type nsIURIContentListener that you can set to whatever you want, and then nsDocShell would consult that object in the listener implementation it returns when you call GetInterface(nsIURIContentListener). No one else but us is going to use this property, so it'll be null all other times, so we're not changing how this longstanding code works for anybody else. And I think it's simpler than adding a new LOAD_PREVENT_EXTERNAL load flag and making sure it's used and propagated in all the load group, channel, docshell, and whatever else goop, on top of doing the things in the previous two comments.

When I said "flag" in comment 27, I meant adding an attribute to nsIDocShell, like e.g. allowPlugins, and then just propagating it to nsDocumentOpenInfo somehow - either as a boolean argument, or using its existing aFlags argument. what do you think of that?

I understand, but I didn't notice that nsDocumentOpenInfo had an mFlags member. (So I assumed that the "somehow" had to boil down to another load flag -- either that or a new property on nsIChannel, since DispatchContent has access to a channel but not the docshell associated with it.) nsURILoader::OpenChannel takes a aWindowContext, which I think is a docshell, so it might work to QI it to nsIDocShell, get the flag, and concat it with aFlags, which becomes nsDocumentOpenInfo's mFlags. I'm not sure who calls OpenChannel and whether it could be modified to include the flag in aFlags instead.

OK, comment 31 and comment 29 work, thanks, Gavin. I have an allowExternalHandling attribute on nsIDocShell whose value is passed as part of the flags to nsDocumentOpenInfo and checked in DispatchContent. In the thumbnail content script, a web progress listener on the docshell captures requests that are stopped due to disabled external handling. (In reply to Drew Willcoxon :adw from comment #32) > I'm not sure who calls OpenChannel and whether it could be modified to > include the flag in aFlags instead. nsURILoader::OpenURI calls it, and OpenURI is called by the docshell, so theoretically we could add an allowExternalHandling parameter to OpenURI instead of modifying the flags passed into OpenChannel based on sniffing the docshell, which seems a little icky. We'll see what whoever reviews this says.

I filed bug 906276 for the back-end changes. This patch uses a web progress listener to listen both for canceled downloads and for normal page loads. I think it makes the script easier to understand actually. An interesting consequence of this design is that canceled downloads end up getting thumbnails too, but they're blank. If they didn't get thumbnails, then every time you open about:newtab, they would trigger new, hopelessly doomed captures, since there are no fresh thumbnails for them. This needs a test, and I'll work on that and attach a separate patch.

Comment on attachment 791636 [details] [diff] [review] use nsIDocShell.allowExternalHandling + a web content listener Review of attachment 791636 [details] [diff] [review]: ----------------------------------------------------------------- Looks fine in general, but I'm concerned about the queue not being necessary. Defering review decision for now because if I am wrong about that I need to better understand things. Also, I think we should file a followup bug for what happens when a thumbnail can't be grabbed - eg, UX might prefer a generic icon, (or even the favicon) rather than just blank which we could just copy - but blank is fine for this bug IMO. ::: toolkit/components/thumbnails/content/backgroundPageThumbsContent.js @@ +38,3 @@ > }, > > + _pendingResponseQueue: [], I don't understand why this needs to be a queue - it will always have exactly 1 or 0 items in it IIUC? (It seems that theoretically _onCapture could be called again while waiting for onStateChange and/or onloadend events, but (a) that's prevented by BackgroundPageThumbs not initiating another until this messages back a completion notification and (b) if that happened we'd have other problems, such as the .stop at the top of _onCapture() Maybe that was a hangover from before you added webProgress.isTopLevel checks (when I can imagine a queue would have been necessary to work out when the top-level one was done) @@ +91,5 @@ > + Ci.nsISupportsWeakReference, > + ]), > +}; > + > +function Response(requestID) { assuming the above is correct, it seems a little overkill to have a new object just for this. However, if you feel it does add value, please rename it to something like CaptureResponse to make it clearer that the "new Response()" above isn't referring to a HTTP response or similar.

You're right. I was thinking of a capture that times out followed by another capture, which would cause the second _onCapture to be called before the first capture finished. But the stop() in _onCapture causes onStateChange to be called synchronously, and I just assumed it would be async. So no more queue. Also gone is the stop(), since loadURI() first stops the currently loading page. I found a bug in the queueing test while working through this: it checks for "?wait" in the URL to detect the timeout URL, but the URL ends up looking like '?{"wait"' (except encoded).

(In reply to Drew Willcoxon :adw from comment #34) > An interesting consequence of this design is that > canceled downloads end up getting thumbnails too, but they're blank. If > they didn't get thumbnails, then every time you open about:newtab, they > would trigger new, hopelessly doomed captures, since there are no fresh > thumbnails for them. Hmm, this seems a bit odd and inefficient. Could we instead just maintain a blacklist of sites we shouldn't try to capture because previous captures of it failed for this reason?

(In reply to Drew Willcoxon :adw from comment #37) > Created attachment 792500 [details] [diff] [review] > use nsIDocShell.allowExternalHandling + a web content listener, 2 Bad patch name, since I renamed the docshell flag allowContentRetargeting (but am waiting on review on that). And on second thought, I don't think a test is necessary. Bug 906276 has a test for the docshell flag, and we rightly don't have thumbnail tests for allowMedia, allowPlugins, etc. Is that all right with you?

(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #38) > Hmm, this seems a bit odd and inefficient. Could we instead just maintain a > blacklist of sites we shouldn't try to capture because previous captures of > it failed for this reason? I don't think it's worth worrying about honestly. It's not odd, it's simple conceptually, and it's not worth the added code complexity. wasErrorResponse is true in these cases, so if anything, if a site doesn't have a thumbnail, it's captured, and it's an error, we could use a generic thumbnail image or favicon like Mark suggests in comment 35.

Comment on attachment 792500 [details] [diff] [review] use nsIDocShell.allowExternalHandling + a web content listener, 2 Review of attachment 792500 [details] [diff] [review]: ----------------------------------------------------------------- nice!

(In reply to Mark Hammond (:markh) from comment #35) > Also, I think we should file a followup bug for what happens when a > thumbnail can't be grabbed - eg, UX might prefer a generic icon, (or even > the favicon) rather than just blank which we could just copy I filed bug 908960.

Landed on inbound with the docshell bug: https://hg.mozilla.org/integration/mozilla-inbound/rev/33a4e19102e7 https://tbpl.mozilla.org/?tree=Try&rev=9d61eb19a7f9

This was backed out by http://hg.mozilla.org/integration/mozilla-inbound/rev/48a3aeb26ed6

Reverted to clear the tree: https://hg.mozilla.org/integration/mozilla-inbound/rev/48a3aeb26ed6 Please make sure to actually look at your try results in the future.

(In reply to Phil Ringnalda (:philor) from comment #46) > https://hg.mozilla.org/mozilla-central/rev/33a4e19102e7 This is reverted by 48a3aeb26ed6 Ms2ger — Revert this CLOSED TREE to changeset 4d3e221584a0.

There are two different problems: when goodResponseUpdateTest calls PageThumbs.captureIfStale, the thumbnail is sometimes, but not always, totally black instead of red as expected; and the newtab preloader is interfering with browser_thumbnails_update.js, which made debugging this trickier than it would have been otherwise. Only the first problem showed up on the push. The preloader interferes when I run browser_thumbnails_update.js in isolation locally most of the time. The black thumbnail seems to happen because the window isn't ready (painted?) when it's captured. We don't have any problems with the current page load listener approach, so I wondered if the web progress listener in this patch is being notified before page load, and it is. It's being notified as a result of this call: http://mxr.mozilla.org/mozilla-central/source/uriloader/base/nsDocLoader.cpp#865 I stuck a breakpoint in a one-off load listener to find where load listeners are called, and it turns out it's the same line. Load listeners are called via the docshell, which is itself a web progress listener that's notified here. But I don't understand why the failure doesn't consistently happen. I do understand why the listener added by the content script is called (most of the time) before the docshell-as-listener: nsDocLoader::DoFireOnStateChange iterates over listeners in reverse, and presumably the listener added by the content script comes after the docshell. Since nsDocLoader fires STATE_IS_WINDOW a few lines below STATE_IS_DOCUMENT, this patch listens for WINDOW instead of DOCUMENT. It consistently works, but maybe if I ran it enough times it'd fail, too. That's the only difference from the previous patch. I wanted to get your thoughts, so I'm asking for review again. I'm rebuilding the world right now, and after that I'll try to find out more about the inconsistent timing. I'll post a preloader patch in a little bit.

Comment on attachment 796265 [details] [diff] [review] listen for window web progress stop, not document Review of attachment 796265 [details] [diff] [review]: ----------------------------------------------------------------- That load event thing is a little strange, but I guess it implies that whatever the "important" load handler does, it must do it synchronously or else we would have hit this before. That implies another spin of the event loop before we actually start a capture would also be an alternative. OTOH though, given your analysis, I don't see how using the window instead of the document would be unreliable, so I think it's fine as it is, especially given the test coverage. ::: toolkit/components/thumbnails/content/backgroundPageThumbsContent.js @@ +47,5 @@ > + Ci.nsIWebNavigation.LOAD_FLAGS_STOP_CONTENT, > + null, null, null); > + // If a page was already loading, onStateChange is synchronously called at > + // this point by loadURI. > + this._requestID = msg.json.id; FTR, I recently learned that msg.json is deprecated and msg.data should be used instead. I don't think this should change here - I just note it FYI)

Gavin suggested filing a new bug for the preloader: bug 910036.

https://tbpl.mozilla.org/?tree=Try&rev=47615d315fec https://hg.mozilla.org/integration/fx-team/rev/8742a951f557

Please uplift this to Aurora25.0a2.

I would uplift it, but there have been other changes to this code in 26, so we'd need to rebase this patch on Aurora, and then when 26 merges into Aurora, we'd have to deal with merge conflicts. The same is true to a smaller extent for bug 906276, which this bug depends on. It's only two more weeks until this bug moves to Aurora, though. :-)

(In reply to Drew Willcoxon :adw from comment #54) > and then when 26 merges into Aurora, we'd have to deal with merge conflicts. That's not true, the changes made on aurora are moved out of the way before we merge.

Has this made it's way to inbound yet because I have the problem?

@markh If you cannot fix this in Aurora25.0a2, I think you should need to back out Bug 870100.

See bug 912763

b/g thumbnails are not going to be in 25 on beta.