[Mac] Simplify field editor code which tracks whether editor rect changed.

[Mac] Simplify field editor code which tracks whether editor rect changed.

Previous code used an elaborate system of checks, which was gradually
becoming unmaintainable.  Now that -resetFieldEditorFrameIfNeeded is
implemented using dirty tricks, it is pretty easy to just check if it
changed rather than trying to predict it.

BUGS=none
TEST=none

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

[Scott Hess - ex-Googler, 2009-10-20 23:42:47]

Trying to get this in before anyone else has to write code to track dirty.

[rohitrao (ping after 24h), 2009-10-20 23:50:30]

http://codereview.chromium.org/295036/diff/1/2
File chrome/browser/cocoa/autocomplete_text_field.mm (right):

http://codereview.chromium.org/295036/diff/1/2#newcode56
Line 56: if (!NSEqualRects(frame, [subview frame])) {
Why is this condition equivalent to fieldEditorNeedsReset?  Does changing the
keyword string somehow also change the frame?

[Scott Hess - ex-Googler, 2009-10-21 00:23:17]

http://codereview.chromium.org/295036/diff/1/2
File chrome/browser/cocoa/autocomplete_text_field.mm (right):

http://codereview.chromium.org/295036/diff/1/2#newcode56
Line 56: if (!NSEqualRects(frame, [subview frame])) {
On 2009/10/20 23:50:30, rohitrao wrote:
> Why is this condition equivalent to fieldEditorNeedsReset?  Does changing the
> keyword string somehow also change the frame?

The previous code used -fieldEditorNeedsReset to attempt to predict when the
editing area was changed.  The assumption was that changing the editing area was
somewhat expensive.  Perfect prediction would have been really hard to code, so
sometimes it just assumed the worst and returned YES, just in case.

The main difference is that now we'll call -drawingRectForBounds: more often. 
It should be pretty cheap, because it just asks for sizes of
NSAttributedStrings.

[pink (ping after 24hrs), 2009-10-21 15:35:59]

drive-by

http://codereview.chromium.org/295036/diff/1/2
File chrome/browser/cocoa/autocomplete_text_field.mm (right):

http://codereview.chromium.org/295036/diff/1/2#newcode43
Line 43: if (editor) {
why not return early if !editor instead of indenting the entire function body?

[Scott Hess - ex-Googler, 2009-10-21 16:32:14]

http://codereview.chromium.org/295036/diff/1/2
File chrome/browser/cocoa/autocomplete_text_field.mm (right):

http://codereview.chromium.org/295036/diff/1/2#newcode43
Line 43: if (editor) {
On 2009/10/21 15:35:59, pink wrote:
> why not return early if !editor instead of indenting the entire function body?

Makes sense.  Done.

[rohitrao (ping after 24h), 2009-10-21 21:43:12]

The changes themselves LGTM.

I'm still not understanding why changing keywordString_ leads to a frame change.
 I didn't see any calls to setFrame: in autocomplete_text_field_cell.mm.  Why
would the NSEqualRects in resetFieldEditorFrameIfNeeded ever be NO?

[Scott Hess - ex-Googler, 2009-10-21 21:50:08]

On 2009/10/21 21:43:12, rohitrao wrote:
> The changes themselves LGTM.
> 
> I'm still not understanding why changing keywordString_ leads to a frame
change.
>  I didn't see any calls to setFrame: in autocomplete_text_field_cell.mm.  Why
> would the NSEqualRects in resetFieldEditorFrameIfNeeded ever be NO?

The cell doesn't have a frame, it just has stuff to draw. 
-drawingRectForBounds: calculates the frame where the text lives, so if you
change a bunch of stuff in the cell, then next time you call
-resetFieldEditorFrameIfNeeded it notices that -drawingRectForBounds: has
changed and adjusts the field editor.

[rohitrao (ping after 24h), 2009-10-21 21:52:40]

Aah, got it.  Thanks for the explanation.  LGTM