Last Comment Bug 751905 - about:memory and JS console broken on inbound tip in unpackaged build
: about:memory and JS console broken on inbound tip in unpackaged build
Status: VERIFIED FIXED
[qa-]
: regression
Product: Core
Classification: Components
Component: General (show other bugs)
: 15 Branch
: All All
: -- normal (vote)
: mozilla15
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
Depends on:
Blocks: CVE-2012-1945 751911
  Show dependency treegraph
 
Reported: 2012-05-04 08:36 PDT by Matt Brubeck (:mbrubeck)
Modified: 2012-05-22 19:19 PDT (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
fixed
+
fixed
fixed
13+
fixed


Attachments
This seems to fix it. (531 bytes, patch)
2012-05-04 11:46 PDT, Johnny Stenback (:jst, jst@mozilla.com)
bzbarsky: review+
Details | Diff | Review
Patch v2. (1.02 KB, patch)
2012-05-04 12:15 PDT, Brian R. Bondy [:bbondy]
netzen: review+
Details | Diff | Review
Pavtch v3. (1.35 KB, patch)
2012-05-04 15:36 PDT, Brian R. Bondy [:bbondy]
no flags Details | Diff | Review

Description Matt Brubeck (:mbrubeck) 2012-05-04 08:36:31 PDT
Not sure where to file this, sorry.

In a local Linux build from inbound, yesterday evening, njn noticed that about:memory would not load in a local (unpackaged) build, and I was able to reproduce this.  Instead of "about:memory", the urlbar shows a file: URI with the location of aboutMemory.xhtml in the srcdir.  The JS console will not load either (it shows an empty window with no console messages).

Bisecting led me to this possible regression range, though njn found that clobbering might alter the results.  (I bisected without clobbering.)  CPG has also been suggested as the culprit.
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=086e419f370a&tochange=26738df8a4e0

There was some possibly relevant logging from my debug build:

JavaScript error: , line 0: nothing active on context (repeated many times)

Security Error: Content at file:///home/mbrubeck/src/mozilla/beta/browser/themes/gnomestripe/searchbar.css may not load or link to chrome://mozapps/skin/places/defaultFavicon.png
Comment 1 Justin Lebar (not reading bugmail) 2012-05-04 08:39:10 PDT
> CPG has also been suggested as the culprit.

I suggested that when I assumed about:memory was broken differently than it is.  :)  This doesn't immediately sound like CPG to me.
Comment 2 Bobby Holley (busy) 2012-05-04 08:43:01 PDT
I noticed a similar error with:

TEST_PATH=js/xpconnect/tests make mochitest-chrome

I doubt that this is CPG-related, because the above is my standard smoketest, which I ran many times with my CPG patches before pushing them. Also, CPG isn't in that regression window.
Comment 3 Bobby Holley (busy) 2012-05-04 08:43:52 PDT
(FWIW, I'm pretty baffled that chrome tests are broken for me locally but not on tinderbox. Maybe it's the way that I'm running them?)
Comment 4 Kyle Huey [:khuey] (khuey@mozilla.com) 2012-05-04 08:51:22 PDT
I think this is https://hg.mozilla.org/integration/mozilla-inbound/rev/60613f18435b
Comment 5 neil@parkwaycc.co.uk 2012-05-04 08:56:11 PDT
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #4)
> I think this is  http://hg.mozilla.org/integration/mozilla-inbound/rev/60613f18435b

Indeed it is.

What's happening is that (in a symlink or unpackaged omnijar build) many chrome files are actually symlinks back to the source tree.

Part of the patch updates channels that point to symlinks to point them at their destination.

Unfortuantely this also ends up clobbering the original chrome: URL in the case that it resolves to a file: URL that is a symlink.
Comment 6 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-05-04 09:14:40 PDT
The right way to do this is probably to clear the LOAD_REPLACE flag on the underlying channel right around http://hg.mozilla.org/mozilla-central/annotate/db1f131884de/chrome/src/nsChromeProtocolHandler.cpp#l215
Comment 7 Johnny Stenback (:jst, jst@mozilla.com) 2012-05-04 11:46:16 PDT
Created attachment 621120 [details] [diff] [review]
This seems to fix it.

Per bz's suggestion this seems to fix both about:memory and the error console. Brian, can you push this through try etc and get this landed?
Comment 8 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-05-04 11:58:53 PDT
Comment on attachment 621120 [details] [diff] [review]
This seems to fix it.

Please move that code to after the comment.

And on a side note, -U8, please!
Comment 9 Brian R. Bondy [:bbondy] 2012-05-04 12:05:19 PDT
> Per bz's suggestion this seems to fix both about:memory and the error console. 
> Brian, can you push this through try etc and get this landed?

I did the same locally, but yup I'll see your patch to landing.
Comment 10 Brian R. Bondy [:bbondy] 2012-05-04 12:15:17 PDT
Created attachment 621126 [details] [diff] [review]
Patch v2.

Implemented bz nits on behalf of jst, pushed to try.
Comment 11 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-05-04 12:17:35 PDT
Comment on attachment 621126 [details] [diff] [review]
Patch v2.

Just to be clear, the -U8 nit was not about the patch per se, but about jst's hg config.  ;)
Comment 12 Brian R. Bondy [:bbondy] 2012-05-04 12:19:44 PDT
understood, I meant about moving the comment.  The auto -U8 came for free from my beautifully configured hgrc ;)
Comment 13 Johnny Stenback (:jst, jst@mozilla.com) 2012-05-04 12:34:50 PDT
-U8 hgrc bug fixed locally :)
Comment 14 Nicholas Nethercote [:njn] (on vacation until July 11) 2012-05-04 14:58:53 PDT
Thanks for filing this, mbrubeck.


