Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2846563002:
Optimize collapsed border calculation (step 1) (Closed)
|
Created:
3 years, 8 months ago by Xianzhu Modified:
3 years, 7 months ago CC:
chromium-reviews, pdr+renderingwatchlist_chromium.org, blink-reviews-style_chromium.org, zoltan1, blink-reviews-layout_chromium.org, szager+layoutwatch_chromium.org, eae+blinkwatch, leviw+renderwatch, jchaffraix+rendering, blink-reviews Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionOptimize collapsed border calculation (step 1)
Previously we didn't cache results of collapsed borders computation
during layout or overflow recalculation, so we had to recompute all
collapsed borders before paint invalidation if the table's collapsed
borders are marked invalid.
Now when a table's collapsed borders need to be invalidated, mark all
cells' collapsed borders need to be invalidated [1]. When a cell needs
its collapsed border values (regardless of lifecycle phase), we check
the invalidation flag and update and cache the result when necessary.
As LayoutTableCell::CollapsedBorderValues will be used for both layout
and paint, the include_color paremeter of ComputeCollapsedXXXBorder()
is removed.
As adjacent cells with the same span share collapsed borders, we can
also get the collapsed border from the adjacent cell if its collapsed
borders are valid.
[1] In the next step, we'll only invalidate collapsed borders for
affected cells instead of all cells.
BUG=626748, 663208
Review-Url: https://codereview.chromium.org/2846563002
Cr-Commit-Position: refs/heads/master@{#468174}
Committed: https://chromium.googlesource.com/chromium/src/+/c731464f86c6a5c5c3f2b64890e04b38750896fc
Patch Set 1 #Patch Set 2 : - #Patch Set 3 : - #
Total comments: 19
Patch Set 4 : - #
Messages
Total messages: 34 (24 generated)
The CQ bit was checked by wangxianzhu@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
The CQ bit was checked by wangxianzhu@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by wangxianzhu@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
wangxianzhu@chromium.org changed reviewers: + dgrogan@chromium.org, robhogan@gmail.com, wkorman@chromium.org
lgtm https://codereview.chromium.org/2846563002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutTable.cpp (right): https://codereview.chromium.org/2846563002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutTable.cpp:789: DCHECK(CollapseBorders()); I was briefly confused with this vs. CollapsedBorders(). We should revise one or the other method name to make clearer the difference (former checks whether style is set, latter returns the collapsed border values instance, and differ by only one char amidst name). https://codereview.chromium.org/2846563002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutTable.h (right): https://codereview.chromium.org/2846563002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutTable.h:419: void InvalidateCollapsedBordersForAllCellsIfNeeded(); nit: believe we're not using 'IfNeeded' suffix any more as it's usually implicit. https://codereview.chromium.org/2846563002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutTableCell.cpp (left): https://codereview.chromium.org/2846563002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutTableCell.cpp:1138: include_color ? enclosing_column_group->ResolveColor( Perhaps worth mentioning this color change in CL description as well, as you did for https://codereview.chromium.org/2838643003 https://codereview.chromium.org/2846563002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutTableCell.cpp (right): https://codereview.chromium.org/2846563002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutTableCell.cpp:604: // We can use the border shared with cellBefore if it is valid. cellBefore -> |cell_before| https://codereview.chromium.org/2846563002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutTableCell.cpp:749: int start_color_property = CSSProperty::ResolveDirectionAwareProperty( We do similar to these 3 lines about 8 times in this file, perhaps worth shortening via static helper function or similar. https://codereview.chromium.org/2846563002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutTableCell.cpp:866: // We can use the border shared with prevCell if it is valid. prevCell -> |prev_cell| https://codereview.chromium.org/2846563002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutTableCell.cpp:1264: void LayoutTableCell::RecalcCollapsedBorderValuesIfNeeded() const { Same as previous -- believe prefer without ...IfNeeded suffix. https://codereview.chromium.org/2846563002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutTableCell.h (right): https://codereview.chromium.org/2846563002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutTableCell.h:38: static const unsigned kUnsetColumnIndex = 0x07FFFFFF; How are these numbers here and below determined and why do they change as part of this patch? Worth adding comment for future readers as well. https://codereview.chromium.org/2846563002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutTableCell.h:332: collapsed_border_values_valid_ = false; When are they valid/invalid? Worth documenting in a comment somewhere, perhaps on the member data? I wondered same re: visually_changed member data.
Description was changed from ========== Optimize collapsed border calculation (step 1) Previously we didn't cache results of collapsed borders computation during layout or overflow recalculation, so we had to recompute all collapsed borders before paint invalidation if the table's collapsed borders are marked invalid. Now when a table's collapsed borders need to be invalidated, mark all cells' collapsed borders need to be invalidated [1]. When a cell needs its collapsed border values (regardless of lifecycle phase), we check the invalidation flag and update and cache the result when necessary. As adjacent cells with the same span share collapsed borders, we can also get the collapsed border from the adjacent cell if its collapsed borders are valid. [1] In the next step, we'll only invalidate collapsed borders for affected cells instead of all cells. BUG=626748,663208 ========== to ========== Optimize collapsed border calculation (step 1) Previously we didn't cache results of collapsed borders computation during layout or overflow recalculation, so we had to recompute all collapsed borders before paint invalidation if the table's collapsed borders are marked invalid. Now when a table's collapsed borders need to be invalidated, mark all cells' collapsed borders need to be invalidated [1]. When a cell needs its collapsed border values (regardless of lifecycle phase), we check the invalidation flag and update and cache the result when necessary. As LayoutTableCell::CollapsedBorderValues will be used for both layout and paint, the include_color paremeter of ComputeCollapsedXXXBorder() is removed. As adjacent cells with the same span share collapsed borders, we can also get the collapsed border from the adjacent cell if its collapsed borders are valid. [1] In the next step, we'll only invalidate collapsed borders for affected cells instead of all cells. BUG=626748,663208 ==========
The CQ bit was checked by wangxianzhu@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2846563002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutTable.cpp (right): https://codereview.chromium.org/2846563002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutTable.cpp:789: DCHECK(CollapseBorders()); On 2017/04/27 20:11:15, wkorman wrote: > I was briefly confused with this vs. CollapsedBorders(). We should revise one or > the other method name to make clearer the difference (former checks whether > style is set, latter returns the collapsed border values instance, and differ by > only one char amidst name). Created https://codereview.chromium.org/2850633003/ to rename it. https://codereview.chromium.org/2846563002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutTable.h (right): https://codereview.chromium.org/2846563002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutTable.h:419: void InvalidateCollapsedBordersForAllCellsIfNeeded(); On 2017/04/27 20:11:15, wkorman wrote: > nit: believe we're not using 'IfNeeded' suffix any more as it's usually > implicit. This "IfNeeded" is meaningful because the function invalidate all cells only if needs_invalidate_collapsed_borders_for_all_cells_ is true. Removing "IfNeeded" will imply we always invalidate all cells which is totally different from what the function will do. https://codereview.chromium.org/2846563002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutTableCell.cpp (left): https://codereview.chromium.org/2846563002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutTableCell.cpp:1138: include_color ? enclosing_column_group->ResolveColor( On 2017/04/27 20:11:15, wkorman wrote: > Perhaps worth mentioning this color change in CL description as well, as you did > for https://codereview.chromium.org/2838643003 Done. https://codereview.chromium.org/2846563002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutTableCell.cpp (right): https://codereview.chromium.org/2846563002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutTableCell.cpp:604: // We can use the border shared with cellBefore if it is valid. On 2017/04/27 20:11:15, wkorman wrote: > cellBefore -> |cell_before| Done. https://codereview.chromium.org/2846563002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutTableCell.cpp:749: int start_color_property = CSSProperty::ResolveDirectionAwareProperty( On 2017/04/27 20:11:15, wkorman wrote: > We do similar to these 3 lines about 8 times in this file, perhaps worth > shortening via static helper function or similar. Done. https://codereview.chromium.org/2846563002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutTableCell.cpp:866: // We can use the border shared with prevCell if it is valid. On 2017/04/27 20:11:15, wkorman wrote: > prevCell -> |prev_cell| Done. https://codereview.chromium.org/2846563002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutTableCell.cpp:1264: void LayoutTableCell::RecalcCollapsedBorderValuesIfNeeded() const { On 2017/04/27 20:11:15, wkorman wrote: > Same as previous -- believe prefer without ...IfNeeded suffix. Renamed to UpdateCollapsedBorderValues(). Simply removing IfNeeded may confuse callers because "Recalc" means always "re-calculate". https://codereview.chromium.org/2846563002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutTableCell.h (right): https://codereview.chromium.org/2846563002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutTableCell.h:38: static const unsigned kUnsetColumnIndex = 0x07FFFFFF; On 2017/04/27 20:11:15, wkorman wrote: > How are these numbers here and below determined and why do they change as part > of this patch? Worth adding comment for future readers as well. Changed to self-explanation code: #define BITS_OF_ABSOLUTE_COLUMN_INDEX 27 static const unsigned kUnsetColumnIndex = (1u << BITS_OF_ABSOLUTE_COLUMN_INDEX) - 1; static const unsigned kMaxColumnIndex = kUnsetColumnIndex - 1; and in LayoutTableCell: unsigned absolute_column_index_ : BITS_OF_ABSOLUTE_COLUMN_INDEX; // When adding or removing bits here, we should also adjust // BITS_OF_ABSOLUTE_COLUMN_INDEX to use remaining bits of a 32-bit word. add added a test LayoutTableCellTest.AbsoluteColumnIndex. https://codereview.chromium.org/2846563002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutTableCell.h:332: collapsed_border_values_valid_ = false; On 2017/04/27 20:11:15, wkorman wrote: > When are they valid/invalid? Worth documenting in a comment somewhere, perhaps > on the member data? I wondered same re: visually_changed member data. Done.
The CQ bit was checked by wangxianzhu@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2846563002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutTable.h (right): https://codereview.chromium.org/2846563002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutTable.h:419: void InvalidateCollapsedBordersForAllCellsIfNeeded(); On 2017/04/28 20:31:44, Xianzhu wrote: > On 2017/04/27 20:11:15, wkorman wrote: > > nit: believe we're not using 'IfNeeded' suffix any more as it's usually > > implicit. > > This "IfNeeded" is meaningful because the function invalidate all cells only if > needs_invalidate_collapsed_borders_for_all_cells_ is true. Removing "IfNeeded" > will imply we always invalidate all cells which is totally different from what > the function will do. > Am fine with this. But for general discussion, I think this is debatable. We have dirty flags for many things, and we have methods that only enact logic if/when/until the flag is set, and not all of those methods are named IfNeeded. Maybe you are proposing, if methods give explicit consideration to dirty state that is itself named need_xxx, then it is worth naming methods with IfNeeded suffix? But, today it seems that it is often implicit that the method will try to be smart, and the manner in which it is smart can be opaque to external callers. See for example the names of a wide range of methods with early-outs in LayoutObject (search for "return;") Or, the many paint invalidation flags on LayoutObject checked in Invalidator methods that always enact some logic. PaintInvalidator::InvalidatePaint always calls UpdatePaintingLayer, but conditionally does other logic, bailing on !object.ShouldCheckForPaintInvalidation or !(context.forced_subtree_invalidation_flags & ~PaintInvalidatorContext::kForcedSubtreeVisualRectUpdate).
On 2017/04/28 21:40:15, wkorman wrote: > https://codereview.chromium.org/2846563002/diff/40001/third_party/WebKit/Sour... > File third_party/WebKit/Source/core/layout/LayoutTable.h (right): > > https://codereview.chromium.org/2846563002/diff/40001/third_party/WebKit/Sour... > third_party/WebKit/Source/core/layout/LayoutTable.h:419: void > InvalidateCollapsedBordersForAllCellsIfNeeded(); > On 2017/04/28 20:31:44, Xianzhu wrote: > > On 2017/04/27 20:11:15, wkorman wrote: > > > nit: believe we're not using 'IfNeeded' suffix any more as it's usually > > > implicit. > > > > This "IfNeeded" is meaningful because the function invalidate all cells only > if > > needs_invalidate_collapsed_borders_for_all_cells_ is true. Removing "IfNeeded" > > will imply we always invalidate all cells which is totally different from what > > the function will do. > > > > Am fine with this. > > But for general discussion, I think this is debatable. We have dirty flags for > many things, and we have methods that only enact logic if/when/until the flag is > set, and not all of those methods are named IfNeeded. > > Maybe you are proposing, if methods give explicit consideration to dirty state > that is itself named need_xxx, then it is worth naming methods with IfNeeded > suffix? > > But, today it seems that it is often implicit that the method will try to be > smart, and the manner in which it is smart can be opaque to external callers. > > See for example the names of a wide range of methods with early-outs in > LayoutObject (search for "return;") > > Or, the many paint invalidation flags on LayoutObject checked in Invalidator > methods that always enact some logic. PaintInvalidator::InvalidatePaint always > calls UpdatePaintingLayer, but conditionally does other logic, bailing on > !object.ShouldCheckForPaintInvalidation or > !(context.forced_subtree_invalidation_flags & > ~PaintInvalidatorContext::kForcedSubtreeVisualRectUpdate). I understand that we don't need "IfNeeded" in cases like: void UpdateSomethingIfNeeded() { if (SomethingIsDirty()) UpdateSomething(); } The checking of SomethingIsDirty() and early return is for performance optimization, and unconditionally doing UpdateSomething() won't harm functionally. However, in cases like: void DestroySomethingIfNeeded() { if (NeedsToDestroySomething()) DestroySomething(); } the meanings of DestroySomethingIfNeeded() and DestroySomething() is totally different. The InvalidateCollapsedBordersOfAllCellsIfNeeded() method is of the latter case. The flag needs_invalidate_collapsed_borders_of_all_cells_ doesn't flag dirty status, but flags a task to do. The method invalidates collapsed borders only if we need to do the task. Otherwise it will do nothing even if there are cells with valid collapsed borders. This is different from InvalidateCollapsedBordersOfAllCells().
On 2017/04/28 21:59:53, Xianzhu wrote:
> However, in cases like:
> void DestroySomethingIfNeeded() {
> if (NeedsToDestroySomething())
> DestroySomething();
> }
> the meanings of DestroySomethingIfNeeded() and DestroySomething() is totally
> different.
>
> The InvalidateCollapsedBordersOfAllCellsIfNeeded() method is of the latter
case.
> The flag needs_invalidate_collapsed_borders_of_all_cells_ doesn't flag dirty
> status, but flags a task to do. The method invalidates collapsed borders only
if
> we need to do the task. Otherwise it will do nothing even if there are cells
> with valid collapsed borders. This is different from
> InvalidateCollapsedBordersOfAllCells().
I see what you are saying. What prompts the need for two methods, versus just
having one DestroySomething() method that just always checks
NeedsToDestroySomething() at the beginning?
I can see:
1. There are some callsites where we want to DestroySomething() regardless of
the needs-flag-state.
2. We want to avoid overhead of the add'l conditional check sometimes (seems
unlikely/tiny).
On 2017/04/28 22:06:00, wkorman wrote:
> On 2017/04/28 21:59:53, Xianzhu wrote:
> > However, in cases like:
> > void DestroySomethingIfNeeded() {
> > if (NeedsToDestroySomething())
> > DestroySomething();
> > }
> > the meanings of DestroySomethingIfNeeded() and DestroySomething() is totally
> > different.
> >
> > The InvalidateCollapsedBordersOfAllCellsIfNeeded() method is of the latter
> case.
> > The flag needs_invalidate_collapsed_borders_of_all_cells_ doesn't flag dirty
> > status, but flags a task to do. The method invalidates collapsed borders
only
> if
> > we need to do the task. Otherwise it will do nothing even if there are cells
> > with valid collapsed borders. This is different from
> > InvalidateCollapsedBordersOfAllCells().
>
> I see what you are saying. What prompts the need for two methods, versus just
> having one DestroySomething() method that just always checks
> NeedsToDestroySomething() at the beginning?
>
> I can see:
>
> 1. There are some callsites where we want to DestroySomething() regardless of
> the needs-flag-state.
>
> 2. We want to avoid overhead of the add'l conditional check sometimes (seems
> unlikely/tiny).
No. The example of the two names just shows what the functions mean, not
necessarily to have two functions. I was saying that DestroySomethingIfNeeded()
can't be named DestroySomething() because they mean totally different things.
On 2017/04/28 22:06:00, wkorman wrote:
> On 2017/04/28 21:59:53, Xianzhu wrote:
> > However, in cases like:
> > void DestroySomethingIfNeeded() {
> > if (NeedsToDestroySomething())
> > DestroySomething();
> > }
> > the meanings of DestroySomethingIfNeeded() and DestroySomething() is totally
> > different.
> >
> > The InvalidateCollapsedBordersOfAllCellsIfNeeded() method is of the latter
> case.
> > The flag needs_invalidate_collapsed_borders_of_all_cells_ doesn't flag dirty
> > status, but flags a task to do. The method invalidates collapsed borders
only
> if
> > we need to do the task. Otherwise it will do nothing even if there are cells
> > with valid collapsed borders. This is different from
> > InvalidateCollapsedBordersOfAllCells().
>
> I see what you are saying. What prompts the need for two methods, versus just
> having one DestroySomething() method that just always checks
> NeedsToDestroySomething() at the beginning?
>
> I can see:
>
> 1. There are some callsites where we want to DestroySomething() regardless of
> the needs-flag-state.
>
> 2. We want to avoid overhead of the add'l conditional check sometimes (seems
> unlikely/tiny).
I re-read and sort of see what you are saying, re:
"needs_invalidate_collapsed_borders_of_all_cells_ doesn't flag dirty
status, but flags a task to do".
May be easiest to discuss further in person next time we have opportunity.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by wangxianzhu@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from wkorman@chromium.org Link to the patchset: https://codereview.chromium.org/2846563002/#ps60001 (title: "-")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 60001, "attempt_start_ts": 1493420906376370,
"parent_rev": "43d9f387a0febd1c7b5ea74b121a6e485638cbdf", "commit_rev":
"c731464f86c6a5c5c3f2b64890e04b38750896fc"}
Message was sent while issue was closed.
Description was changed from ========== Optimize collapsed border calculation (step 1) Previously we didn't cache results of collapsed borders computation during layout or overflow recalculation, so we had to recompute all collapsed borders before paint invalidation if the table's collapsed borders are marked invalid. Now when a table's collapsed borders need to be invalidated, mark all cells' collapsed borders need to be invalidated [1]. When a cell needs its collapsed border values (regardless of lifecycle phase), we check the invalidation flag and update and cache the result when necessary. As LayoutTableCell::CollapsedBorderValues will be used for both layout and paint, the include_color paremeter of ComputeCollapsedXXXBorder() is removed. As adjacent cells with the same span share collapsed borders, we can also get the collapsed border from the adjacent cell if its collapsed borders are valid. [1] In the next step, we'll only invalidate collapsed borders for affected cells instead of all cells. BUG=626748,663208 ========== to ========== Optimize collapsed border calculation (step 1) Previously we didn't cache results of collapsed borders computation during layout or overflow recalculation, so we had to recompute all collapsed borders before paint invalidation if the table's collapsed borders are marked invalid. Now when a table's collapsed borders need to be invalidated, mark all cells' collapsed borders need to be invalidated [1]. When a cell needs its collapsed border values (regardless of lifecycle phase), we check the invalidation flag and update and cache the result when necessary. As LayoutTableCell::CollapsedBorderValues will be used for both layout and paint, the include_color paremeter of ComputeCollapsedXXXBorder() is removed. As adjacent cells with the same span share collapsed borders, we can also get the collapsed border from the adjacent cell if its collapsed borders are valid. [1] In the next step, we'll only invalidate collapsed borders for affected cells instead of all cells. BUG=626748,663208 Review-Url: https://codereview.chromium.org/2846563002 Cr-Commit-Position: refs/heads/master@{#468174} Committed: https://chromium.googlesource.com/chromium/src/+/c731464f86c6a5c5c3f2b64890e0... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://chromium.googlesource.com/chromium/src/+/c731464f86c6a5c5c3f2b64890e0... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
