Last Comment Bug 695640 - Profile View Source pre chunking and remove it if unnecessary
: Profile View Source pre chunking and remove it if unnecessary
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: HTML: Parser (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla11
Assigned To: Henri Sivonen (:hsivonen)
:
: Andrew Overholt [:overholt]
Mentors:
Depends on: 263359 482921
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-19 03:16 PDT by Henri Sivonen (:hsivonen)
Modified: 2011-12-13 14:39 PST (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Remove pre chunking (3.25 KB, patch)
2011-11-29 07:39 PST, Henri Sivonen (:hsivonen)
bugs: review+
christian: approval‑mozilla‑aurora-
Details | Diff | Splinter Review

Description Henri Sivonen (:hsivonen) 2011-10-19 03:16:55 PDT
Now that bug 263359 has been fixed, the part of bug 482921 that refixes bug 86355 may be unnecessary. Find out if it is unnecessary and remove if unnecessary.
Comment 1 Henri Sivonen (:hsivonen) 2011-11-29 07:39:20 PST
Created attachment 577611 [details] [diff] [review]
Remove pre chunking

I profiled View Source on a Hebrew news article and an Arabic news article with and without this patch and there seemed to be no interesting differences. AFAICT, the pre chunking is no longer needed, so let's get rid of it.
Comment 2 Simon Montagu :smontagu 2011-11-29 10:45:55 PST
Comment on attachment 577611 [details] [diff] [review]
Remove pre chunking

Review of attachment 577611 [details] [diff] [review]:
-----------------------------------------------------------------

I'm not sure what feedback you want from me here. I'm all for doing this, given that there's no perf impact.
Comment 3 Henri Sivonen (:hsivonen) 2011-11-30 09:48:09 PST
(In reply to Simon Montagu from comment #2)
> I'm all for doing this, given that there's no perf impact.

Great. Landed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c5d44e6e957b
Comment 4 Matt Brubeck (:mbrubeck) 2011-11-30 11:48:15 PST
Backed out on inbound (along with two other bugs from the same push) because of test failures:
https://hg.mozilla.org/integration/mozilla-inbound/rev/20a24dd3f56b

If this wasn't responsible for the failures, it can re-land.
Comment 5 Geoff Lankow (:darktrojan) 2011-11-30 16:16:30 PST
Re-landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/fcba47973317
Comment 6 Marco Bonardo [::mak] 2011-12-01 04:45:40 PST
https://hg.mozilla.org/mozilla-central/rev/fcba47973317
Comment 7 Henri Sivonen (:hsivonen) 2011-12-13 05:09:25 PST
Comment on attachment 577611 [details] [diff] [review]
Remove pre chunking

(This comment has been posted on all the bugs mentioned in this comment, except bug 710142, so that the release drivers see it regardless of the order in which they process approval requests.)

The new View Source implementation landed before Firefox 10 moved to Aurora. Afterwards, a bunch of regressions were identified. Many of the regression fixes didn't land before Firefox 10 moved to Aurora but they have now been fixed on trunk except for bug 710142.

To avoid shipping with regressions, we either need to land all the regression fixes on Aurora for Firefox 10 (followed by a fix for bug 710142 in Beta if it doesn't make it before Dec 20th) or switch back to the old View Source implementation on Aurora. The new View Source implementation provides much better diagnostics for Web developers than the old View Source implementation.

So I'd like to ask the release drivers to either approve bug 535530, bug 699356, bug 699365, bug 700034, bug 700361, bug 703965, bug 704667 and bug 705473 plus bug 695640, which is a non-regression tweak, or to approve bug 710175 for reverting to the old View Source implementation for Firefox 10.
Comment 8 christian 2011-12-13 14:39:08 PST
Comment on attachment 577611 [details] [diff] [review]
Remove pre chunking

[triage comment]
We decided to back out the new view source parser (bug 710175) rather than take this fixup for Firefox 10.

Denying for Aurora.

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