Always call the class methods to save/restore contexts.

ui/gfx/scoped_ns_graphics_context_save_gstate_mac.h

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.

[Mark Mentovai, 2011/08/04 23:20:17]

See also
third_party/WebKit/Source/WebCore/platform/mac/LocalCurrentGraphicsContext.mm.
We do build this file. Weird.

 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef UI_GFX_SCOPED_NS_GRAPHICS_CONTEXT_SAVE_GSTATE_MAC_H_
 #define UI_GFX_SCOPED_NS_GRAPHICS_CONTEXT_SAVE_GSTATE_MAC_H_
 

[Mark Mentovai, 2011/08/04 23:20:17]

Add #pragma once.

[Avi (use Gerrit), 2011/08/05 00:18:50]

Done.

 #include "ui/ui_api.h"

[Mark Mentovai, 2011/08/04 23:20:17]

Sort headers: base < ui.

[Avi (use Gerrit), 2011/08/05 00:18:50]

Done.

 #include "base/basictypes.h"
-#include "base/memory/scoped_nsobject.h"
 
 @class NSGraphicsContext;

[Mark Mentovai, 2011/08/04 23:20:17]

I would do:

#if defined(__OBJC__)
@class NSGraphicsContext;
#else
class NSGraphicsContext;
#endif

because there’s nothing inherently Objective about this interface. I doubt
anyone would want to use it from non-Objective-C code, but hey, it’s possible.

[Avi (use Gerrit), 2011/08/05 00:18:50]

Removed; not needed.

 
 namespace gfx {
 
+// A class to save/restore the state of the current context.
 class UI_API ScopedNSGraphicsContextSaveGState {
  public:
-  // If |context| is nil, it will use the |+currentContext|.
-  explicit ScopedNSGraphicsContextSaveGState(NSGraphicsContext* context = nil);
+  ScopedNSGraphicsContextSaveGState();
   ~ScopedNSGraphicsContextSaveGState();
 
- private:
-  scoped_nsobject<NSGraphicsContext> context_;

[Robert Sesek, 2011/08/04 22:15:44]

So if we were to be paranoid, we'd keep a weak ptr here and DCHECK_EQ in the
dtor that +currentContext == the pointer. Paranoia is maybe a good thing (tm)?

[Avi (use Gerrit), 2011/08/04 22:21:03]

You mean before the save context and after the restore? Not sure if it's
paranoia or not trusting the OS to do its job...

[Robert Sesek, 2011/08/04 22:22:13]

No, it'd prevent changing the current context within the scope of the scoper.

-
   DISALLOW_COPY_AND_ASSIGN(ScopedNSGraphicsContextSaveGState);

[Mark Mentovai, 2011/08/04 23:20:17]

This needs to be in a private: section.

[Avi (use Gerrit), 2011/08/05 00:18:50]

Done.

 };
 
 }  // namespace gfx
 
 #endif  // UI_GFX_SCOPED_NS_GRAPHICS_CONTEXT_SAVE_GSTATE_MAC_H_

[Mark Mentovai, 2011/08/04 23:20:17]

Additional potential interface abuse occurs in:

Operations on +[NSGraphicsContext currentContext] without surrounding
save/restore—these are “draw” routines and it may be OK if some caller is
responsible for doing the save/restore and it doesn’t matter if we dirty the
state:

chrome/browser/ui/cocoa/gradient_button_cell.mm -[GradientButtonCell
drawBorderAndFillForTheme:…]

chrome/browser/ui/cocoa/styled_text_field_cell.mm -[StyledTextFieldCell
drawWithFrame:inView:]

chrome/browser/ui/cocoa/background_tile_view.mm  -[BackgroundTileView drawRect:]

chrome/browser/ui/cocoa/browser_frame_view.mm +[BrowserFrameView
drawWindowThemeInDirtyRect:…]

chrome/browser/ui/cocoa/toolbar/toolbar_view.mm -[ToolbarView drawRect:]

+[NSGraphicsContext restoreGraphicsState] without matching +[NSGraphicsContext
saveGraphicsState]:—candidate for conversion to use this class:

ui/base/clipboard/clipboard_mac.mm
ui::Clipboard::ReadImage(ui::Clipboard::Buffer)

Use of +[NSGraphicsContext currentContext] and +[NSGraphicsContext
setCurrentContext:] instead of +[NSGraphicsContext saveGraphicsState] and
+[NSGraphicsContext restoreGraphicsState]—possible candidate for conversion to
this interface or to be rewritten to use
+saveGraphicsState/+restoreGraphicsState:

webkit/tools/test_shell/mac/webwidget_host.mm WebWidgetHost::Paint()

third_party/WebKit/Tools/DumpRenderTree/chromium/WebThemeEngineDRTMac.mm
WebThemeEngineDRTMac::paintNSScrollerScrollbarThumb(…)

Uses that appear safe but are candidates for conversion to use this class:

chrome/default_plugin/plugin_impl_mac.mm
PluginInstallerImpl::OnDrawRect(CGContextRef, CGRect)

Uses of -saveGraphicsState/-restoreGraphicsState instead of
+saveGraphicsState/+restoreGraphicsState:

third_party/GTM/AppKit/GTMFadeTruncatingTextFieldCell.m
-[GTMFadeTruncatingTextFieldCell drawTextGradientPart:…]

third_party/GTM/AppKit/GTMFadeTruncatingTextFieldCell.m
-[GTMFadeTruncatingTextFieldCell drawInteriorWithFrame:inView:]

[Mark Mentovai, 2011/08/04 23:20:17]

Also, note http://code.google.com/p/chromium/issues/detail?id=90140#c31

I think the use in chrome/browser/ui/cocoa/hover_close_button.mm
+[HoverCloseButton imageForBounds:…] is correct, though. Maybe it’s just
evilness with the old broken scoper that we’re replacing anyway.

[Avi (use Gerrit), 2011/08/05 00:18:50]

For draw routines, AppKit is responsible for setup and teardown; we should be
fine.
Done.
Can I do that one later? Been way too long since I've touched that.
Upstream is out of scope for this CL.
Done.
Upstream is out of scope for this CL.

[Avi (use Gerrit), 2011/08/05 00:18:50]

Now it's good.