webkitpy: Make webkit_base() protected.

webkitpy: Make webkit_base() protected.

Rename it to _webkit_base(). The new name conflicts with the existing
_webkit_base, and this CL removes it by @memoized.

BUG=710535
Review-Url: https://codereview.chromium.org/2867713007
Cr-Commit-Position: refs/heads/master@{#470832}
Committed: https://chromium.googlesource.com/chromium/src/+/5727d41e60da68273c3a583e42ff447f47b26d93

[tkent, 2017-05-09 01:18:46]

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

[tkent, 2017-05-09 01:19:50]

tkent@chromium.org changed reviewers:
+ jeffcarp@chromium.org, qyearsley@chromium.org

[tkent, 2017-05-09 01:19:50]

qyearsley@, jeffcarp@, would you review this please?

https://codereview.chromium.org/2867713007/diff/1/third_party/WebKit/Tools/Sc...
File
third_party/WebKit/Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py
(left):

https://codereview.chromium.org/2867713007/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py:66:
self._webkit_base_dir_len = len(self._port.webkit_base())
_webkit_base_dir_len isn't used.

[commit-bot: I haz the power, 2017-05-09 01:19:57]

Dry run: 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-05-09 01:51:53]

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

[commit-bot: I haz the power, 2017-05-09 01:51:54]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[jeffcarp, 2017-05-09 19:26:56]

lgtm

[qyearsley, 2017-05-09 21:46:28]

LGTM with nits

https://codereview.chromium.org/2867713007/diff/1/third_party/WebKit/Tools/Sc...
File third_party/WebKit/Tools/Scripts/webkitpy/common/webkit_finder.py (right):

https://codereview.chromium.org/2867713007/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/common/webkit_finder.py:87: # in order
to make the code robust against directory structure changes.
This comment should be unnecessary, since we have a pylint check for any new
code that tries to call protected methods on other objects.

https://codereview.chromium.org/2867713007/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/common/webkit_finder.py:103:
self._chromium_base =
self._filesystem.dirname(self._filesystem.dirname(self._webkit_base()))
Potential minor change for consistency: Now that _webkit_base is a memoized
method, perhaps this one could be too, and then the _chromium_base attribute
would be unnecessary too.

https://codereview.chromium.org/2867713007/diff/1/third_party/WebKit/Tools/Sc...
File third_party/WebKit/Tools/Scripts/webkitpy/common/webkit_finder_unittest.py
(right):

https://codereview.chromium.org/2867713007/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/common/webkit_finder_unittest.py:28:
self.assertEqual(finder._webkit_base(), '/mock-checkout/third_party/WebKit')
I would expect a "protected-access" pylint warning here. Since here we're
explicitly testing a protected method, we could add a # pylint:
disable=protected-access comment to disable any such warnings here.

https://codereview.chromium.org/2867713007/diff/1/third_party/WebKit/Tools/Sc...
File third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/test.py (left):

https://codereview.chromium.org/2867713007/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/test.py:469: return
'/test.checkout'
I would have expected that changing this might require updating tests. I guess
that's not the case?

https://codereview.chromium.org/2867713007/diff/1/third_party/WebKit/Tools/Sc...
File third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/test.py
(right):

https://codereview.chromium.org/2867713007/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/test.py:466: return
PERF_TEST_DIR
Later, we may want to change the layout_tests_dir and perf_tests_dir used by
TestPort -- right now they're hard-coded to special non-standard locations
'/test.checkout/LayoutTests' and '/test.checkout/PerformanceTests'.

After this change, this is now inconsistent with webkit_base() below, which will
return something more like "/mock_checkout/third_party/WebKit" (I think).

https://codereview.chromium.org/2867713007/diff/1/third_party/WebKit/Tools/Sc...
File
third_party/WebKit/Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py
(left):

https://codereview.chromium.org/2867713007/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py:66:
self._webkit_base_dir_len = len(self._port.webkit_base())
On 2017/05/09 at 01:19:50, tkent wrote:
> _webkit_base_dir_len isn't used.

Excellent :-)

[tkent, 2017-05-10 23:42:21]

https://codereview.chromium.org/2867713007/diff/1/third_party/WebKit/Tools/Sc...
File third_party/WebKit/Tools/Scripts/webkitpy/common/webkit_finder.py (right):

https://codereview.chromium.org/2867713007/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/common/webkit_finder.py:87: # in order
to make the code robust against directory structure changes.
On 2017/05/09 at 21:46:28, qyearsley wrote:
> This comment should be unnecessary, since we have a pylint check for any new
code that tries to call protected methods on other objects.

