New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 755814 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug



Sign in to add a comment

Place Dialog Touch Bar Support Behind Finch Flag

Project Member Reported by spqc...@chromium.org, Aug 16 2017

Issue description

Dialog touch bar support is currently behind the Browser Touch Bar flag.
This needs to be separated into a separate flag that's disabled by default.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/62e55ea7afd2d153e85e1540800a9eb0d5b079a6

commit 62e55ea7afd2d153e85e1540800a9eb0d5b079a6
Author: spqchan <spqchan@chromium.org>
Date: Wed Aug 16 16:54:57 2017

Feature Flag for Dialog Touch Bar Support

Dialog touch bar support is currently behind
the Browser Touch Bar flag. This needs to be
separated into a separate flag that's disabled
by default.

Bug:  755814 
Change-Id: I8f9ca544d48548f37f48aa529123e97910e58b79
Reviewed-on: https://chromium-review.googlesource.com/616341
Commit-Queue: Sarah Chan <spqchan@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494827}
[modify] https://crrev.com/62e55ea7afd2d153e85e1540800a9eb0d5b079a6/chrome/browser/about_flags.cc
[modify] https://crrev.com/62e55ea7afd2d153e85e1540800a9eb0d5b079a6/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/62e55ea7afd2d153e85e1540800a9eb0d5b079a6/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/62e55ea7afd2d153e85e1540800a9eb0d5b079a6/chrome/browser/ui/cocoa/bookmarks/bookmark_bubble_controller.mm
[modify] https://crrev.com/62e55ea7afd2d153e85e1540800a9eb0d5b079a6/chrome/browser/ui/cocoa/bookmarks/bookmark_editor_base_controller.mm
[modify] https://crrev.com/62e55ea7afd2d153e85e1540800a9eb0d5b079a6/chrome/browser/ui/cocoa/bookmarks/bookmark_editor_controller.mm
[modify] https://crrev.com/62e55ea7afd2d153e85e1540800a9eb0d5b079a6/chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm
[modify] https://crrev.com/62e55ea7afd2d153e85e1540800a9eb0d5b079a6/chrome/common/chrome_features.cc
[modify] https://crrev.com/62e55ea7afd2d153e85e1540800a9eb0d5b079a6/chrome/common/chrome_features.h
[modify] https://crrev.com/62e55ea7afd2d153e85e1540800a9eb0d5b079a6/tools/metrics/histograms/enums.xml

Project Member

Comment 2 by bugdroid1@chromium.org, Aug 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/284f65ce7c5e28508d9383ed72a58e46db6acc1d

commit 284f65ce7c5e28508d9383ed72a58e46db6acc1d
Author: Jeremy Roman <jbroman@chromium.org>
Date: Wed Aug 16 20:13:15 2017

Revert "Feature Flag for Dialog Touch Bar Support"

This reverts commit 62e55ea7afd2d153e85e1540800a9eb0d5b079a6.

Reason for revert: Breaks ContentSettingBubbleControllerTest.TouchBar:
https://build.chromium.org/p/chromium.mac/builders/Mac10.12%20Tests/builds/3958

I suspect that the InitAndEnableFeature call in that test simply needs to be updated correspondingly, but reverting since I'm not certain and this should be easy to reland with a fix if so.

Original change's description:
> Feature Flag for Dialog Touch Bar Support
> 
> Dialog touch bar support is currently behind
> the Browser Touch Bar flag. This needs to be
> separated into a separate flag that's disabled
> by default.
> 
> Bug:  755814 
> Change-Id: I8f9ca544d48548f37f48aa529123e97910e58b79
> Reviewed-on: https://chromium-review.googlesource.com/616341
> Commit-Queue: Sarah Chan <spqchan@chromium.org>
> Reviewed-by: Robert Sesek <rsesek@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#494827}

TBR=spqchan@chromium.org,rsesek@chromium.org

