[DevTools] Refactor Linkifier to unify link processing.

[DevTools] Refactor Linkifier to unify link processing.

BUG=665661
Committed: https://crrev.com/081f467eccb37a5ded14adbf17881f8780f3bb92
Cr-Commit-Position: refs/heads/master@{#433925}

[dgozman, 2016-11-20 04:54:11]

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

[commit-bot: I haz the power, 2016-11-20 04:54:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dgozman, 2016-11-20 06:19:57]

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

[commit-bot: I haz the power, 2016-11-20 06:20:08]

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, 2016-11-20 10:21:25]

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

[commit-bot: I haz the power, 2016-11-20 10:21:26]

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

[dgozman, 2016-11-21 19:19:02]

dgozman@chromium.org changed reviewers:
+ lushnikov@chromium.org

[dgozman, 2016-11-21 19:19:03]

Could you please take a look?

[lushnikov, 2016-11-21 20:14:07]

lgtm

https://codereview.chromium.org/2512353002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/components/Linkifier.js
(right):

https://codereview.chromium.org/2512353002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/components/Linkifier.js:208:
info.enableDecorator = true;
here and below: just info.enableDecorator = this._useLinkDecorator;

https://codereview.chromium.org/2512353002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/components/Linkifier.js:296: var
anchor = Components.Linkifier._createLink('', classes || '');
so what is "anchor" and what is "link"? Do we differentiate between them?

https://codereview.chromium.org/2512353002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/components/Linkifier.js:367: if
(!info.enableDecorator)
if (!info || !info.enableDecorator)

https://codereview.chromium.org/2512353002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/components/Linkifier.js:390: if
(!url || url.trim().toLowerCase().startsWith('javascript:')) {
this drops link.lineNumber and link.columnNumber which you wanted to keep

https://codereview.chromium.org/2512353002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/components/Linkifier.js:457: return
/** @type {?Components._LinkInfo} */ ((link &&
link[Components.Linkifier._infoSymbol]) || null);
nit: we tend to not use '&&' in this way.

return link ? link[Components.Linkifier._info] || null : null;

https://codereview.chromium.org/2512353002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/components/Linkifier.js:490: if
(info && info.revealable)
let's not bail-out

[dgozman, 2016-11-21 22:37:48]

https://codereview.chromium.org/2512353002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/components/Linkifier.js
(right):

https://codereview.chromium.org/2512353002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/components/Linkifier.js:208:
info.enableDecorator = true;
On 2016/11/21 20:14:07, lushnikov wrote:
> here and below: just info.enableDecorator = this._useLinkDecorator;

Done.

https://codereview.chromium.org/2512353002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/components/Linkifier.js:296: var
anchor = Components.Linkifier._createLink('', classes || '');
On 2016/11/21 20:14:07, lushnikov wrote:
> so what is "anchor" and what is "link"? Do we differentiate between them?

Same thing. I just think that linkifier should work with "links", but changing
every place is not worth it :-)

https://codereview.chromium.org/2512353002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/components/Linkifier.js:367: if
(!info.enableDecorator)
On 2016/11/21 20:14:07, lushnikov wrote:
> if (!info || !info.enableDecorator)

Done.

https://codereview.chromium.org/2512353002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/components/Linkifier.js:390: if
(!url || url.trim().toLowerCase().startsWith('javascript:')) {
On 2016/11/21 20:14:07, lushnikov wrote:
> this drops link.lineNumber and link.columnNumber which you wanted to keep

Don't need them anymore, since we don't use external links in linkifier since 3
patches before.

https://codereview.chromium.org/2512353002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/components/Linkifier.js:457: return
/** @type {?Components._LinkInfo} */ ((link &&
link[Components.Linkifier._infoSymbol]) || null);
On 2016/11/21 20:14:07, lushnikov wrote:
> nit: we tend to not use '&&' in this way.
> 
> return link ? link[Components.Linkifier._info] || null : null;

Done.

https://codereview.chromium.org/2512353002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/components/Linkifier.js:490: if
(info && info.revealable)
On 2016/11/21 20:14:07, lushnikov wrote:
> let's not bail-out

Done.

[dgozman, 2016-11-21 22:37:52]

The CQ bit was checked by dgozman@chromium.org

[dgozman, 2016-11-21 22:37:53]

The patchset sent to the CQ was uploaded after l-g-t-m from
lushnikov@chromium.org
Link to the patchset: https://codereview.chromium.org/2512353002/#ps60001
(title: "fixed review comments")

[commit-bot: I haz the power, 2016-11-21 22:38:59]

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, 2016-11-22 00:12:51]

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

