Closed
Bug 1313083
Opened 8 years ago
Closed 8 years ago
Repair line layout debug flags in nsLineLayout
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
FIXED
mozilla52
| Tracking | Status | |
|---|---|---|
| firefox52 | --- | fixed |
People
(Reporter: chenpighead, Assigned: chenpighead)
Details
Attachments
(3 files)
While hacking around layout codes, I'd turn on debugging flags in nsLineLayout [1] from time to time. It appears some of them are broken, and some of them are even not used anymore. I would like to fix this. Here is the plan: 1. Turn on each of these flags one-by-one, so I can verify if all the flags work as usual. If any error is found, fix it. 2. Remove the unused flags. [1] http://searchfox.org/mozilla-central/rev/8cf1367dd89cc36ef8f025dfc6af6d5c086838a7/layout/generic/nsLineLayout.cpp#33-47
|
Assignee | |
Comment 1•8 years ago
|
||
Maybe we could file a followup to further refactor all these flags in the form of what we did for GECKO_BLOCK_DEBUG_FLAGS [1]. These may gain us few advantages: 1. Get these flags and codes under protection of automatic tests (DEBUG build) 2. We can turn on/off these flags at runtime by exporting environment variables [1] http://searchfox.org/mozilla-central/rev/8cf1367dd89cc36ef8f025dfc6af6d5c086838a7/layout/generic/nsBlockFrame.cpp#130-217
|
|
Assignee | |
Updated•8 years ago
|
Summary: Repair debugging flags in nsLineLayout → Repair line layout debug flags in nsLineLayout
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
|
Assignee | |
Comment 5•8 years ago
|
||
| mozreview-review | ||
Comment on attachment 8804763 [details] Bug 1313083 - Fix NOISY_BLOCKDIR_ALIGN line layout debug flag. https://reviewboard.mozilla.org/r/88652/#review87710 Hi Ting-Yu, could you help review these patches? Please let me know if you're not comfortable with reviewing them, I could forward the requests to other candidates then.
|
|
Assignee | |
Comment 6•8 years ago
|
||
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=051b1d3d78e4
|
|
Assignee | |
Comment 7•8 years ago
|
||
(In reply to Jeremy Chen [:jeremychen] UTC+8 from comment #6) > try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=051b1d3d78e4 Looks like huge amount of logs were printed, which causes either test timeout or log file size exceeding limits. Since all the build test are green, we shall be fine I think.
|
||
Comment 8•8 years ago
|
||
| mozreview-review | ||
Comment on attachment 8804763 [details] Bug 1313083 - Fix NOISY_BLOCKDIR_ALIGN line layout debug flag. https://reviewboard.mozilla.org/r/88654/#review87992
Attachment #8804763 -
Flags: review?(tlin) → review+
|
|
||
Comment 9•8 years ago
|
||
| mozreview-review | ||
Comment on attachment 8804764 [details] Bug 1313083 - Fix REALLY_NOISY_REFLOW line layout debug flag. https://reviewboard.mozilla.org/r/88656/#review87994
Attachment #8804764 -
Flags: review?(tlin) → review+
|
|
||
Comment 10•8 years ago
|
||
| mozreview-review | ||
Comment on attachment 8804765 [details] Bug 1313083 - Remove unused line layout debug flags. https://reviewboard.mozilla.org/r/88658/#review87996 These flags do not appear to be used in nsLineLayout.cpp. Nice.
Attachment #8804765 -
Flags: review?(tlin) → review+
|
|
||
Comment 11•8 years ago
|
||
> Hi Ting-Yu, could you help review these patches? Please let me know if
> you're not comfortable with reviewing them, I could forward the requests to
> other candidates then.
I'm happy to review these, but I'm not officially a layout peer. There patch fixed only build errors when certain debug flags are defined, so should be relative safe for me to review.|
|
Assignee | |
Comment 12•8 years ago
|
||
(In reply to Ting-Yu Lin [:TYLin] (UTC+8) from comment #11) > > Hi Ting-Yu, could you help review these patches? Please let me know if > > you're not comfortable with reviewing them, I could forward the requests to > > other candidates then. > > I'm happy to review these, but I'm not officially a layout peer. There patch > fixed only build errors when certain debug flags are defined, so should be > relative safe for me to review. Yeah, I think so too. Thank you for the review. :-)
|
||
Comment 13•8 years ago
|
||
Pushed by jichen@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e29de31acba0 Fix NOISY_BLOCKDIR_ALIGN line layout debug flag. r=TYLin https://hg.mozilla.org/integration/autoland/rev/c4b900415af6 Fix REALLY_NOISY_REFLOW line layout debug flag. r=TYLin https://hg.mozilla.org/integration/autoland/rev/9b93581a04ef Remove unused line layout debug flags. r=TYLin
|
||
Comment 14•8 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/e29de31acba0 https://hg.mozilla.org/mozilla-central/rev/c4b900415af6 https://hg.mozilla.org/mozilla-central/rev/9b93581a04ef
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox52:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in
before you can comment on or make changes to this bug.
Description
•