De-templatize ui::AXPosition

De-templatize ui::AXPosition

This is required to share the Objective C code in BrowserAccessibilityCocoa

BUG=657884

[tapted, 2017-06-15 02:56:39]

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

[commit-bot: I haz the power, 2017-06-15 02:56:44]

Dry run: CQ is trying da patch.

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

[tapted, 2017-06-15 02:59:19]

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

[commit-bot: I haz the power, 2017-06-15 02:59:26]

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-06-15 03:15:35]

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

[commit-bot: I haz the power, 2017-06-15 03:15:36]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[tapted, 2017-06-15 05:20:53]

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

[commit-bot: I haz the power, 2017-06-15 05:21:07]

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-06-15 05:53:39]

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

[commit-bot: I haz the power, 2017-06-15 05:53:40]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[tapted, 2017-06-15 07:15:21]

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

[commit-bot: I haz the power, 2017-06-15 07:15:33]

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-06-15 08:08:17]

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

[commit-bot: I haz the power, 2017-06-15 08:08:18]

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...)

[tapted, 2017-06-15 11:53:26]

Description was changed from

==========
De-templatize ui::AXPosition

This is required to share the Objective C code in BrowserAccessibilityCocoa

BUG=657884
==========

to

==========
De-templatize ui::AXPosition

This is required to share the Objective C code in BrowserAccessibilityCocoa

BUG=657884
==========

[tapted, 2017-06-15 11:53:27]

tapted@chromium.org changed reviewers:
+ dmazzoni@chromium.org

[tapted, 2017-06-15 11:57:44]

Hi Dominic, please take a look. There's lots more to do, but this is probably a
manageable chunk. I think this is landable, but I'm happy to hold off until the
follow-ups are further along. (but of course any feedback you have will help
reduce the churn in the follow-ups too). Thanks!

https://codereview.chromium.org/2934953004/diff/80001/ui/accessibility/ax_pos...
File ui/accessibility/ax_position.h (right):

https://codereview.chromium.org/2934953004/diff/80001/ui/accessibility/ax_pos...
ui/accessibility/ax_position.h:104: std::string str;
of course.. plan is to move all these methods to the .cc in a follow-up

https://codereview.chromium.org/2934953004/diff/80001/ui/accessibility/ax_pos...
ui/accessibility/ax_position.h:1123: }
plan would be to move these Create* methods into a Factory, which I've started
sketching out in
https://codereview.chromium.org/2940923003/diff/1/ui/accessibility/platform/t...

https://codereview.chromium.org/2934953004/diff/80001/ui/accessibility/ax_pos...
ui/accessibility/ax_position.h:1145: static ConcreteInstance
FromBase(AXPositionInstance instance) {
I'd hoped to avoid this function.. currently only the Windows code is using it,
in a method marked "todo remove this method".

https://codereview.chromium.org/2934953004/diff/80001/ui/accessibility/ax_ran...
File ui/accessibility/ax_range.h (left):

https://codereview.chromium.org/2934953004/diff/80001/ui/accessibility/ax_ran...
ui/accessibility/ax_range.h:50: if (this != other) {
I removed this method since it's not currently instantiated (otherwise this line
fails since `this` is a pointer but `other` is a ref).

[dmazzoni, 2017-06-15 19:23:00]

dmazzoni@chromium.org changed reviewers:
+ nektar@chromium.org

[dmazzoni, 2017-06-15 19:23:01]

+nektar

Nektarios, do you have any concerns?

[tapted, 2017-06-19 10:41:41]

There's another landable chunk in https://codereview.chromium.org/2940923003/ so
you can get a feel for where I'm heading with this :)

And, Dominic, you suggested earlier over email that we meet, so I'll set
something up that might work. Happy to discuss more there. Thanks!

[chromium-reviews, 2017-06-19 14:33:05]

Let's meet to discuss because I don't quite understand the need for this.
The reason I created a template in the first place is so that we'll have 
an instantiation for web view in AXPlatformPosition and one for Views in 
AXNodePosition.
Is using ui/accessibility/platform/ax_node_position.cc not possible?
Why do we need a class that will be returning Cocoa objects? Couldn't we 
use AXNode objects for positions and map to Cocoa objects only when 
needed? That's what we do in Web view. We use BrowserAccessibility 
objects for positions and map to Cocoa objects only when needed.

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.