Rename InvalidatePaintIfNeeded to drop deprecated IfNeeded suffix.

Rename InvalidatePaintIfNeeded to drop deprecated IfNeeded suffix.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2852573002
Cr-Commit-Position: refs/heads/master@{#467913}
Committed: https://chromium.googlesource.com/chromium/src/+/905b7630a1d5c68e354c417f6cce20c75458b7f9

[wkorman, 2017-04-27 22:49:35]

Description was changed from

==========
Rename InvalidatePaintIfNeeded to drop deprecated IfNeeded suffix.
==========

to

==========
Rename InvalidatePaintIfNeeded to drop deprecated IfNeeded suffix.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[wkorman, 2017-04-27 22:50:21]

wkorman@chromium.org changed reviewers:
+ pdr@chromium.org, wangxianzhu@chromium.org

[wkorman, 2017-04-27 22:50:44]

The CQ bit was checked by wkorman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-27 22:51:38]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[pdr., 2017-04-27 22:52:58]

On 2017/04/27 at 22:50:22, wkorman wrote:
> 

LGTM

Please wait for Xianzhu to review as well.

[Xianzhu, 2017-04-27 22:59:29]

Can you explain more why "IfNeeded" is deprecated?

[wkorman, 2017-04-27 23:13:39]

On 2017/04/27 22:59:29, Xianzhu wrote:
> Can you explain more why "IfNeeded" is deprecated?

Open to debate as always. The background is, when I joined group ~1.5 years ago,
I found the IfNeeded suffix inconsistent and confusing. We used it some places,
and not others. We had many methods that would check whether a value being set
or state to be updated was already so, and if so early-out, which is essentially
the definition of IfNeeded, but were not named with IfNeeded.

It was also long. Google3 has a habit of using MaybeXxx prefix in similar
situations, and Maybe is at least shorter than IfNeeded.

I talked with chrishtr@ and pdr@ in person about it and also sent a couple of
rename changes at the time around updateLifecyclePhases which is what triggered
the discussions IIRC, it was when I was working on http://crrev.com/1411303005/.
Didn't we used to have updateLifecyclePhasesIfNeeded or
updateAllLifecyclePhasesIfNeeded? At some point we did away with that and I
think it was outgrowth of this discussion.

My general takeaway was that we felt IfNeeded was not useful and we should cease
to use it, and rename existing usages as opportunity presented.

Flipping it, initial questions I see are (1) can we define clearly when one
should use IfNeeded suffix for a method name, vs. not, and (2) do we feel like
adding that suffix to a method name is important and useful information.

Personally I would vote for removing it solely on basis of shortening method
names, a.k.a., my answer to (2) is no, but others may feel differently.

[pdr., 2017-04-27 23:24:47]

I think *IsNeeded is generally redundant. For example,
"LayoutObject::UpdateCursorImages" could be named UpdateCursorImagesIfNeeded
because it only updates the cursor if the cursor is different.

I don't think it's really deprecated in general, and I don't know of a
project-wide stance on this. My opinion is that we should only add IsNeeded when
it really adds information.

[wkorman, 2017-04-27 23:30:00]

On 2017/04/27 23:24:47, pdr (OOO until May 3rd) wrote:
> I think *IsNeeded is generally redundant. For example,
> "LayoutObject::UpdateCursorImages" could be named UpdateCursorImagesIfNeeded
> because it only updates the cursor if the cursor is different.
> 
> I don't think it's really deprecated in general, and I don't know of a
> project-wide stance on this. My opinion is that we should only add IsNeeded
when
> it really adds information.

I would be interested to see some examples where we feel it really adds
information.

If we go forward with this change I do plan to send another change to rename the
remaining in paint, refs below.

Yeah, I didn't see it as a project-wide deprecation, but rather a paint focused
one. But we didn't discuss it outside of that initial in person conversation,
really, so it's good that we're doing it now.

