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

Issue 2675213002: [Mac] DialogBrowserTest implementation to invoke Content settings bubble dialogs. (Closed)

Created:
3 years, 10 months ago by tapted
Modified:
3 years, 10 months ago
Reviewers:
kylix_rd
CC:
chromium-reviews, tfarina, mac-reviews_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[Mac] DialogBrowserTest implementation to invoke Content settings bubble dialogs. There needs to be a way to invoke all the various content settings bubble dialog views. This wires up the BrowserDialogTest framework in order to invoke/test them. BUG=652024 patch from issue 2668833003 at patchset 180001 (http://crrev.com/2668833003#ps180001)

Patch Set 1 #

Patch Set 2 : Gets it working on Mac #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+228 lines, -3 lines) Patch
M chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.h View 1 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm View 1 1 chunk +13 lines, -0 lines 0 comments Download
M chrome/browser/ui/location_bar/location_bar.h View 1 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/browser/ui/test/browser_dialog_browsertest.cc View 2 chunks +8 lines, -0 lines 0 comments Download
A chrome/browser/ui/views/location_bar/content_setting_bubble_dialog_browsertest.cc View 1 1 chunk +187 lines, -0 lines 0 comments Download
M chrome/browser/ui/views/location_bar/location_bar_view.h View 1 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/ui/views/location_bar/location_bar_view.cc View 1 1 chunk +10 lines, -0 lines 1 comment Download
M chrome/test/BUILD.gn View 1 1 chunk +1 line, -0 lines 0 comments Download
M ui/views/test/widget_test_aura.cc View 1 chunk +3 lines, -3 lines 0 comments Download

Messages

Total messages: 6 (5 generated)
kylix_rd
3 years, 10 months ago (2017-02-06 14:12:40 UTC) #6
https://codereview.chromium.org/2675213002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/location_bar/location_bar_view.cc (right):

https://codereview.chromium.org/2675213002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/location_bar/location_bar_view.cc:1108: }
Doesn't relying on the number and order of the items in content_setting_views_
make this code extremely brittle? What if that order changes? There seems to be
no obvious tie between the index passed in here and the list in
content_setting_bubble_dialog_browsertest.cc. IMO, you've traded fewer changes
to release code for a more brittle test.

Powered by Google App Engine
This is Rietveld 408576698