(In reply to Bobby Holley (:bholley) from comment #3)
> (FWIW, I'm pretty baffled that chrome tests are broken for me locally but
> not on tinderbox. Maybe it's the way that I'm running them?)

That is worrying to me.  I guess that tinderbox builds are not "a symlink or unpackaged omnijar build"?
Comment 15 Kyle Huey [:khuey] (khuey@mozilla.com) 2012-05-04 15:01:59 PDT
Nope, tinderbox builds are packaged.
Comment 16 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-05-04 15:16:54 PDT
Seems like a comment is in order? Seems non-obvious why LOAD_REPLACE is being stripped out here.
Comment 17 Brian R. Bondy [:bbondy] 2012-05-04 15:36:53 PDT
Created attachment 621195 [details] [diff] [review]
Pavtch v3.

Added its own comment, will land tonight when try results finish.
Comment 18 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-05-04 16:30:48 PDT
Actually, I believe the original comment clearly described what's going on.  For someone who knows how the principal of a load and the URI of a document is determined, at least.  For someone who doesn't know that this whole part of the code is complete mumbo-jumbo without _way_ better comments than what's there right now.
Comment 19 Brian R. Bondy [:bbondy] 2012-05-04 16:32:13 PDT
Are you OK if I land patch v2?
Comment 20 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-05-04 16:54:30 PDT
Yeah, go for it.
Comment 21 Brian R. Bondy [:bbondy] 2012-05-04 18:50:09 PDT
http://hg.mozilla.org/mozilla-central/rev/0a48e6561534
Comment 22 Brian R. Bondy [:bbondy] 2012-05-18 20:28:42 PDT
Fixed for Aurora in changeset:
http://hg.mozilla.org/releases/mozilla-aurora/rev/cf5c4540fe22
See Bug 670514
Comment 23 Alex Keybl [:akeybl] 2012-05-21 15:57:37 PDT
(In reply to Brian R. Bondy [:bbondy] from comment #22)
> Fixed for Aurora in changeset:
> http://hg.mozilla.org/releases/mozilla-aurora/rev/cf5c4540fe22
> See Bug 670514

I believe the same goes for beta now.
Comment 24 Brian R. Bondy [:bbondy] 2012-05-21 16:04:30 PDT
Yup thanks for marking.
See Bug 670514
http://hg.mozilla.org/releases/mozilla-beta/rev/7a2c1909e205
Comment 25 Brian R. Bondy [:bbondy] 2012-05-21 17:55:41 PDT
http://hg.mozilla.org/releases/mozilla-esr10/rev/8ef222645a1a
Comment 26 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-05-22 15:20:49 PDT
Matt, could you please verify this is now fixed in Firefox 13, 14, 15, and latest-mozilla-esr10? or provide QA with steps to reproduce this in those builds?
Comment 27 Matt Brubeck (:mbrubeck) 2012-05-22 18:45:04 PDT
This bug does not affect the builds produced by mozilla; it only affects builds on developers' machines. I can mark this verified on trunk but I don't have time to test it on all the branches. (I'm on vacation this week.)
Comment 28 Nicholas Nethercote [:njn] (on vacation until July 11) 2012-05-22 19:04:55 PDT
(In reply to Matt Brubeck (:mbrubeck) [back on 5/21] from comment #27)
> This bug does not affect the builds produced by mozilla; it only affects
> builds on developers' machines. I can mark this verified on trunk but I
> don't have time to test it on all the branches.

I can confirm it's fine on trunk (mbrubeck and I were the first to notice the bustage).  I don't think it's relevant to other branches because the bug that caused it (bug 670514) landed during the FF15 dev cycle.
Comment 29 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-05-22 19:19:10 PDT
Okay, thanks Matt and Nicholas. I'm marking this [qa-] based on your comments. Please change the whiteboard tag to [qa+] if something changes and there is something QA needs to do to verify this fix.

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