% find ./third_party/WebKit/Source/core/paint -name "*.h" | xargs grep IfNeeded
| grep -v InvalidatePaintIfNeeded
./third_party/WebKit/Source/core/paint/ViewPaintInvalidator.h:  void
InvalidateBackgroundIfNeeded();
./third_party/WebKit/Source/core/paint/BoxPaintInvalidator.h:  void
InvalidateScrollingContentsBackgroundIfNeeded();
./third_party/WebKit/Source/core/paint/BoxPaintInvalidator.h:  void
SavePreviousBoxGeometriesIfNeeded();
./third_party/WebKit/Source/core/paint/PaintLayerStackingNode.h:  void
UpdateLayerListsIfNeeded();
./third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetector.h:  void
MarkNextPaintAsMeaningfulIfNeeded(const LayoutObjectCounter&,
./third_party/WebKit/Source/core/paint/PrePaintTreeWalk.h:  ALWAYS_INLINE void
InvalidatePaintLayerOptimizationsIfNeeded(
./third_party/WebKit/Source/core/paint/PaintInvalidator.h:  ALWAYS_INLINE void
UpdateVisualRectIfNeeded(const LayoutObject&,
./third_party/WebKit/Source/core/paint/ObjectPainter.h:  void
AddPDFURLRectIfNeeded(const PaintInfo&, const LayoutPoint& paint_offset);
./third_party/WebKit/Source/core/paint/InlineFlowBoxPainter.h:  LayoutRect
FrameRectClampedToLineTopAndBottomIfNeeded() const;
./third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.h:  void
InvalidateSelectionIfNeeded(PaintInvalidationReason);
./third_party/WebKit/Source/core/paint/BlockPainter.h:  void
PaintOverflowControlsIfNeeded(const PaintInfo&, const LayoutPoint&);
./third_party/WebKit/Source/core/paint/PaintInvalidationCapableScrollableArea.h:
 void InvalidatePaintOfScrollControlsIfNeeded(const PaintInvalidationState&);
./third_party/WebKit/Source/core/paint/PaintInvalidationCapableScrollableArea.h:
 void InvalidatePaintOfScrollControlsIfNeeded(const PaintInvalidatorContext&);

% find ./third_party/WebKit/Source/platform/graphics -name "*.h" | xargs grep
IfNeeded | grep -v InvalidatePaintIfNeeded
./third_party/WebKit/Source/platform/graphics/paint/GeometryMapperTransformCache.h:
 void InvalidateCacheIfNeeded();
./third_party/WebKit/Source/platform/graphics/paint/PaintController.h:  static
void AddItemToIndexIfNeeded(const DisplayItem&,
./third_party/WebKit/Source/platform/graphics/paint/GeometryMapperClipCache.h: 
void InvalidateCacheIfNeeded();
./third_party/WebKit/Source/platform/graphics/ImageBuffer.h:  void
PrepareSurfaceForPaintingIfNeeded() {
./third_party/WebKit/Source/platform/graphics/ImageBuffer.h:   
surface_->PrepareSurfaceForPaintingIfNeeded();
./third_party/WebKit/Source/platform/graphics/AcceleratedStaticBitmapImage.h: 
void CreateImageFromMailboxIfNeeded();
./third_party/WebKit/Source/platform/graphics/AcceleratedStaticBitmapImage.h: 
void WaitSyncTokenIfNeeded();
./third_party/WebKit/Source/platform/graphics/ImageBufferSurface.h:  virtual
void PrepareSurfaceForPaintingIfNeeded() {}
./third_party/WebKit/Source/platform/graphics/GraphicsContextStateSaver.h:  void
SaveIfNeeded() {
./third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.h:  void
ResolveIfNeeded();
./third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.h:  // True if
resolveIfNeeded() has been called since the last time
./third_party/WebKit/Source/platform/graphics/gpu/SharedGpuContext.h:  void
CreateContextProviderIfNeeded();

[Xianzhu, 2017-04-28 00:09:33]

Thanks for explanation. I think it's reasonable to remove most of the IfNeeded
postfixes.

LGTM.

[commit-bot: I haz the power, 2017-04-28 01:36:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-28 01:36:59]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_layout_tests_slimming_paint_v2 on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_layout_te...)
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Xianzhu, 2017-04-28 03:42:47]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2017-04-28 03:43:23]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-28 05:55:44]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1493350967550480, "parent_rev":
"8a09e380ca0d6d41a52d066da4b288dacc29f40e", "commit_rev":
"905b7630a1d5c68e354c417f6cce20c75458b7f9"}

[commit-bot: I haz the power, 2017-04-28 05:55:53]

Description was changed from

==========
Rename InvalidatePaintIfNeeded to drop deprecated IfNeeded suffix.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Rename InvalidatePaintIfNeeded to drop deprecated IfNeeded suffix.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2852573002
Cr-Commit-Position: refs/heads/master@{#467913}
Committed:
https://chromium.googlesource.com/chromium/src/+/905b7630a1d5c68e354c417f6cce...
==========

[commit-bot: I haz the power, 2017-04-28 05:55:54]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/905b7630a1d5c68e354c417f6cce...