1278294 - tree logging improvements

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

Attachment: part1: make TreeInfo to log documents

Comment 1

[alexander :surkov (:asurkov)]

Attachment: part2: make UpdateTreeOnRemoval to use TreeInfo method

Comment 2

[alexander :surkov (:asurkov)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2b197e09ce5b50e40d153ac6ae3dfaadb9fd9346
Bug 1278294 - make TreeInfo to log document address, r=yzen

Comment 3

[alexander :surkov (:asurkov)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/452617569aebabeb6da039064460347114a7ba00
Bug 1278294 - UpdateTreeOnRemoval should use logging::TreeInfo, part2, r=yzen

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

This caused build bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=29725823&repo=mozilla-inbound

 Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/acc2acfacebd

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2b197e09ce5b

Comment 6

[alexander :surkov (:asurkov)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1a4ecd8c393b2239e959742925f70ceb3ef165a6
 Bug 1278294 - UpdateTreeOnRemoval should use logging::TreeInfo, part2, r=yzen

Comment 7

[alexander :surkov (:asurkov)]

Attachment: part3: log doc address for text changes

Comment 8

[alexander :surkov (:asurkov)]

Attachment: part4: all other tree loggings to log doc address

Comment 9

[alexander :surkov (:asurkov)]

Attachment: part5: tree logging for single insertions

Comment 10

[alexander :surkov (:asurkov)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f0006e4f551ead7f84b1ba79ec7f39e5c39a6e61
Bug 1278294 - text logging should log document address, part3, r=yzen

Comment 11

[Yura Zenevich [:yzen] (please use "needinfo")]

Comment on attachment 8761264 [details] [diff] [review]
part5: tree logging for single insertions

Review of attachment 8761264 [details] [diff] [review]:
-----------------------------------------------------------------

thanks

::: accessible/generic/DocAccessible.cpp
@@ +1819,5 @@
> +                    aContainer);
> +#endif
> +
> +#ifdef A11Y_LOG
> +    logging::TreeInfo("traversing an inserted node", logging::eVerbose,

nit: indentation.

Comment 12

[alexander :surkov (:asurkov)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b1f370d930d5d4347c38bb99a5def3bee6d91bd5
Bug 1278294 - make all other tree loggings to log document address, part4, r=yzen

Comment 13

[alexander :surkov (:asurkov)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e9cd625ccf89aafd0c9fa5ac292160c43587c55b
Bug 1278294 - add tree logging for single content insertion, part5, r=yzen

Comment 14

[alexander :surkov (:asurkov)]

Attachment: part6: isolate tree dumping into separate method for easier reuse

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/1a4ecd8c393b

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f0006e4f551e
https://hg.mozilla.org/mozilla-central/rev/b1f370d930d5
https://hg.mozilla.org/mozilla-central/rev/e9cd625ccf89

Comment 17

[alexander :surkov (:asurkov)]

Attachment: part7: DOM tree logging

I found it useful to add in the code when needed for debugging proposes.

Comment 18

[Yura Zenevich [:yzen] (please use "needinfo")]

Comment on attachment 8761315 [details] [diff] [review]
part6: isolate tree dumping into separate method for easier reuse

Review of attachment 8761315 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good , thanks

Comment 19

[Yura Zenevich [:yzen] (please use "needinfo")]

Comment on attachment 8762142 [details] [diff] [review]
part7: DOM tree logging

Review of attachment 8762142 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. Just a quick question: does it make sense to assign root->Get...() calls to a variable to avoid calling the method twice? Or it doesn't really affect performance?

Comment 20

[alexander :surkov (:asurkov)]

(In reply to Yura Zenevich [:yzen] from comment #19)
> Comment on attachment 8762142 [details] [diff] [review]
> part7: DOM tree logging
> 
> Review of attachment 8762142 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good. Just a quick question: does it make sense to assign
> root->Get...() calls to a variable to avoid calling the method twice? Or it
> doesn't really affect performance?

if you call that logging function at all that will hurt your performance :) the logging is not about performance in general. Anyway those Get methods has to be inline methods, and it may be even faster to call them directly, as you don't have to create a local variable.

Comment 21

[Yura Zenevich [:yzen] (please use "needinfo")]

gotcha

Comment 22

[alexander :surkov (:asurkov)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ac276f95b75118731e3ef913f2786858e7c7d206
Bug 1278294 - move accessible tree logging into separate method, part=6, r=yzen

Comment 23

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ac276f95b751

Comment 24

[alexander :surkov (:asurkov)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/46178702a3cae1d9f18228c4d7a0af99a0b0ebcc
Bug 1278294 - add DOM tree logging, part=7, r=yzen

Comment 25

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/46178702a3ca