|
|
Created:
3 years, 9 months ago by dougt Modified:
3 years, 9 months ago CC:
chromium-reviews, aboxhall+watch_chromium.org, nektar+watch_chromium.org, yuzo+watch_chromium.org, je_julie, dmazzoni+watch_chromium.org, dtseng+watch_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionDLL Export AXPlatformNodeWin.
In order to use ui::AXPlatformNodeWin from content, we must dll export
these classes.
BUG=703369
Review-Url: https://codereview.chromium.org/2766463002
Cr-Commit-Position: refs/heads/master@{#459376}
Committed: https://chromium.googlesource.com/chromium/src/+/8abfeeb28cc7c6a4fb2bfeca1da3dc82d13ea24b
Patch Set 1 #Patch Set 2 : updated lnk + ws #Patch Set 3 : use NON_EXPORTED_BASE macro \./ #
Total comments: 2
Patch Set 4 : Use NON_EXPORTED_BASE #Messages
Total messages: 25 (14 generated)
The CQ bit was checked by dougt@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
dougt@chromium.org changed reviewers: + dmazzoni@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
The CQ bit was checked by dougt@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Better link to the very latest docs instead of to 2005. https://msdn.microsoft.com/en-us/library/3tdb471s(v=vs.140).aspx
I am wondering if it would be better to just AX_EXPORT a template instantiation of the base class CComRootEx<...>. I fear that if not we might have issues with the component build of Chromium. I am certainly not an expert.
dmazzoni@chromium.org changed reviewers: + ananta@chromium.org, cpu@chromium.org
lgtm, but I was previously unfamiliar with this warning. @cpu or @ananta, could either of you confirm that this looks reasonable?
hmm, I would typically ask rvargas@ but he is gone. There is trickiness exporting classes that are not pod-like Maybe also ask scottmg?
scottmg@chromium.org changed reviewers: + scottmg@chromium.org
We have a macro for this, see NON_EXPORTED_BASE. Please use that instead of disabling the warning inline.
On 2017/03/23 01:16:14, scottmg wrote: > We have a macro for this, see NON_EXPORTED_BASE. Please use that instead of > disabling the warning inline. Awesome suggestion, thx!
Description was changed from ========== DLL Export AXPlatformNodeBase and AXPlatformNodeWin. In order to use ui::AXPlatformNodeWin and ui:AXPlatformNodeBase from content, we must dll export these classes. BUG=703369 ========== to ========== DLL Export AXPlatformNodeWin. In order to use ui::AXPlatformNodeWin from content, we must dll export these classes. BUG=703369 ==========
lgtm with that https://codereview.chromium.org/2766463002/diff/40001/ui/accessibility/platfo... File ui/accessibility/platform/ax_platform_node_win.h (right): https://codereview.chromium.org/2766463002/diff/40001/ui/accessibility/platfo... ui/accessibility/platform/ax_platform_node_win.h:12: #include "base/observer_list.h" #include "base/compiler_specific.h"
https://codereview.chromium.org/2766463002/diff/40001/ui/accessibility/platfo... File ui/accessibility/platform/ax_platform_node_win.h (right): https://codereview.chromium.org/2766463002/diff/40001/ui/accessibility/platfo... ui/accessibility/platform/ax_platform_node_win.h:12: #include "base/observer_list.h" On 2017/03/23 05:00:10, scottmg wrote: > #include "base/compiler_specific.h" Done.
The CQ bit was checked by dougt@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dmazzoni@chromium.org, scottmg@chromium.org Link to the patchset: https://codereview.chromium.org/2766463002/#ps60001 (title: "Use NON_EXPORTED_BASE")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch. Bot data: {"patchset_id": 60001, "attempt_start_ts": 1490335364116370, "parent_rev": "049f833eba3d4dedd636728c504f5edcb44d91e0", "commit_rev": "8abfeeb28cc7c6a4fb2bfeca1da3dc82d13ea24b"}
Message was sent while issue was closed.
Description was changed from ========== DLL Export AXPlatformNodeWin. In order to use ui::AXPlatformNodeWin from content, we must dll export these classes. BUG=703369 ========== to ========== DLL Export AXPlatformNodeWin. In order to use ui::AXPlatformNodeWin from content, we must dll export these classes. BUG=703369 Review-Url: https://codereview.chromium.org/2766463002 Cr-Commit-Position: refs/heads/master@{#459376} Committed: https://chromium.googlesource.com/chromium/src/+/8abfeeb28cc7c6a4fb2bfeca1da3... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://chromium.googlesource.com/chromium/src/+/8abfeeb28cc7c6a4fb2bfeca1da3... |