ok, removed.

https://codereview.chromium.org/2867713007/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/common/webkit_finder.py:103:
self._chromium_base =
self._filesystem.dirname(self._filesystem.dirname(self._webkit_base()))
On 2017/05/09 at 21:46:28, qyearsley wrote:
> Potential minor change for consistency: Now that _webkit_base is a memoized
method, perhaps this one could be too, and then the _chromium_base attribute
would be unnecessary too.

I'll do it in a separated CL.

https://codereview.chromium.org/2867713007/diff/1/third_party/WebKit/Tools/Sc...
File third_party/WebKit/Tools/Scripts/webkitpy/common/webkit_finder_unittest.py
(right):

https://codereview.chromium.org/2867713007/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/common/webkit_finder_unittest.py:28:
self.assertEqual(finder._webkit_base(), '/mock-checkout/third_party/WebKit')
On 2017/05/09 at 21:46:28, qyearsley wrote:
> I would expect a "protected-access" pylint warning here. Since here we're
explicitly testing a protected method, we could add a # pylint:
disable=protected-access comment to disable any such warnings here.

Ah, right. pylint complains this.  Neither test-webkitpy nor presubmit don't run
pylint?

https://codereview.chromium.org/2867713007/diff/1/third_party/WebKit/Tools/Sc...
File third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/test.py (left):

https://codereview.chromium.org/2867713007/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/test.py:469: return
'/test.checkout'
On 2017/05/09 at 21:46:28, qyearsley wrote:
> I would have expected that changing this might require updating tests. I guess
that's not the case?

Port.webkit_base() was used only for PerTestsRunner._webkit_base_dir_len. 
That's a dead code.

https://codereview.chromium.org/2867713007/diff/1/third_party/WebKit/Tools/Sc...
File third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/test.py
(right):

https://codereview.chromium.org/2867713007/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/test.py:466: return
PERF_TEST_DIR
On 2017/05/09 at 21:46:28, qyearsley wrote:
> Later, we may want to change the layout_tests_dir and perf_tests_dir used by
TestPort -- right now they're hard-coded to special non-standard locations
'/test.checkout/LayoutTests' and '/test.checkout/PerformanceTests'.
> 
> After this change, this is now inconsistent with webkit_base() below, which
will return something more like "/mock_checkout/third_party/WebKit" (I think).

Port.layout_tests_dir can be different from webkit_base()/LayoutTests because of
layout_tests_directory option. So we don't need to change
TestPort.layout_tests_dir, IMO.

We should remove Port.perf_tests_dir() as the TODO(qyearsley) in base.py says.

[tkent, 2017-05-10 23:42:25]

The CQ bit was checked by tkent@chromium.org

[tkent, 2017-05-10 23:42:25]

The patchset sent to the CQ was uploaded after l-g-t-m from
qyearsley@chromium.org, jeffcarp@chromium.org
Link to the patchset: https://codereview.chromium.org/2867713007/#ps20001
(title: "Removed a code comment, avoid a pylint warning")

[commit-bot: I haz the power, 2017-05-10 23:43:49]

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-05-11 01:44:49]

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

[commit-bot: I haz the power, 2017-05-11 01:44:50]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tkent, 2017-05-11 04:17:25]

The CQ bit was checked by tkent@chromium.org

[commit-bot: I haz the power, 2017-05-11 04:19:08]

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-05-11 05:53:57]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1494476245897860,
"parent_rev": "4addda75cfc290605d77e545181bfbec17cdb163", "commit_rev":
"5727d41e60da68273c3a583e42ff447f47b26d93"}

[commit-bot: I haz the power, 2017-05-11 05:54:06]

Description was changed from

==========
webkitpy: Make webkit_base() protected.

Rename it to _webkit_base(). The new name conflicts with the existing
_webkit_base, and this CL removes it by @memoized.

BUG=710535
==========

to

==========
webkitpy: Make webkit_base() protected.

Rename it to _webkit_base(). The new name conflicts with the existing
_webkit_base, and this CL removes it by @memoized.

BUG=710535

Review-Url: https://codereview.chromium.org/2867713007
Cr-Commit-Position: refs/heads/master@{#470832}
Committed:
https://chromium.googlesource.com/chromium/src/+/5727d41e60da68273c3a583e42ff...
==========

[commit-bot: I haz the power, 2017-05-11 05:54:07]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/5727d41e60da68273c3a583e42ff...