Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(429)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 184023003: Make InlineBox::renderer() and related subclass methods return reference. (Closed)

Created:
6 years, 9 months ago by ostap
Modified:
6 years, 9 months ago
Reviewers:
Inactive, rwlbuis
CC:
blink-reviews, ed+blinkwatch_opera.com, krit, gyuyoung.kim_webkit.org, zoltan1, dsinclair, sof, eae+blinkwatch, leviw+renderwatch, fs, dglazkov+blink, f(malita), adamk+blink_chromium.org, jchaffraix+rendering, Inactive, bemjb+rendering_chromium.org, pdr., Stephen Chennney, esprehn
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

Make InlineBox::renderer() and related subclass methods return reference. It always created like InlineBox(*this) so it always has a renderer. Also removes NULL checks and asserts which are not longer required. BUG= Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=168237

Patch Set 1 #

Total comments: 49

Patch Set 2 : Updated by review comments. #

Total comments: 4

Patch Set 3 : Updated by review comments. #

Patch Set 4 : Move m_renderer on its old place to avoid InlineBox object size increase. #

Total comments: 2

Patch Set 5 : No reference reassignment. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+558 lines, -595 lines) Patch
M Source/core/editing/RenderedPosition.cpp View 6 chunks +6 lines, -6 lines 0 comments Download
M Source/core/editing/VisiblePosition.cpp View 1 2 3 4 13 chunks +17 lines, -17 lines 0 comments Download
M Source/core/editing/VisibleUnits.cpp View 1 10 chunks +20 lines, -28 lines 0 comments Download
M Source/core/rendering/EllipsisBox.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/rendering/EllipsisBox.cpp View 9 chunks +20 lines, -20 lines 0 comments Download
M Source/core/rendering/InlineBox.h View 1 2 3 6 chunks +8 lines, -9 lines 0 comments Download
M Source/core/rendering/InlineBox.cpp View 1 9 chunks +42 lines, -44 lines 0 comments Download
M Source/core/rendering/InlineFlowBox.h View 5 chunks +6 lines, -6 lines 0 comments Download
M Source/core/rendering/InlineFlowBox.cpp View 1 55 chunks +138 lines, -134 lines 0 comments Download
M Source/core/rendering/InlineTextBox.h View 3 chunks +3 lines, -3 lines 0 comments Download
M Source/core/rendering/InlineTextBox.cpp View 1 44 chunks +86 lines, -94 lines 0 comments Download
M Source/core/rendering/RenderBlock.cpp View 2 chunks +6 lines, -6 lines 0 comments Download
M Source/core/rendering/RenderBlockFlow.cpp View 1 1 chunk +1 line, -1 line 0 comments Download
M Source/core/rendering/RenderBlockLineLayout.cpp View 1 2 3 4 3 chunks +4 lines, -4 lines 0 comments Download
M Source/core/rendering/RenderBox.cpp View 2 chunks +2 lines, -2 lines 0 comments Download
M Source/core/rendering/RenderDeprecatedFlexibleBox.cpp View 1 2 chunks +8 lines, -8 lines 0 comments Download
M Source/core/rendering/RenderInline.cpp View 4 chunks +4 lines, -4 lines 0 comments Download
M Source/core/rendering/RenderListMarker.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/core/rendering/RenderObject.h View 1 2 3 4 1 chunk +8 lines, -0 lines 0 comments Download
M Source/core/rendering/RenderObject.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/rendering/RenderReplaced.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/core/rendering/RenderText.cpp View 4 chunks +5 lines, -6 lines 0 comments Download
M Source/core/rendering/RootInlineBox.h View 2 chunks +2 lines, -2 lines 0 comments Download
M Source/core/rendering/RootInlineBox.cpp View 27 chunks +76 lines, -74 lines 0 comments Download
M Source/core/rendering/TrailingFloatsRootInlineBox.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/rendering/svg/RenderSVGInline.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/rendering/svg/RenderSVGInlineText.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/rendering/svg/RenderSVGText.cpp View 2 chunks +2 lines, -2 lines 0 comments Download
M Source/core/rendering/svg/SVGInlineFlowBox.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/rendering/svg/SVGInlineFlowBox.cpp View 1 chunk +1 line, -4 lines 0 comments Download
M Source/core/rendering/svg/SVGInlineTextBox.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/rendering/svg/SVGInlineTextBox.cpp View 1 20 chunks +50 lines, -67 lines 0 comments Download
M Source/core/rendering/svg/SVGRenderTreeAsText.cpp View 1 2 1 chunk +3 lines, -4 lines 0 comments Download
M Source/core/rendering/svg/SVGRootInlineBox.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/rendering/svg/SVGRootInlineBox.cpp View 1 9 chunks +17 lines, -24 lines 0 comments Download
M Source/core/rendering/svg/SVGTextChunkBuilder.cpp View 2 chunks +3 lines, -4 lines 0 comments Download
M Source/core/rendering/svg/SVGTextLayoutEngine.cpp View 1 1 chunk +6 lines, -7 lines 0 comments Download
M Source/core/rendering/svg/SVGTextQuery.cpp View 2 chunks +2 lines, -3 lines 0 comments Download

Messages