Change-Id: I2b1f82cd576ad3aded383d673d9e1f52a1f0b053
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  755814 
Reviewed-on: https://chromium-review.googlesource.com/617860
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494918}
[modify] https://crrev.com/284f65ce7c5e28508d9383ed72a58e46db6acc1d/chrome/browser/about_flags.cc
[modify] https://crrev.com/284f65ce7c5e28508d9383ed72a58e46db6acc1d/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/284f65ce7c5e28508d9383ed72a58e46db6acc1d/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/284f65ce7c5e28508d9383ed72a58e46db6acc1d/chrome/browser/ui/cocoa/bookmarks/bookmark_bubble_controller.mm
[modify] https://crrev.com/284f65ce7c5e28508d9383ed72a58e46db6acc1d/chrome/browser/ui/cocoa/bookmarks/bookmark_editor_base_controller.mm
[modify] https://crrev.com/284f65ce7c5e28508d9383ed72a58e46db6acc1d/chrome/browser/ui/cocoa/bookmarks/bookmark_editor_controller.mm
[modify] https://crrev.com/284f65ce7c5e28508d9383ed72a58e46db6acc1d/chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm
[modify] https://crrev.com/284f65ce7c5e28508d9383ed72a58e46db6acc1d/chrome/common/chrome_features.cc
[modify] https://crrev.com/284f65ce7c5e28508d9383ed72a58e46db6acc1d/chrome/common/chrome_features.h
[modify] https://crrev.com/284f65ce7c5e28508d9383ed72a58e46db6acc1d/tools/metrics/histograms/enums.xml

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/eb1b3b1a757988622cab119a5f95b118c3338591

commit eb1b3b1a757988622cab119a5f95b118c3338591
Author: spqchan <spqchan@chromium.org>
Date: Thu Aug 17 17:11:31 2017

Feature Flag for Dialog Touch Bar Support

Dialog touch bar support is currently behind
the Browser Touch Bar flag. This needs to be
separated into a separate flag that's disabled
by default.

This is an attempt to reland
https://chromium-review.googlesource.com/c/616341
which had caused several tests to failed. The test
failures are addressed in this CL

Bug:  755814 
Change-Id: I458527a859527b8ebcccca5b12ca9f55b4d39b2c
Reviewed-on: https://chromium-review.googlesource.com/617386
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Sarah Chan <spqchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495203}
[modify] https://crrev.com/eb1b3b1a757988622cab119a5f95b118c3338591/chrome/browser/about_flags.cc
[modify] https://crrev.com/eb1b3b1a757988622cab119a5f95b118c3338591/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/eb1b3b1a757988622cab119a5f95b118c3338591/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/eb1b3b1a757988622cab119a5f95b118c3338591/chrome/browser/ui/cocoa/bookmarks/bookmark_bubble_controller.mm
[modify] https://crrev.com/eb1b3b1a757988622cab119a5f95b118c3338591/chrome/browser/ui/cocoa/bookmarks/bookmark_bubble_controller_unittest.mm
[modify] https://crrev.com/eb1b3b1a757988622cab119a5f95b118c3338591/chrome/browser/ui/cocoa/bookmarks/bookmark_editor_base_controller.mm
[modify] https://crrev.com/eb1b3b1a757988622cab119a5f95b118c3338591/chrome/browser/ui/cocoa/bookmarks/bookmark_editor_base_controller_unittest.mm
[modify] https://crrev.com/eb1b3b1a757988622cab119a5f95b118c3338591/chrome/browser/ui/cocoa/bookmarks/bookmark_editor_controller.mm
[modify] https://crrev.com/eb1b3b1a757988622cab119a5f95b118c3338591/chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm
[modify] https://crrev.com/eb1b3b1a757988622cab119a5f95b118c3338591/chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa_browsertest.mm
[modify] https://crrev.com/eb1b3b1a757988622cab119a5f95b118c3338591/chrome/common/chrome_features.cc
[modify] https://crrev.com/eb1b3b1a757988622cab119a5f95b118c3338591/chrome/common/chrome_features.h
[modify] https://crrev.com/eb1b3b1a757988622cab119a5f95b118c3338591/tools/metrics/histograms/enums.xml

Comment 4 by gov...@chromium.org, Aug 21 2017

