Ewann Pellé (Gerrit)

unread,

3:36 AM (19 hours ago) 3:36 AM

to Min Xiong, Mark Cogan, AyeAye, chromium...@chromium.org, dtraino...@chromium.org, feature-me...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org

Attention needed from Mark Cogan and Min Xiong

Ewann Pellé voted and added 1 comment

Votes added by Ewann Pellé

Code-Review

+1

1 comment

File ios/chrome/browser/download/ui/download_list/download_list_table_view_controller.mm

Line 239, Patchset 3 (Latest): performPrimaryActionForRowAtIndexPath:(NSIndexPath*)indexPath {

Ewann Pellé . unresolved

nit for a future CL: You probably also want to support the swipe to delete gesture.

Open in Gerrit

Related details

Attention is currently required from:

Mark Cogan
Min Xiong

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Min Xiong (Gerrit)

unread,

3:38 AM (19 hours ago) 3:38 AM

to Ewann Pellé, Mark Cogan, AyeAye, chromium...@chromium.org, dtraino...@chromium.org, feature-me...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org

Attention needed from Mark Cogan

Min Xiong added 1 comment

File ios/chrome/browser/download/ui/download_list/download_list_table_view_controller.mm

Line 239, Patchset 3 (Latest): performPrimaryActionForRowAtIndexPath:(NSIndexPath*)indexPath {

Ewann Pellé . resolved

nit for a future CL: You probably also want to support the swipe to delete gesture.

Min Xiong

Get, I will add the action in the next CL

Open in Gerrit

Related details

Attention is currently required from:

Mark Cogan

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Min Xiong (Gerrit)

unread,

3:44 AM (19 hours ago) 3:44 AM

to Ewann Pellé, Mark Cogan, AyeAye, chromium...@chromium.org, dtraino...@chromium.org, feature-me...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org

Attention needed from Mark Cogan

Min Xiong added 1 comment

Patchset-level comments

File-level comment, Patchset 3 (Latest):

Min Xiong . unresolved

Now I ma investigating on the bug: 457710580, I found that `TableViewCellContentView.applyConfiguration` 
```
 _title.text = _configuration.title;
//  _title.textColor =
//      _configuration.titleColor ?: [UIColor colorNamed:kTextPrimaryColor];
  if (_configuration.attributedTitle) {
    _title.attributedText = _configuration.attributedTitle;
  }
  _title.hidden = !_title.text;
  _title.enabled = !_configuration.textDisabled;
  _title.lineBreakMode = _configuration.titleLineBreakMode;

  _subtitle.text = _configuration.subtitle;
//  _subtitle.textColor =
//      _configuration.subtitleColor ?: [UIColor colorNamed:kTextSecondaryColor];
  if (_configuration.attributedSubtitle) {
    _subtitle.attributedText = _configuration.attributedSubtitle;
  }
  _subtitle.hidden = !_subtitle.text;
  _subtitle.enabled = !_configuration.textDisabled;
  _subtitle.lineBreakMode = _configuration.subtitleLineBreakMode;
```
The `UIColorNamed` will cause very high cpu usage (it seems that the method has been swizzled), will be over 80% in the sub tread, Anyone why

Open in Gerrit

Related details

Attention is currently required from:

Mark Cogan

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Min Xiong (Gerrit)

unread,

4:11 AM (18 hours ago) 4:11 AM

to Ewann Pellé, Mark Cogan, AyeAye, chromium...@chromium.org, dtraino...@chromium.org, feature-me...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org

Attention needed from Mark Cogan

Min Xiong added 1 comment

Patchset-level comments

File-level comment, Patchset 3 (Latest):

Min Xiong . resolved

Now I ma investigating on the bug: 457710580, I found that `TableViewCellContentView.applyConfiguration` 
```
 _title.text = _configuration.title;
//  _title.textColor =
//      _configuration.titleColor ?: [UIColor colorNamed:kTextPrimaryColor];
  if (_configuration.attributedTitle) {
    _title.attributedText = _configuration.attributedTitle;
  }
  _title.hidden = !_title.text;
  _title.enabled = !_configuration.textDisabled;
  _title.lineBreakMode = _configuration.titleLineBreakMode;

  _subtitle.text = _configuration.subtitle;
//  _subtitle.textColor =
//      _configuration.subtitleColor ?: [UIColor colorNamed:kTextSecondaryColor];
  if (_configuration.attributedSubtitle) {
    _subtitle.attributedText = _configuration.attributedSubtitle;
  }
  _subtitle.hidden = !_subtitle.text;
  _subtitle.enabled = !_configuration.textDisabled;
  _subtitle.lineBreakMode = _configuration.subtitleLineBreakMode;
```
The `UIColorNamed` will cause very high cpu usage (it seems that the method has been swizzled), will be over 80% in the sub tread, Anyone why

Min Xiong

I see that only happen on debug mode.

Open in Gerrit

Related details

Attention is currently required from:

Mark Cogan

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Mark Cogan (Gerrit)

unread,

5:30 AM (17 hours ago) 5:30 AM

to Min Xiong, Chromium LUCI CQ, Ewann Pellé, AyeAye, chromium...@chromium.org, dtraino...@chromium.org, feature-me...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org

Attention needed from Min Xiong

Mark Cogan added 1 comment

Patchset-level comments

File-level comment, Patchset 3 (Latest):

Mark Cogan . resolved

Can you add a regression test for this bug?

Open in Gerrit

Related details

Attention is currently required from:

Min Xiong

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Min Xiong (Gerrit)

unread,

9:35 AM (13 hours ago) 9:35 AM

to Chromium LUCI CQ, Ewann Pellé, Mark Cogan, AyeAye, chromium...@chromium.org, dtraino...@chromium.org, feature-me...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org

Attention needed from Ewann Pellé and Mark Cogan

Min Xiong added 2 comments

Patchset-level comments

File-level comment, Patchset 3:

Mark Cogan . resolved

Can you add a regression test for this bug?

Min Xiong

Test added.

File ios/chrome/browser/download/ui/download_list/download_list_table_view_controller.mm

Line 239, Patchset 3: performPrimaryActionForRowAtIndexPath:(NSIndexPath*)indexPath {

Ewann Pellé . resolved

nit for a future CL: You probably also want to support the swipe to delete gesture.

Min Xiong

Get, I will add the action in the next CL

Min Xiong

Swipe delete gesture added.

Open in Gerrit

Related details

Attention is currently required from:

Ewann Pellé
Mark Cogan

Submit Requirements:

Code-Coverage
Code-Owners

Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Ewann Pellé (Gerrit)

unread,

9:40 AM (13 hours ago) 9:40 AM

to Min Xiong, Chromium LUCI CQ, Mark Cogan, AyeAye, chromium...@chromium.org, dtraino...@chromium.org, feature-me...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org

Attention needed from Mark Cogan and Min Xiong

Ewann Pellé added 1 comment

File ios/chrome/browser/download/ui/download_list/download_list_table_view_controller.mm

Line 239, Patchset 3: performPrimaryActionForRowAtIndexPath:(NSIndexPath*)indexPath {

Ewann Pellé . unresolved

nit for a future CL: You probably also want to support the swipe to delete gesture.

Min Xiong

Get, I will add the action in the next CL

Min Xiong

Swipe delete gesture added.

Ewann Pellé

I think you should split this change in a separate CL.

Open in Gerrit

Related details

Attention is currently required from:

Mark Cogan
Min Xiong

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

No-Unresolved-Comments

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Min Xiong (Gerrit)

unread,

10:01 AM (12 hours ago) 10:01 AM

to Chromium LUCI CQ, Ewann Pellé, Mark Cogan, AyeAye, chromium...@chromium.org, dtraino...@chromium.org, feature-me...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org

Attention needed from Ewann Pellé and Mark Cogan

Min Xiong added 1 comment

File ios/chrome/browser/download/ui/download_list/download_list_table_view_controller.mm

Line 239, Patchset 3: performPrimaryActionForRowAtIndexPath:(NSIndexPath*)indexPath {

Ewann Pellé . resolved

nit for a future CL: You probably also want to support the swipe to delete gesture.

Min Xiong

Get, I will add the action in the next CL

Min Xiong

Swipe delete gesture added.

Ewann Pellé

I think you should split this change in a separate CL.

Min Xiong

Removed.

Open in Gerrit

Related details

Attention is currently required from:

Ewann Pellé
Mark Cogan

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Mark Cogan (Gerrit)

unread,

12:12 PM (10 hours ago) 12:12 PM

to Min Xiong, Chromium LUCI CQ, Ewann Pellé, AyeAye, chromium...@chromium.org, dtraino...@chromium.org, feature-me...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org

Attention needed from Ewann Pellé and Min Xiong

Mark Cogan voted and added 1 comment

Votes added by Mark Cogan

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 5 (Latest):

Mark Cogan . resolved

LGTM

Open in Gerrit

Related details

Attention is currently required from:

Ewann Pellé
Min Xiong

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Ewann Pellé (Gerrit)

unread,

12:13 PM (10 hours ago) 12:13 PM

to Min Xiong, Mark Cogan, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, dtraino...@chromium.org, feature-me...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org

Attention needed from Min Xiong

Ewann Pellé voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Min Xiong

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Min Xiong (Gerrit)

unread,

8:54 PM (2 hours ago) 8:54 PM

to Ewann Pellé, Mark Cogan, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, dtraino...@chromium.org, feature-me...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org

Min Xiong voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Reply all

Reply to author

Forward

[iOS] Fix the canceled task can't be removed and the file exist check. [chromium/src : main]

Ewann Pellé (Gerrit)

Ewann Pellé voted and added 1 comment

Votes added by Ewann Pellé

1 comment

Related details

Min Xiong (Gerrit)

Min Xiong added 1 comment

Related details

Min Xiong (Gerrit)

Min Xiong added 1 comment

Related details

Min Xiong (Gerrit)

Min Xiong added 1 comment

Related details

Mark Cogan (Gerrit)

Mark Cogan added 1 comment

Related details

Min Xiong (Gerrit)

Min Xiong added 2 comments

Related details

Ewann Pellé (Gerrit)

Ewann Pellé added 1 comment

Related details

Min Xiong (Gerrit)

Min Xiong added 1 comment

Related details

Mark Cogan (Gerrit)

Mark Cogan voted and added 1 comment

Votes added by Mark Cogan

1 comment

Related details

Ewann Pellé (Gerrit)

Ewann Pellé voted Code-Review+1

Related details

Min Xiong (Gerrit)

Min Xiong voted Commit-Queue+2

Related details