[commit-bot: I haz the power, 2016-11-22 00:12:52]

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

[dgozman, 2016-11-22 00:36:49]

The CQ bit was checked by dgozman@chromium.org

[commit-bot: I haz the power, 2016-11-22 00:37:16]

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, 2016-11-22 01:22:17]

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

[commit-bot: I haz the power, 2016-11-22 01:22:18]

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

[dgozman, 2016-11-22 01:29:58]

The CQ bit was checked by dgozman@chromium.org

[commit-bot: I haz the power, 2016-11-22 01:30:37]

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, 2016-11-22 02:30:11]

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

[commit-bot: I haz the power, 2016-11-22 02:30:14]

Failed to apply patch for
third_party/WebKit/Source/devtools/front_end/timeline/TimelineUIUtils.js:
While running git apply --index -p1;
  error: patch failed:
third_party/WebKit/Source/devtools/front_end/timeline/TimelineUIUtils.js:1530
  error:
third_party/WebKit/Source/devtools/front_end/timeline/TimelineUIUtils.js: patch
does not apply

Patch:      
third_party/WebKit/Source/devtools/front_end/timeline/TimelineUIUtils.js
Index: third_party/WebKit/Source/devtools/front_end/timeline/TimelineUIUtils.js
diff --git
a/third_party/WebKit/Source/devtools/front_end/timeline/TimelineUIUtils.js
b/third_party/WebKit/Source/devtools/front_end/timeline/TimelineUIUtils.js
index
cf95e4c76472014e222830a7095e5891ec0e5212..610ba9300fce6e35bfcd94295bb558a3dfd41bae
100644
--- a/third_party/WebKit/Source/devtools/front_end/timeline/TimelineUIUtils.js
+++ b/third_party/WebKit/Source/devtools/front_end/timeline/TimelineUIUtils.js
@@ -1530,7 +1530,7 @@ Timeline.TimelineUIUtils = class {
     if (Runtime.experiments.isEnabled('layersPanel') && frame.layerTree) {
       contentHelper.appendElementRow(
           Common.UIString('Layer tree'),
-          Components.Linkifier.linkifyUsingRevealer(frame.layerTree,
Common.UIString('show')));
+          Components.Linkifier.linkifyRevealable(frame.layerTree,
Common.UIString('show')));
     }
 
     /**

[dgozman, 2016-11-22 17:24:54]

The CQ bit was checked by dgozman@chromium.org

[dgozman, 2016-11-22 17:24:54]

The patchset sent to the CQ was uploaded after l-g-t-m from
lushnikov@chromium.org
Link to the patchset: https://codereview.chromium.org/2512353002/#ps80001
(title: "rebased")

[commit-bot: I haz the power, 2016-11-22 17:25: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, 2016-11-22 18:42:01]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1479835494217450,
"parent_rev": "ead6ee132ae422909e507fde606aebe1f83b7e25", "commit_rev":
"89cfea0b7b9f5a7746d32f7277d28aa5d95497d1"}

[commit-bot: I haz the power, 2016-11-22 18:42:29]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-11-22 18:44:13]

Description was changed from

==========
[DevTools] Refactor Linkifier to unify link processing.

BUG=665661
==========

to

==========
[DevTools] Refactor Linkifier to unify link processing.

BUG=665661

Committed: https://crrev.com/081f467eccb37a5ded14adbf17881f8780f3bb92
Cr-Commit-Position: refs/heads/master@{#433925}
==========

[commit-bot: I haz the power, 2016-11-22 18:44:14]

Patchset 5 (id:??) landed as
https://crrev.com/081f467eccb37a5ded14adbf17881f8780f3bb92
Cr-Commit-Position: refs/heads/master@{#433925}