Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(61)

Issue 479753003: Transform XML tree viewer to blink-in-js implementation. (Closed)

Created:
6 years, 3 months ago by vivekg_samsung
Modified:
6 years, 3 months ago
CC:
blink-reviews
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

Transform XML tree viewer to blink-in-js implementation. This CL moves the XML tree viewer to use blink-in-js implementation. As part of this CL the XMLTreeViewer.idl exposes transformDocumentToTreeView which is used by XMLDocumentParser to transform into tree view. This CL should follow up with the removal of Source/core/xml/XMLViewer.[css/js] and the subsequent changes in public/blink_resources.grd. BUG=341031 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=181328

Patch Set 1 #

Total comments: 3

Patch Set 2 : #

Total comments: 6

Patch Set 3 : #

Total comments: 5

Patch Set 4 : #

Total comments: 1

Patch Set 5 : Fixed declaration order in core_generated.gyp #

Patch Set 6 : Patch for landing! #

Total comments: 4

Patch Set 7 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+478 lines, -126 lines) Patch
M LayoutTests/xmlviewer/extensions-api.html View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M Source/core/BUILD.gn View 1 2 3 4 5 1 chunk +2 lines, -1 line 0 comments Download
M Source/core/core.gypi View 1 2 3 4 5 6 2 chunks +1 line, -2 lines 0 comments Download
M Source/core/core_generated.gyp View 1 2 3 1 chunk +2 lines, -1 line 0 comments Download
A + Source/core/xml/DocumentXMLTreeViewer.idl View 1 2 3 4 5 6 1 chunk +2 lines, -2 lines 0 comments Download
A Source/core/xml/DocumentXMLTreeViewer.js View 1 2 3 4 5 6 1 chunk +466 lines, -0 lines 0 comments Download
M Source/core/xml/XMLTreeViewer.h View 1 2 1 chunk +0 lines, -51 lines 0 comments Download
D Source/core/xml/XMLTreeViewer.cpp View 1 chunk +0 lines, -65 lines 0 comments Download
M Source/core/xml/parser/XMLDocumentParser.cpp View 1 2 3 4 5 6 3 chunks +4 lines, -3 lines 0 comments Download

Messages