[Bulk Edit]
URGENT - PTAL.
M61 Stable promotion is coming soon and your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and get it merged into the release branch ASAP. 

Know that this issue shouldn't block the release?  Remove the ReleaseBlock-Stable label or move to M62. Thank you!

Note: We will only have 2 beta releases before Stable promotion. Plan is to cut M61 Stable RC on 08/31/17.

Cc: pbomm...@chromium.org gov...@chromium.org
spqchan@ can you please request a merge to M61(branch:3163) so that we can pick the CL for upcoming Beta refresh this Wednesday. 
Labels: Merge-Request-61
Project Member

Comment 7 by sheriffbot@chromium.org, Aug 21 2017

Labels: -Merge-Request-61 Merge-Review-61 Hotlist-Merge-Review
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Justification: This is critical since dialog support isn't ready to ship out yet. There are automated tests.

Comment 9 by gov...@chromium.org, Aug 21 2017

Labels: -Merge-Review-61 Merge-Approved-61
Approving merge to M61 branch 3163 based on comment #8.
Project Member

Comment 10 by bugdroid1@chromium.org, Aug 22 2017

Labels: -merge-approved-61 merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f65a8764d92374cd10bee3f3fb225e6101cc3431

commit f65a8764d92374cd10bee3f3fb225e6101cc3431
Author: spqchan <spqchan@chromium.org>
Date: Tue Aug 22 16:53:05 2017

Feature Flag for Dialog Touch Bar Support

Dialog touch bar support is currently behind
the Browser Touch Bar flag. This needs to be
separated into a separate flag that's disabled
by default.

This is an attempt to reland
https://chromium-review.googlesource.com/c/616341
which had caused several tests to failed. The test
failures are addressed in this CL

(cherry picked from commit eb1b3b1a757988622cab119a5f95b118c3338591)

Bug:  755814 
Change-Id: I458527a859527b8ebcccca5b12ca9f55b4d39b2c
Reviewed-on: https://chromium-review.googlesource.com/617386
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Sarah Chan <spqchan@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#495203}
Reviewed-on: https://chromium-review.googlesource.com/624872
Reviewed-by: Sarah Chan <spqchan@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#755}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/f65a8764d92374cd10bee3f3fb225e6101cc3431/chrome/browser/about_flags.cc
[modify] https://crrev.com/f65a8764d92374cd10bee3f3fb225e6101cc3431/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/f65a8764d92374cd10bee3f3fb225e6101cc3431/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/f65a8764d92374cd10bee3f3fb225e6101cc3431/chrome/browser/ui/cocoa/bookmarks/bookmark_bubble_controller.mm
[modify] https://crrev.com/f65a8764d92374cd10bee3f3fb225e6101cc3431/chrome/browser/ui/cocoa/bookmarks/bookmark_bubble_controller_unittest.mm
[modify] https://crrev.com/f65a8764d92374cd10bee3f3fb225e6101cc3431/chrome/browser/ui/cocoa/bookmarks/bookmark_editor_base_controller.mm
[modify] https://crrev.com/f65a8764d92374cd10bee3f3fb225e6101cc3431/chrome/browser/ui/cocoa/bookmarks/bookmark_editor_base_controller_unittest.mm
[modify] https://crrev.com/f65a8764d92374cd10bee3f3fb225e6101cc3431/chrome/browser/ui/cocoa/bookmarks/bookmark_editor_controller.mm
[modify] https://crrev.com/f65a8764d92374cd10bee3f3fb225e6101cc3431/chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm
[modify] https://crrev.com/f65a8764d92374cd10bee3f3fb225e6101cc3431/chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa_browsertest.mm
[modify] https://crrev.com/f65a8764d92374cd10bee3f3fb225e6101cc3431/chrome/common/chrome_features.cc
[modify] https://crrev.com/f65a8764d92374cd10bee3f3fb225e6101cc3431/chrome/common/chrome_features.h
[modify] https://crrev.com/f65a8764d92374cd10bee3f3fb225e6101cc3431/tools/metrics/histograms/enums.xml

Status: Fixed (was: Assigned)

Sign in to add a comment