Total messages: 28 (0 generated)
ostap
6 years, 9 months ago (2014-02-28 01:38:24 UTC) #1
Inactive
Looks like we often compare renderer addresses. We may want to overload the operator==() for ...
6 years, 9 months ago (2014-02-28 02:28:18 UTC) #2
Inactive
Also, please run the try bots when you reupload.
6 years, 9 months ago (2014-02-28 02:28:55 UTC) #3
rwlbuis
On 2014/02/28 02:28:55, Chris Dumez wrote: > Also, please run the try bots when you ...
6 years, 9 months ago (2014-02-28 02:35:17 UTC) #4
ostap
https://codereview.chromium.org/184023003/diff/1/Source/core/editing/VisibleUnits.cpp File Source/core/editing/VisibleUnits.cpp (right): https://codereview.chromium.org/184023003/diff/1/Source/core/editing/VisibleUnits.cpp#newcode902 Source/core/editing/VisibleUnits.cpp:902: RenderBlockFlow* containingBlock = &root->block(); On 2014/02/28 02:28:19, Chris Dumez ...
6 years, 9 months ago (2014-02-28 08:29:13 UTC) #5
Inactive
https://codereview.chromium.org/184023003/diff/20001/Source/core/rendering/RenderObject.h File Source/core/rendering/RenderObject.h (right): https://codereview.chromium.org/184023003/diff/20001/Source/core/rendering/RenderObject.h#newcode1227 Source/core/rendering/RenderObject.h:1227: inline bool operator==(const PassRefPtr<RenderObject>& a, const RenderObject& b) { ...
6 years, 9 months ago (2014-02-28 14:16:42 UTC) #6
ostap
https://codereview.chromium.org/184023003/diff/20001/Source/core/rendering/RenderObject.h File Source/core/rendering/RenderObject.h (right): https://codereview.chromium.org/184023003/diff/20001/Source/core/rendering/RenderObject.h#newcode1227 Source/core/rendering/RenderObject.h:1227: inline bool operator==(const PassRefPtr<RenderObject>& a, const RenderObject& b) { ...
6 years, 9 months ago (2014-02-28 15:37:22 UTC) #7
Inactive
lgtm
6 years, 9 months ago (2014-02-28 15:39:53 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sl.ostapenko@samsung.com/184023003/40001
6 years, 9 months ago (2014-02-28 15:40:24 UTC) #9
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-02-28 16:45:19 UTC) #10
commit-bot: I haz the power
Retried try job too often on linux_blink_rel for step(s) blink_heap_unittests, blink_platform_unittests, webkit_lint, webkit_python_tests, webkit_tests, webkit_unit_tests, ...
6 years, 9 months ago (2014-02-28 16:45:20 UTC) #11
ostap
The CQ bit was checked by sl.ostapenko@samsung.com
6 years, 9 months ago (2014-02-28 17:35:07 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sl.ostapenko@samsung.com/184023003/60001
6 years, 9 months ago (2014-02-28 17:35:16 UTC) #13
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-02-28 19:17:45 UTC) #14
commit-bot: I haz the power
Retried try job too often on mac_blink_rel for step(s) blink_heap_unittests, blink_platform_unittests, webkit_tests, webkit_unit_tests, wtf_unittests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_blink_rel&number=20539
6 years, 9 months ago (2014-02-28 19:17:45 UTC) #15
Inactive
https://codereview.chromium.org/184023003/diff/60001/Source/core/editing/VisiblePosition.cpp File Source/core/editing/VisiblePosition.cpp (right): https://codereview.chromium.org/184023003/diff/60001/Source/core/editing/VisiblePosition.cpp#newcode148 Source/core/editing/VisiblePosition.cpp:148: renderer = box->renderer(); not lgtm, you are reassigning a ...
6 years, 9 months ago (2014-02-28 20:26:45 UTC) #16
ostap
https://codereview.chromium.org/184023003/diff/60001/Source/core/editing/VisiblePosition.cpp File Source/core/editing/VisiblePosition.cpp (right): https://codereview.chromium.org/184023003/diff/60001/Source/core/editing/VisiblePosition.cpp#newcode148 Source/core/editing/VisiblePosition.cpp:148: renderer = box->renderer(); On 2014/02/28 20:26:45, Chris Dumez wrote: ...
6 years, 9 months ago (2014-02-28 20:41:37 UTC) #17
Inactive
lgtm
6 years, 9 months ago (2014-02-28 20:50:47 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sl.ostapenko@samsung.com/184023003/80001
6 years, 9 months ago (2014-02-28 20:50:58 UTC) #19
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-02-28 22:08:45 UTC) #20
commit-bot: I haz the power
Retried try job too often on blink_android_compile_dbg for step(s) webkit_lint http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=blink_android_compile_dbg&number=1042
6 years, 9 months ago (2014-02-28 22:08:45 UTC) #21
ostap
The CQ bit was checked by sl.ostapenko@samsung.com
6 years, 9 months ago (2014-02-28 22:40:54 UTC) #22
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sl.ostapenko@samsung.com/184023003/80001
6 years, 9 months ago (2014-02-28 22:41:00 UTC) #23
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-02-28 23:49:18 UTC) #24
commit-bot: I haz the power
Retried try job too often on blink_android_compile_rel for step(s) webkit_lint http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=blink_android_compile_rel&number=1126
6 years, 9 months ago (2014-02-28 23:49:19 UTC) #25
ostap
The CQ bit was checked by sl.ostapenko@samsung.com
6 years, 9 months ago (2014-03-01 04:06:25 UTC) #26
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sl.ostapenko@samsung.com/184023003/80001
6 years, 9 months ago (2014-03-01 04:06:38 UTC) #27
commit-bot: I haz the power
6 years, 9 months ago (2014-03-01 04:07:34 UTC) #28
Message was sent while issue was closed. 
Change committed as 168237

Powered by Google App Engine
This is Rietveld 408576698