Total messages: 51 (3 generated)
vivekg
vivekg@chromium.org changed reviewers: + haraken@chromium.org, vivekg@chromium.org
6 years, 3 months ago (2014-08-26 05:35:21 UTC) #1
vivekg
PTAL, thank you!
6 years, 3 months ago (2014-08-26 05:35:22 UTC) #2
haraken
haraken@chromium.org changed reviewers: + abarth@chromium.org, hajimehoshi@chromium.org
6 years, 3 months ago (2014-08-26 05:38:42 UTC) #3
haraken
You'll need to add XMLViewer.idl and XMLViewer.js ?
6 years, 3 months ago (2014-08-26 05:38:42 UTC) #4
eseidel
eseidel@chromium.org changed reviewers: + eseidel@chromium.org
6 years, 3 months ago (2014-08-26 05:39:28 UTC) #5
eseidel
https://codereview.chromium.org/479753003/diff/1/Source/core/xml/XMLTreeViewer.h File Source/core/xml/XMLTreeViewer.h (right): https://codereview.chromium.org/479753003/diff/1/Source/core/xml/XMLTreeViewer.h#newcode39 Source/core/xml/XMLTreeViewer.h:39: class XMLTreeViewer FINAL : public GarbageCollectedFinalized<XMLTreeViewer>, public ScriptWrappable { ...
6 years, 3 months ago (2014-08-26 05:39:28 UTC) #6
vivekg
On 2014/08/26 05:38:42, haraken wrote: > You'll need to add XMLViewer.idl and XMLViewer.js ? Yeah ...
6 years, 3 months ago (2014-08-26 05:39:43 UTC) #7
eseidel
https://codereview.chromium.org/479753003/diff/20001/Source/core/xml/XMLTreeViewer.idl File Source/core/xml/XMLTreeViewer.idl (right): https://codereview.chromium.org/479753003/diff/20001/Source/core/xml/XMLTreeViewer.idl#newcode7 Source/core/xml/XMLTreeViewer.idl:7: [ImplementedInPrivateScript, OnlyExposedToPrivateScript] void transformDocumentToTreeView(); What's the point of moving ...
6 years, 3 months ago (2014-08-26 05:40:55 UTC) #8
eseidel
6 years, 3 months ago (2014-08-26 05:40:55 UTC) #9
vivekg
https://codereview.chromium.org/479753003/diff/20001/Source/core/xml/XMLTreeViewer.h File Source/core/xml/XMLTreeViewer.h (right): https://codereview.chromium.org/479753003/diff/20001/Source/core/xml/XMLTreeViewer.h#newcode39 Source/core/xml/XMLTreeViewer.h:39: class XMLTreeViewer FINAL : public GarbageCollectedFinalized<XMLTreeViewer>, public ScriptWrappable { ...
6 years, 3 months ago (2014-08-26 05:41:48 UTC) #10
vivekg
On 2014/08/26 at 05:39:28, eseidel wrote: > https://codereview.chromium.org/479753003/diff/1/Source/core/xml/XMLTreeViewer.h > File Source/core/xml/XMLTreeViewer.h (right): > > https://codereview.chromium.org/479753003/diff/1/Source/core/xml/XMLTreeViewer.h#newcode39 ...
6 years, 3 months ago (2014-08-26 05:49:19 UTC) #11
vivekg
https://codereview.chromium.org/479753003/diff/20001/Source/core/xml/XMLTreeViewer.idl File Source/core/xml/XMLTreeViewer.idl (right): https://codereview.chromium.org/479753003/diff/20001/Source/core/xml/XMLTreeViewer.idl#newcode7 Source/core/xml/XMLTreeViewer.idl:7: [ImplementedInPrivateScript, OnlyExposedToPrivateScript] void transformDocumentToTreeView(); On 2014/08/26 at 05:40:54, eseidel ...
6 years, 3 months ago (2014-08-26 05:51:58 UTC) #12
eseidel
Sorry, I misunderstood how that idl function worked. I see now that this is entirely ...
6 years, 3 months ago (2014-08-26 05:54:17 UTC) #13
eseidel
https://codereview.chromium.org/479753003/diff/20001/Source/core/core_generated.gyp File Source/core/core_generated.gyp (right): https://codereview.chromium.org/479753003/diff/20001/Source/core/core_generated.gyp#newcode154 Source/core/core_generated.gyp:154: 'xml/XMLTreeViewer.js', do these need to be root relative? I'm ...
6 years, 3 months ago (2014-08-26 05:56:18 UTC) #14
haraken
https://codereview.chromium.org/479753003/diff/20001/Source/core/xml/XMLTreeViewer.h File Source/core/xml/XMLTreeViewer.h (right): https://codereview.chromium.org/479753003/diff/20001/Source/core/xml/XMLTreeViewer.h#newcode39 Source/core/xml/XMLTreeViewer.h:39: class XMLTreeViewer FINAL : public GarbageCollectedFinalized<XMLTreeViewer>, public ScriptWrappable { ...
6 years, 3 months ago (2014-08-26 05:57:49 UTC) #15
haraken
On 2014/08/26 05:54:17, eseidel wrote: > Sorry, I misunderstood how that idl function worked. I ...
6 years, 3 months ago (2014-08-26 06:00:45 UTC) #16
haraken
On 2014/08/26 05:49:19, vivekg_ wrote: > On 2014/08/26 at 05:39:28, eseidel wrote: > > > ...
6 years, 3 months ago (2014-08-26 06:03:37 UTC) #17
vivekg
On 2014/08/26 at 06:03:37, haraken wrote: > On 2014/08/26 05:49:19, vivekg_ wrote: > > On ...
6 years, 3 months ago (2014-08-26 07:42:41 UTC) #18
haraken
Mostly looks good. Just to confirm: XMLTreeViewer.js is basically just a copy of the old ...
6 years, 3 months ago (2014-08-26 08:05:08 UTC) #19
haraken
https://codereview.chromium.org/479753003/diff/40001/Source/core/xml/XMLTreeViewer.idl File Source/core/xml/XMLTreeViewer.idl (right): https://codereview.chromium.org/479753003/diff/40001/Source/core/xml/XMLTreeViewer.idl#newcode5 Source/core/xml/XMLTreeViewer.idl:5: [ImplementedInPrivateScript, OnlyExposedToPrivateScript] void transformDocumentToTreeView(); abarth@: I feel OK with ...
6 years, 3 months ago (2014-08-26 08:08:19 UTC) #20
vivekg
On 2014/08/26 at 08:05:08, haraken wrote: > Mostly looks good. > > Just to confirm: ...
6 years, 3 months ago (2014-08-26 08:52:46 UTC) #21
haraken
LGTM, but let's wait for abarth@. https://codereview.chromium.org/479753003/diff/60001/Source/core/xml/DocumentXMLTreeViewer.h File Source/core/xml/DocumentXMLTreeViewer.h (right): https://codereview.chromium.org/479753003/diff/60001/Source/core/xml/DocumentXMLTreeViewer.h#newcode11 Source/core/xml/DocumentXMLTreeViewer.h:11: // DocumentXMLTreeViewer.h. We ...
6 years, 3 months ago (2014-08-26 08:56:19 UTC) #22
vivekg
On 2014/08/26 at 08:56:19, haraken wrote: > LGTM, but let's wait for abarth@. > Sure, ...
6 years, 3 months ago (2014-08-26 09:00:33 UTC) #23
haraken
vivekg@: Looks like xmlviewer/extensions-api.html is failing. abarth@: Would you take a look at the private ...
6 years, 3 months ago (2014-08-27 01:09:16 UTC) #24
abarth-chromium
On 2014/08/27 at 01:09:16, haraken wrote: > abarth@: Would you take a look at the ...
6 years, 3 months ago (2014-08-27 17:15:31 UTC) #25
vivekg
I have changed the code to bubble the load event further so that the extension ...
6 years, 3 months ago (2014-08-28 08:07:16 UTC) #26
vivekg
vivekg@chromium.org changed reviewers: + pfeldman@chromium.org
6 years, 3 months ago (2014-08-28 08:13:08 UTC) #27
vivekg
+pfeldman for deprecation of onAfterWebkitXMLViewerLoaded
6 years, 3 months ago (2014-08-28 08:13:08 UTC) #28
haraken
haraken@chromium.org changed reviewers: + yurys@chromium.org
6 years, 3 months ago (2014-08-28 08:33:24 UTC) #29
haraken
Looks like pfeldman@ is OOO for a while... yurys@, abarth@: In a nutshell, we are ...
6 years, 3 months ago (2014-08-28 08:33:24 UTC) #30
yurys
yurys@chromium.org changed reviewers: + vsevik@chromium.org
6 years, 3 months ago (2014-08-28 11:00:02 UTC) #31
yurys
+vsevik who worked with this code.
6 years, 3 months ago (2014-08-28 11:00:02 UTC) #32
vsevik
I know two extensions for prettifying XML: XV - XML Viewer (https://chrome.google.com/webstore/detail/xv-%E2%80%94-xml-viewer/eeocglpgjdpaefaedpblffpeebgmgddk?hl=en) and XML Tree ...
6 years, 3 months ago (2014-08-28 11:58:18 UTC) #33
vsevik
Also, why don't you remove XMLViewer.js/css files in the same patch?
6 years, 3 months ago (2014-08-28 11:58:56 UTC) #34
haraken
On 2014/08/28 11:58:18, vsevik wrote: > I know two extensions for prettifying XML: XV - ...
6 years, 3 months ago (2014-08-28 12:00:06 UTC) #35
vivekg
On 2014/08/28 at 11:58:56, vsevik wrote: > Also, why don't you remove XMLViewer.js/css files in ...
6 years, 3 months ago (2014-09-02 06:56:54 UTC) #36
vivekg
On 2014/08/28 at 11:58:18, vsevik wrote: > I know two extensions for prettifying XML: XV ...
6 years, 3 months ago (2014-09-02 07:00:14 UTC) #37
haraken
On 2014/09/02 07:00:14, vivekg_ wrote: > On 2014/08/28 at 11:58:18, vsevik wrote: > > I ...
6 years, 3 months ago (2014-09-02 07:27:31 UTC) #38
vivekg
On 2014/09/02 at 07:27:31, haraken wrote: > On 2014/09/02 07:00:14, vivekg_ wrote: > > On ...
6 years, 3 months ago (2014-09-02 07:28:48 UTC) #39
PhistucK
A drive by review. I only highlighted a few cases, but the entire file should ...
6 years, 3 months ago (2014-09-02 07:36:45 UTC) #41
yosin_UTC9
https://codereview.chromium.org/479753003/diff/100001/Source/core/xml/DocumentXMLTreeViewer.idl File Source/core/xml/DocumentXMLTreeViewer.idl (right): https://codereview.chromium.org/479753003/diff/100001/Source/core/xml/DocumentXMLTreeViewer.idl#newcode5 Source/core/xml/DocumentXMLTreeViewer.idl:5: partial interface Document { nit: Could you add extended ...
6 years, 3 months ago (2014-09-02 08:01:55 UTC) #43
vivekg
On 2014/09/02 at 08:01:55, yosin wrote: > https://codereview.chromium.org/479753003/diff/100001/Source/core/xml/DocumentXMLTreeViewer.idl > File Source/core/xml/DocumentXMLTreeViewer.idl (right): > > https://codereview.chromium.org/479753003/diff/100001/Source/core/xml/DocumentXMLTreeViewer.idl#newcode5 ...
6 years, 3 months ago (2014-09-02 09:00:31 UTC) #44
vivekg
On 2014/09/02 at 07:36:45, phistuck wrote: > A drive by review. > > I only ...
6 years, 3 months ago (2014-09-02 09:02:23 UTC) #45
haraken
LGTM. Let's wait for a couple of days to give folks a chance to comment ...
6 years, 3 months ago (2014-09-02 09:10:45 UTC) #46
vivekg
On 2014/09/02 at 09:10:45, haraken wrote: > LGTM. > > Let's wait for a couple ...
6 years, 3 months ago (2014-09-02 09:14:13 UTC) #47
haraken
On 2014/09/02 09:14:13, vivekg_ wrote: > On 2014/09/02 at 09:10:45, haraken wrote: > > LGTM. ...
6 years, 3 months ago (2014-09-03 16:01:23 UTC) #48
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vivek.vg@samsung.com/479753003/120001
6 years, 3 months ago (2014-09-03 16:12:58 UTC) #50
commit-bot: I haz the power
6 years, 3 months ago (2014-09-03 16:54:43 UTC) #51
Message was sent while issue was closed.
Committed patchset #7 (id:120001) as 181328

Powered by Google App Engine
This is Rietveld 408576698