Display content settings applying to the current otr session only.

Display content settings applying to the current otr session only.

Also allow for editing them... 

The XIB change adds a column at the end of the table with a NSCheckboxCell in it and the title IDS_EXCEPTIONS_OTR_HEADER

BUG=44480
TEST=manual

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=51507

[jochen (gone - plz use gerrit), 2010-06-29 14:23:23]

please review.

The Mac UI bits are missing... the mac implementation is a real mess, and after
trying to figure out how to do this with the local mac experts, I decided to
file a bug against it for thakis who did this.

[jochen (gone - plz use gerrit), 2010-06-29 17:33:19]

sorry for not including screenshots btw. Here's what the UI looks like:

When no incognito window exists, there's no change visible.

When an incognito window exists
- there's a label below the list of patterns saying something like "Exceptions
in italics apply to the current incognito session only"
- Exceptions that apply to incognito only are displayed in italics
- when you add/edit an exceptions, there's an additional checkbox under the
action combobox that says "applies to the current incognito session only"

[Nico, 2010-06-29 17:34:42]

Chances are I don't have time to do this for m6.

[jochen (gone - plz use gerrit), 2010-06-29 18:20:21]

On 2010/06/29 17:34:42, Nico wrote:
> Chances are I don't have time to do this for m6.

I'll send you my patch I got so far, probably it's easy for you to fix the
remaining bits then.

[Peter Kasting, 2010-06-29 18:28:07]

LGTM with comments

http://codereview.chromium.org/2858032/diff/1/2
File chrome/app/generated_resources.grd (right):

http://codereview.chromium.org/2858032/diff/1/2#newcode4476
chrome/app/generated_resources.grd:4476: Exceptions displayed in italics apply
to this incognito session only.
Nit: "apply ... only" -> "only apply ..."?

http://codereview.chromium.org/2858032/diff/1/2#newcode4511
chrome/app/generated_resources.grd:4511: Applies to this incognito session only
Nit: "Applies ... only" -> "Only apply ..."?

http://codereview.chromium.org/2858032/diff/1/4
File chrome/browser/cocoa/content_exceptions_window_controller.mm (right):

http://codereview.chromium.org/2858032/diff/1/4#newcode146
chrome/browser/cocoa/content_exceptions_window_controller.mm:146:
offTheRecordSettingsMap:(HostContentSettingsMap*)offTheRecordSettingsMap {
I'm not an ObjC style expert, but the indentings in this file look wrong
(nothing should be 0-space indented).  You should get a Mac person to check.

http://codereview.chromium.org/2858032/diff/1/6
File chrome/browser/cocoa/content_settings_dialog_controller.mm (right):

http://codereview.chromium.org/2858032/diff/1/6#newcode357
chrome/browser/cocoa/content_settings_dialog_controller.mm:357:
offTheRecordSettingsMap:offTheRecordSettingsMap]
Nit: Again, this indent seems wrong

http://codereview.chromium.org/2858032/diff/1/7
File chrome/browser/content_exceptions_table_model.cc (right):

http://codereview.chromium.org/2858032/diff/1/7#newcode31
chrome/browser/content_exceptions_table_model.cc:31: if (is_off_the_record &&
!off_the_record_map_) {
Nit: Just DCHECK() this rather than handling it

http://codereview.chromium.org/2858032/diff/1/7#newcode36
chrome/browser/content_exceptions_table_model.cc:36: is_off_the_record ?
off_the_record_map_ : map_;
Nit: It seems a little lame that we do a lot of these in several functions. 
Maybe there should be some small private inline functions, so that here you can
just do:

entries()->push_back(...);

etc.

http://codereview.chromium.org/2858032/diff/1/7#newcode79
chrome/browser/content_exceptions_table_model.cc:79: if (is_off_the_record &&
!off_the_record_map_) {
Nit: Just DCHECK() this rather than handling it

http://codereview.chromium.org/2858032/diff/1/7#newcode91
chrome/browser/content_exceptions_table_model.cc:91: if
(entries_to_search->at(i).first == pattern)
Nit: Why are you using at()?  Use [].

http://codereview.chromium.org/2858032/diff/1/21
File chrome/browser/views/options/exception_editor_view.h (right):

http://codereview.chromium.org/2858032/diff/1/21#newcode108
chrome/browser/views/options/exception_editor_view.h:108: views::Checkbox*
incognito_cb_;
Nit: I don't know if the type suffixes are a good idea when Combobox and
Checkbox are both "cb".  Maybe we should just name these something meaningful
enough on their own.

[jochen (gone - plz use gerrit), 2010-06-30 16:08:41]

Nico, please have a look at the cocoa changes. Here's how it looks like if you
have an incognito window open (if not, there's no visible change):
http://imgur.com/RzIfk.png

I've updated the description to reflect the xib changes

http://codereview.chromium.org/2858032/diff/1/2
File chrome/app/generated_resources.grd (right):

http://codereview.chromium.org/2858032/diff/1/2#newcode4476
chrome/app/generated_resources.grd:4476: Exceptions displayed in italics apply
to this incognito session only.
On 2010/06/29 18:28:08, Peter Kasting wrote:
> Nit: "apply ... only" -> "only apply ..."?

Done.

http://codereview.chromium.org/2858032/diff/1/2#newcode4511
chrome/app/generated_resources.grd:4511: Applies to this incognito session only
On 2010/06/29 18:28:08, Peter Kasting wrote:
> Nit: "Applies ... only" -> "Only apply ..."?

Done.

http://codereview.chromium.org/2858032/diff/1/7
File chrome/browser/content_exceptions_table_model.cc (right):

http://codereview.chromium.org/2858032/diff/1/7#newcode31
chrome/browser/content_exceptions_table_model.cc:31: if (is_off_the_record &&
!off_the_record_map_) {
On 2010/06/29 18:28:08, Peter Kasting wrote:
> Nit: Just DCHECK() this rather than handling it

Done.

http://codereview.chromium.org/2858032/diff/1/7#newcode36
chrome/browser/content_exceptions_table_model.cc:36: is_off_the_record ?
off_the_record_map_ : map_;
On 2010/06/29 18:28:08, Peter Kasting wrote:
> Nit: It seems a little lame that we do a lot of these in several functions. 
> Maybe there should be some small private inline functions, so that here you
can
> just do:
> 
> entries()->push_back(...);
> 
> etc.

Done.

http://codereview.chromium.org/2858032/diff/1/7#newcode79
chrome/browser/content_exceptions_table_model.cc:79: if (is_off_the_record &&
!off_the_record_map_) {
On 2010/06/29 18:28:08, Peter Kasting wrote:
> Nit: Just DCHECK() this rather than handling it

Done.

http://codereview.chromium.org/2858032/diff/1/7#newcode91
chrome/browser/content_exceptions_table_model.cc:91: if
(entries_to_search->at(i).first == pattern)
On 2010/06/29 18:28:08, Peter Kasting wrote:
> Nit: Why are you using at()?  Use [].

Done.

[Nico, 2010-06-30 16:30:18]

Mac parts LG. Thanks for doing them!

The UI looks…interesting :-P I assume this is is all ui leads approved?

http://codereview.chromium.org/2858032/diff/11001/12003
File chrome/browser/cocoa/content_exceptions_window_controller.h (right):

http://codereview.chromium.org/2858032/diff/11001/12003#newcode38
chrome/browser/cocoa/content_exceptions_window_controller.h:38: // is allowed.
s/is/be/

http://codereview.chromium.org/2858032/diff/11001/12003#newcode39
chrome/browser/cocoa/content_exceptions_window_controller.h:39: BOOL
otrIsAllowed_;
Nit: otrAllowed_? (feel free to ignore)

http://codereview.chromium.org/2858032/diff/11001/12004
File chrome/browser/cocoa/content_exceptions_window_controller.mm (right):

http://codereview.chromium.org/2858032/diff/11001/12004#newcode24
chrome/browser/cocoa/content_exceptions_window_controller.mm:24:
forOTR:(BOOL)isOTR;
forOtr. Do not keep abbreviations in upper-case. (PdfUrl reads better than
PDFURL for example)

http://codereview.chromium.org/2858032/diff/11001/12006
File chrome/browser/cocoa/content_settings_dialog_controller.mm (right):

http://codereview.chromium.org/2858032/diff/11001/12006#newcode358
chrome/browser/cocoa/content_settings_dialog_controller.mm:358:
attachSheetTo:[self window]];
Indent this lik


  [[ContentExceptionsWindowController controllerForType:settingsType
                                            settingsMap:settingsMap
                                         otrSettingsMap:offTheRecordSettingsMap]
      attachSheetTo:[self window]];