Ran ppapi/generators/generator.py

Ran ppapi/generators/generator.py

This CL is a result of running ppapi/generators/generator.py on a clean
source tree. I've been updating some PPAPI interfaces (for example see
https://codereview.chromium.org/1769593002/ and noticed that there is
a lot of changed files after running generator.py).

[servolk, 2016-03-08 23:58:16]

Description was changed from

==========
Ran ppapi/generators/generator.py

Looks like nobody ran ppapi generator.py for a while, since I get a
number of changed files (mostly under thunks/) after running
generator.py on a clean source tree.
==========

to

==========
Ran ppapi/generators/generator.py

This CL is a result of running ppapi/generators/generator.py on a clean
source tree. I've been updating some PPAPI interfaces (for example see
https://codereview.chromium.org/1769593002/ and noticed that there is
a lot of changed files after running generator.py).
==========

[servolk, 2016-03-08 23:58:17]

servolk@chromium.org changed reviewers:
+ raymes@chromium.org

[servolk, 2016-03-09 00:00:39]

On 2016/03/08 23:58:17, servolk wrote:
> mailto:servolk@chromium.org changed reviewers:
> + mailto:raymes@chromium.org

Most of the changes seem to be only in code formatting, but there are also some
actual changes thrown in (e.g. in ppapi/thunk/ppb_flash_font_file_thunk.cc) so
please review carefully.

[raymes, 2016-03-09 00:25:33]

On 2016/03/09 00:00:39, servolk wrote:
> On 2016/03/08 23:58:17, servolk wrote:
> > mailto:servolk@chromium.org changed reviewers:
> > + mailto:raymes@chromium.org
> 
> Most of the changes seem to be only in code formatting, but there are also
some
> actual changes thrown in (e.g. in ppapi/thunk/ppb_flash_font_file_thunk.cc) so
> please review carefully.

Hmm do you think this is because someone ran git cl format on the thunks? What
does this look like if you run git cl format? 

(it would be good to distinguish the functional changes here from the formatting
ones if possible).

[servolk, 2016-03-09 00:33:54]

On 2016/03/09 00:25:33, raymes wrote:
> On 2016/03/09 00:00:39, servolk wrote:
> > On 2016/03/08 23:58:17, servolk wrote:
> > > mailto:servolk@chromium.org changed reviewers:
> > > + mailto:raymes@chromium.org
> > 
> > Most of the changes seem to be only in code formatting, but there are also
> some
> > actual changes thrown in (e.g. in ppapi/thunk/ppb_flash_font_file_thunk.cc)
so
> > please review carefully.
> 
> Hmm do you think this is because someone ran git cl format on the thunks? What
> does this look like if you run git cl format? 
> 
> (it would be good to distinguish the functional changes here from the
formatting
> ones if possible).

I'm not sure what happened exactly, but running 'git cl format ppapi' on a clean
source tree produces no changes, and running 'git cl format ppapi' after running
ppapi/generators/generator.py also doesn't revert any of the changes, which
makes me think git cl format is somehow ignoring the generated files, but
generated files writted by generator.py are somehow still processed with git cl
format.
I'm also surprised to see that there are build failures after running
generator.py. So far all the failures seem to be due to function signature
mismatch in ppapi/thunk/ppb_graphics_3d_api.h:
FAILED: cd ../../ppapi; python ../native_client/build/build_nexe.py --root ..
--product-dir ../out/Release/xyz --config-name Release -t
../native_client/toolchain/ --arch x86-64 --build newlib_nlib_clang --name
../out/Release/gen/tc_irt/lib64/libppapi_shared_nacl.a --objdir
../out/Release/obj/ppapi/ppapi_shared_nacl.gen/irt-x86-64/ppapi_shared_nacl
"--include-dirs=../out/Release/gen/tc_newlib/include .. \"../out/Release/gen\"
.. ../third_party/khronos ../gpu" "--compile_flags=-O2 -g -Wall
-fdiagnostics-show-option -Werror  -Wno-unused-function -Wno-char-subscripts
-Wno-c++11-extensions -Wno-unnamed-type-template-args -Wno-extra-semi
-Wno-unused-private-field -Wno-char-subscripts -Wno-unused-function
\"-std=gnu++11\"  -Os -fno-exceptions -ffunction-sections -fdata-sections
-fasynchronous-unwind-tables -fomit-frame-pointer -integrated-as" --gomadir
/b/build/slave/linux_chromeos/build/src/build/goma/client
"--defines=\"__STDC_LIMIT_MACROS=1\" \"__STDC_FORMAT_MACROS=1\"
\"_GNU_SOURCE=1\" \"_POSIX_C_SOURCE=199506\" \"_XOPEN_SOURCE=600\"
\"DYNAMIC_ANNOTATIONS_ENABLED=1\" \"DYNAMIC_ANNOTATIONS_PREFIX=NACL_\"
\"NACL_BUILD_ARCH=x86\" V8_DEPRECATION_WARNINGS \"CLD_VERSION=2\"
\"_FILE_OFFSET_BITS=64\" CHROMIUM_BUILD \"CR_CLANG_REVISION=261368-1\"
UI_COMPOSITOR_IMAGE_TRANSPORT \"USE_AURA=1\" \"USE_ASH=1\" \"USE_PANGO=1\"
\"USE_CAIRO=1\" \"USE_DEFAULT_RENDER_THEME=1\" \"USE_LIBJPEG_TURBO=1\"
\"USE_X11=1\" \"IMAGE_LOADER_EXTENSION=1\" \"ENABLE_WEBRTC=1\"
\"ENABLE_MEDIA_ROUTER=1\" USE_PROPRIETARY_CODECS ENABLE_PEPPER_CDMS
ENABLE_CONFIGURATION_POLICY ENABLE_NOTIFICATIONS \"ENABLE_TOPCHROME_MD=1\"
\"ENABLE_WAYLAND_SERVER=1\" USE_UDEV \"DCHECK_ALWAYS_ON=1\"
FIELDTRIAL_TESTING_ENABLED \"ENABLE_TASK_MANAGER=1\" \"ENABLE_EXTENSIONS=1\"
\"ENABLE_PDF=1\" \"ENABLE_PLUGINS=1\" \"ENABLE_SESSION_SERVICE=1\"
\"ENABLE_THEMES=1\" \"ENABLE_AUTOFILL_DIALOG=1\" \"ENABLE_PRINTING=1\"
\"ENABLE_PRINT_PREVIEW=1\" \"ENABLE_SPELLCHECK=1\"
\"ENABLE_CAPTIVE_PORTAL_DETECTION=1\" \"ENABLE_APP_LIST=1\"
\"ENABLE_SUPERVISED_USERS=1\" \"ENABLE_MDNS=1\" \"ENABLE_SERVICE_DISCOVERY=1\"
V8_USE_EXTERNAL_STARTUP_DATA FULL_SAFE_BROWSING SAFE_BROWSING_CSD
SAFE_BROWSING_DB_LOCAL \"USE_LIBPCI=1\" \"USE_OPENSSL=1\"
\"USE_OPENSSL_CERTS=1\" __STDC_CONSTANT_MACROS __STDC_FORMAT_MACROS
PPAPI_SHARED_IMPLEMENTATION PPAPI_THUNK_IMPLEMENTATION"
"--link_flags=-B../out/Release/gen/tc_irt/lib64   -Wl,--gc-sections
-nodefaultlibs -lc++ -lm -lirt_browser -Wl,--start-group -lc -lnacl -lgcc
-lgcc_eh -Wl,--end-group"
"--source-list=../out/gypfiles/ppapi/irt-x86-64.ppapi_shared_nacl.source_list.gypcmd"
thunk/ppb_graphics_3d_thunk.cc:85:70: error: too few arguments to function call,
expected 2, have 1
  return enter.SetResult(enter.object()->SwapBuffers(enter.callback()));
                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~                 ^
../ppapi/thunk/ppb_graphics_3d_api.h:35:3: note: 'SwapBuffers' declared here
  virtual int32_t SwapBuffers(scoped_refptr<TrackedCallback> callback,
  ^
1 error generated.

But I'm not sure what's the right thing to do here. Any ideas?

[raymes, 2016-03-09 01:57:15]

The two real errors are because people changed the thunk without
regenerating it. This should give a PRESUBMIT warning but quite possibly
people ignored it.

I'm not sure how the formatting changes happened though.

On Wed, 9 Mar 2016 at 11:33 <servolk@chromium.org> wrote:

> On 2016/03/09 00:25:33, raymes wrote:
> > On 2016/03/09 00:00:39, servolk wrote:
> > > On 2016/03/08 23:58:17, servolk wrote:
> > > > mailto:servolk@chromium.org changed reviewers:
> > > > + mailto:raymes@chromium.org
> > >
> > > Most of the changes seem to be only in code formatting, but there are
> also
> > some
> > > actual changes thrown in (e.g. in
> ppapi/thunk/ppb_flash_font_file_thunk.cc)
> so
> > > please review carefully.
> >
> > Hmm do you think this is because someone ran git cl format on the
> thunks? What
> > does this look like if you run git cl format?
> >
> > (it would be good to distinguish the functional changes here from the
> formatting
> > ones if possible).
>
> I'm not sure what happened exactly, but running 'git cl format ppapi' on a
> clean
> source tree produces no changes, and running 'git cl format ppapi' after
> running
> ppapi/generators/generator.py also doesn't revert any of the changes, which
> makes me think git cl format is somehow ignoring the generated files, but
> generated files writted by generator.py are somehow still processed with
> git cl
> format.
> I'm also surprised to see that there are build failures after running
> generator.py. So far all the failures seem to be due to function signature
> mismatch in ppapi/thunk/ppb_graphics_3d_api.h:
> FAILED: cd ../../ppapi; python ../native_client/build/build_nexe.py --root
> ..
> --product-dir ../out/Release/xyz --config-name Release -t
> ../native_client/toolchain/ --arch x86-64 --build newlib_nlib_clang --name
> ../out/Release/gen/tc_irt/lib64/libppapi_shared_nacl.a --objdir
> ../out/Release/obj/ppapi/ppapi_shared_nacl.gen/irt-x86-64/ppapi_shared_nacl
> "--include-dirs=../out/Release/gen/tc_newlib/include ..
> \"../out/Release/gen\"
> .. ../third_party/khronos ../gpu" "--compile_flags=-O2 -g -Wall
> -fdiagnostics-show-option -Werror -Wno-unused-function -Wno-char-subscripts
> -Wno-c++11-extensions -Wno-unnamed-type-template-args -Wno-extra-semi
> -Wno-unused-private-field -Wno-char-subscripts -Wno-unused-function
> \"-std=gnu++11\" -Os -fno-exceptions -ffunction-sections -fdata-sections
> -fasynchronous-unwind-tables -fomit-frame-pointer -integrated-as" --gomadir
> /b/build/slave/linux_chromeos/build/src/build/goma/client
> "--defines=\"__STDC_LIMIT_MACROS=1\" \"__STDC_FORMAT_MACROS=1\"
> \"_GNU_SOURCE=1\" \"_POSIX_C_SOURCE=199506\" \"_XOPEN_SOURCE=600\"
> \"DYNAMIC_ANNOTATIONS_ENABLED=1\" \"DYNAMIC_ANNOTATIONS_PREFIX=NACL_\"
> \"NACL_BUILD_ARCH=x86\" V8_DEPRECATION_WARNINGS \"CLD_VERSION=2\"
> \"_FILE_OFFSET_BITS=64\" CHROMIUM_BUILD \"CR_CLANG_REVISION=261368-1\"
> UI_COMPOSITOR_IMAGE_TRANSPORT \"USE_AURA=1\" \"USE_ASH=1\" \"USE_PANGO=1\"
> \"USE_CAIRO=1\" \"USE_DEFAULT_RENDER_THEME=1\" \"USE_LIBJPEG_TURBO=1\"
> \"USE_X11=1\" \"IMAGE_LOADER_EXTENSION=1\" \"ENABLE_WEBRTC=1\"
> \"ENABLE_MEDIA_ROUTER=1\" USE_PROPRIETARY_CODECS ENABLE_PEPPER_CDMS
> ENABLE_CONFIGURATION_POLICY ENABLE_NOTIFICATIONS \"ENABLE_TOPCHROME_MD=1\"
> \"ENABLE_WAYLAND_SERVER=1\" USE_UDEV \"DCHECK_ALWAYS_ON=1\"
> FIELDTRIAL_TESTING_ENABLED \"ENABLE_TASK_MANAGER=1\"
> \"ENABLE_EXTENSIONS=1\"
> \"ENABLE_PDF=1\" \"ENABLE_PLUGINS=1\" \"ENABLE_SESSION_SERVICE=1\"
> \"ENABLE_THEMES=1\" \"ENABLE_AUTOFILL_DIALOG=1\" \"ENABLE_PRINTING=1\"
> \"ENABLE_PRINT_PREVIEW=1\" \"ENABLE_SPELLCHECK=1\"
> \"ENABLE_CAPTIVE_PORTAL_DETECTION=1\" \"ENABLE_APP_LIST=1\"
> \"ENABLE_SUPERVISED_USERS=1\" \"ENABLE_MDNS=1\"
> \"ENABLE_SERVICE_DISCOVERY=1\"
> V8_USE_EXTERNAL_STARTUP_DATA FULL_SAFE_BROWSING SAFE_BROWSING_CSD
> SAFE_BROWSING_DB_LOCAL \"USE_LIBPCI=1\" \"USE_OPENSSL=1\"
> \"USE_OPENSSL_CERTS=1\" __STDC_CONSTANT_MACROS __STDC_FORMAT_MACROS
> PPAPI_SHARED_IMPLEMENTATION PPAPI_THUNK_IMPLEMENTATION"
> "--link_flags=-B../out/Release/gen/tc_irt/lib64 -Wl,--gc-sections
> -nodefaultlibs -lc++ -lm -lirt_browser -Wl,--start-group -lc -lnacl -lgcc
> -lgcc_eh -Wl,--end-group"
>
>
"--source-list=../out/gypfiles/ppapi/irt-x86-64.ppapi_shared_nacl.source_list.gypcmd"
> thunk/ppb_graphics_3d_thunk.cc:85:70: error: too few arguments to function
> call,
> expected 2, have 1
> return enter.SetResult(enter.object()->SwapBuffers(enter.callback()));
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
> ../ppapi/thunk/ppb_graphics_3d_api.h:35:3: note: 'SwapBuffers' declared
> here
> virtual int32_t SwapBuffers(scoped_refptr<TrackedCallback> callback,
> ^
> 1 error generated.
>
> But I'm not sure what's the right thing to do here. Any ideas?
>
> https://codereview.chromium.org/1778683003/
>

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

[raymes, 2016-03-09 02:02:47]

The references to stdint.h were added manually to the thunks - that's why
they're being removed when regenerating. So we should look at why they were
added or add them to the generator.

On Wed, 9 Mar 2016 at 12:57 Raymes Khoury <raymes@chromium.org> wrote:

> The two real errors are because people changed the thunk without
> regenerating it. This should give a PRESUBMIT warning but quite possibly
> people ignored it.
>
> I'm not sure how the formatting changes happened though.
>
> On Wed, 9 Mar 2016 at 11:33 <servolk@chromium.org> wrote:
>
>> On 2016/03/09 00:25:33, raymes wrote:
>> > On 2016/03/09 00:00:39, servolk wrote:
>> > > On 2016/03/08 23:58:17, servolk wrote:
>> > > > mailto:servolk@chromium.org changed reviewers:
>> > > > + mailto:raymes@chromium.org
>> > >
>> > > Most of the changes seem to be only in code formatting, but there are
>> also
>> > some
>> > > actual changes thrown in (e.g. in
>> ppapi/thunk/ppb_flash_font_file_thunk.cc)
>> so
>> > > please review carefully.
>> >
>> > Hmm do you think this is because someone ran git cl format on the
>> thunks? What
>> > does this look like if you run git cl format?
>> >
>> > (it would be good to distinguish the functional changes here from the
>> formatting
>> > ones if possible).
>>
>> I'm not sure what happened exactly, but running 'git cl format ppapi' on
>> a clean
>> source tree produces no changes, and running 'git cl format ppapi' after
>> running
>> ppapi/generators/generator.py also doesn't revert any of the changes,
>> which
>> makes me think git cl format is somehow ignoring the generated files, but
>> generated files writted by generator.py are somehow still processed with
>> git cl
>> format.
>> I'm also surprised to see that there are build failures after running
>> generator.py. So far all the failures seem to be due to function signature
>> mismatch in ppapi/thunk/ppb_graphics_3d_api.h:
>> FAILED: cd ../../ppapi; python ../native_client/build/build_nexe.py
>> --root ..
>> --product-dir ../out/Release/xyz --config-name Release -t
>> ../native_client/toolchain/ --arch x86-64 --build newlib_nlib_clang --name
>> ../out/Release/gen/tc_irt/lib64/libppapi_shared_nacl.a --objdir
>>
>> ../out/Release/obj/ppapi/ppapi_shared_nacl.gen/irt-x86-64/ppapi_shared_nacl
>> "--include-dirs=../out/Release/gen/tc_newlib/include ..
>> \"../out/Release/gen\"
>> .. ../third_party/khronos ../gpu" "--compile_flags=-O2 -g -Wall
>> -fdiagnostics-show-option -Werror -Wno-unused-function
>> -Wno-char-subscripts
>> -Wno-c++11-extensions -Wno-unnamed-type-template-args -Wno-extra-semi
>> -Wno-unused-private-field -Wno-char-subscripts -Wno-unused-function
>> \"-std=gnu++11\" -Os -fno-exceptions -ffunction-sections -fdata-sections
>> -fasynchronous-unwind-tables -fomit-frame-pointer -integrated-as"
>> --gomadir
>> /b/build/slave/linux_chromeos/build/src/build/goma/client
>> "--defines=\"__STDC_LIMIT_MACROS=1\" \"__STDC_FORMAT_MACROS=1\"
>> \"_GNU_SOURCE=1\" \"_POSIX_C_SOURCE=199506\" \"_XOPEN_SOURCE=600\"
>> \"DYNAMIC_ANNOTATIONS_ENABLED=1\" \"DYNAMIC_ANNOTATIONS_PREFIX=NACL_\"
>> \"NACL_BUILD_ARCH=x86\" V8_DEPRECATION_WARNINGS \"CLD_VERSION=2\"
>> \"_FILE_OFFSET_BITS=64\" CHROMIUM_BUILD \"CR_CLANG_REVISION=261368-1\"
>> UI_COMPOSITOR_IMAGE_TRANSPORT \"USE_AURA=1\" \"USE_ASH=1\" \"USE_PANGO=1\"
>> \"USE_CAIRO=1\" \"USE_DEFAULT_RENDER_THEME=1\" \"USE_LIBJPEG_TURBO=1\"
>> \"USE_X11=1\" \"IMAGE_LOADER_EXTENSION=1\" \"ENABLE_WEBRTC=1\"
>> \"ENABLE_MEDIA_ROUTER=1\" USE_PROPRIETARY_CODECS ENABLE_PEPPER_CDMS
>> ENABLE_CONFIGURATION_POLICY ENABLE_NOTIFICATIONS \"ENABLE_TOPCHROME_MD=1\"
>> \"ENABLE_WAYLAND_SERVER=1\" USE_UDEV \"DCHECK_ALWAYS_ON=1\"
>> FIELDTRIAL_TESTING_ENABLED \"ENABLE_TASK_MANAGER=1\"
>> \"ENABLE_EXTENSIONS=1\"
>> \"ENABLE_PDF=1\" \"ENABLE_PLUGINS=1\" \"ENABLE_SESSION_SERVICE=1\"
>> \"ENABLE_THEMES=1\" \"ENABLE_AUTOFILL_DIALOG=1\" \"ENABLE_PRINTING=1\"
>> \"ENABLE_PRINT_PREVIEW=1\" \"ENABLE_SPELLCHECK=1\"
>> \"ENABLE_CAPTIVE_PORTAL_DETECTION=1\" \"ENABLE_APP_LIST=1\"
>> \"ENABLE_SUPERVISED_USERS=1\" \"ENABLE_MDNS=1\"
>> \"ENABLE_SERVICE_DISCOVERY=1\"
>> V8_USE_EXTERNAL_STARTUP_DATA FULL_SAFE_BROWSING SAFE_BROWSING_CSD
>> SAFE_BROWSING_DB_LOCAL \"USE_LIBPCI=1\" \"USE_OPENSSL=1\"
>> \"USE_OPENSSL_CERTS=1\" __STDC_CONSTANT_MACROS __STDC_FORMAT_MACROS
>> PPAPI_SHARED_IMPLEMENTATION PPAPI_THUNK_IMPLEMENTATION"
>> "--link_flags=-B../out/Release/gen/tc_irt/lib64 -Wl,--gc-sections
>> -nodefaultlibs -lc++ -lm -lirt_browser -Wl,--start-group -lc -lnacl -lgcc
>> -lgcc_eh -Wl,--end-group"
>>
>>
"--source-list=../out/gypfiles/ppapi/irt-x86-64.ppapi_shared_nacl.source_list.gypcmd"
>> thunk/ppb_graphics_3d_thunk.cc:85:70: error: too few arguments to
>> function call,
>> expected 2, have 1
>> return enter.SetResult(enter.object()->SwapBuffers(enter.callback()));
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
>> ../ppapi/thunk/ppb_graphics_3d_api.h:35:3: note: 'SwapBuffers' declared
>> here
>> virtual int32_t SwapBuffers(scoped_refptr<TrackedCallback> callback,
>> ^
>> 1 error generated.
>>
>> But I'm not sure what's the right thing to do here. Any ideas?
>>
>> https://codereview.chromium.org/1778683003/
>>
>

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

[raymes, 2016-03-09 02:08:55]

The formatting changes happened via a manual clang format:
https://codereview.chromium.org/238923007

I filed https://bugs.chromium.org/p/chromium/issues/detail?id=593205

If you need to use the generator, I'd recommend just running it on the IDLs
you're touching or reverting the files you're not interested in until we
fix the bug (unless you want to help with that :)

Thanks!


On Wed, 9 Mar 2016 at 13:02 Raymes Khoury <raymes@chromium.org> wrote:

> The references to stdint.h were added manually to the thunks - that's why
> they're being removed when regenerating. So we should look at why they were
> added or add them to the generator.
>
> On Wed, 9 Mar 2016 at 12:57 Raymes Khoury <raymes@chromium.org> wrote:
>
>> The two real errors are because people changed the thunk without
>> regenerating it. This should give a PRESUBMIT warning but quite possibly
>> people ignored it.
>>
>> I'm not sure how the formatting changes happened though.
>>
>> On Wed, 9 Mar 2016 at 11:33 <servolk@chromium.org> wrote:
>>
>>> On 2016/03/09 00:25:33, raymes wrote:
>>> > On 2016/03/09 00:00:39, servolk wrote:
>>> > > On 2016/03/08 23:58:17, servolk wrote:
>>> > > > mailto:servolk@chromium.org changed reviewers:
>>> > > > + mailto:raymes@chromium.org
>>> > >
>>> > > Most of the changes seem to be only in code formatting, but there
>>> are also
>>> > some
>>> > > actual changes thrown in (e.g. in
>>> ppapi/thunk/ppb_flash_font_file_thunk.cc)
>>> so
>>> > > please review carefully.
>>> >
>>> > Hmm do you think this is because someone ran git cl format on the
>>> thunks? What
>>> > does this look like if you run git cl format?
>>> >
>>> > (it would be good to distinguish the functional changes here from the
>>> formatting
>>> > ones if possible).
>>>
>>> I'm not sure what happened exactly, but running 'git cl format ppapi' on
>>> a clean
>>> source tree produces no changes, and running 'git cl format ppapi' after
>>> running
>>> ppapi/generators/generator.py also doesn't revert any of the changes,
>>> which
>>> makes me think git cl format is somehow ignoring the generated files, but
>>> generated files writted by generator.py are somehow still processed with
>>> git cl
>>> format.
>>> I'm also surprised to see that there are build failures after running
>>> generator.py. So far all the failures seem to be due to function
>>> signature
>>> mismatch in ppapi/thunk/ppb_graphics_3d_api.h:
>>> FAILED: cd ../../ppapi; python ../native_client/build/build_nexe.py
>>> --root ..
>>> --product-dir ../out/Release/xyz --config-name Release -t
>>> ../native_client/toolchain/ --arch x86-64 --build newlib_nlib_clang
>>> --name
>>> ../out/Release/gen/tc_irt/lib64/libppapi_shared_nacl.a --objdir
>>>
>>> ../out/Release/obj/ppapi/ppapi_shared_nacl.gen/irt-x86-64/ppapi_shared_nacl
>>> "--include-dirs=../out/Release/gen/tc_newlib/include ..
>>> \"../out/Release/gen\"
>>> .. ../third_party/khronos ../gpu" "--compile_flags=-O2 -g -Wall
>>> -fdiagnostics-show-option -Werror -Wno-unused-function
>>> -Wno-char-subscripts
>>> -Wno-c++11-extensions -Wno-unnamed-type-template-args -Wno-extra-semi
>>> -Wno-unused-private-field -Wno-char-subscripts -Wno-unused-function
>>> \"-std=gnu++11\" -Os -fno-exceptions -ffunction-sections -fdata-sections
>>> -fasynchronous-unwind-tables -fomit-frame-pointer -integrated-as"
>>> --gomadir
>>> /b/build/slave/linux_chromeos/build/src/build/goma/client
>>> "--defines=\"__STDC_LIMIT_MACROS=1\" \"__STDC_FORMAT_MACROS=1\"
>>> \"_GNU_SOURCE=1\" \"_POSIX_C_SOURCE=199506\" \"_XOPEN_SOURCE=600\"
>>> \"DYNAMIC_ANNOTATIONS_ENABLED=1\" \"DYNAMIC_ANNOTATIONS_PREFIX=NACL_\"
>>> \"NACL_BUILD_ARCH=x86\" V8_DEPRECATION_WARNINGS \"CLD_VERSION=2\"
>>> \"_FILE_OFFSET_BITS=64\" CHROMIUM_BUILD \"CR_CLANG_REVISION=261368-1\"
>>> UI_COMPOSITOR_IMAGE_TRANSPORT \"USE_AURA=1\" \"USE_ASH=1\"
>>> \"USE_PANGO=1\"
>>> \"USE_CAIRO=1\" \"USE_DEFAULT_RENDER_THEME=1\" \"USE_LIBJPEG_TURBO=1\"
>>> \"USE_X11=1\" \"IMAGE_LOADER_EXTENSION=1\" \"ENABLE_WEBRTC=1\"
>>> \"ENABLE_MEDIA_ROUTER=1\" USE_PROPRIETARY_CODECS ENABLE_PEPPER_CDMS
>>> ENABLE_CONFIGURATION_POLICY ENABLE_NOTIFICATIONS
>>> \"ENABLE_TOPCHROME_MD=1\"
>>> \"ENABLE_WAYLAND_SERVER=1\" USE_UDEV \"DCHECK_ALWAYS_ON=1\"
>>> FIELDTRIAL_TESTING_ENABLED \"ENABLE_TASK_MANAGER=1\"
>>> \"ENABLE_EXTENSIONS=1\"
>>> \"ENABLE_PDF=1\" \"ENABLE_PLUGINS=1\" \"ENABLE_SESSION_SERVICE=1\"
>>> \"ENABLE_THEMES=1\" \"ENABLE_AUTOFILL_DIALOG=1\" \"ENABLE_PRINTING=1\"
>>> \"ENABLE_PRINT_PREVIEW=1\" \"ENABLE_SPELLCHECK=1\"
>>> \"ENABLE_CAPTIVE_PORTAL_DETECTION=1\" \"ENABLE_APP_LIST=1\"
>>> \"ENABLE_SUPERVISED_USERS=1\" \"ENABLE_MDNS=1\"
>>> \"ENABLE_SERVICE_DISCOVERY=1\"
>>> V8_USE_EXTERNAL_STARTUP_DATA FULL_SAFE_BROWSING SAFE_BROWSING_CSD
>>> SAFE_BROWSING_DB_LOCAL \"USE_LIBPCI=1\" \"USE_OPENSSL=1\"
>>> \"USE_OPENSSL_CERTS=1\" __STDC_CONSTANT_MACROS __STDC_FORMAT_MACROS
>>> PPAPI_SHARED_IMPLEMENTATION PPAPI_THUNK_IMPLEMENTATION"
>>> "--link_flags=-B../out/Release/gen/tc_irt/lib64 -Wl,--gc-sections
>>> -nodefaultlibs -lc++ -lm -lirt_browser -Wl,--start-group -lc -lnacl -lgcc
>>> -lgcc_eh -Wl,--end-group"
>>>
>>>
"--source-list=../out/gypfiles/ppapi/irt-x86-64.ppapi_shared_nacl.source_list.gypcmd"
>>> thunk/ppb_graphics_3d_thunk.cc:85:70: error: too few arguments to
>>> function call,
>>> expected 2, have 1
>>> return enter.SetResult(enter.object()->SwapBuffers(enter.callback()));
>>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
>>> ../ppapi/thunk/ppb_graphics_3d_api.h:35:3: note: 'SwapBuffers' declared
>>> here
>>> virtual int32_t SwapBuffers(scoped_refptr<TrackedCallback> callback,
>>> ^
>>> 1 error generated.
>>>
>>> But I'm not sure what's the right thing to do here. Any ideas?
>>>
>>> https://codereview.chromium.org/1778683003/
>>>
>>

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

[chromium-reviews, 2016-03-09 02:09:58]

Any ideas how to fix that issue with the SwapBuffers? Should we add
the new parameter to the SwapBuffers method in
ppapi/thunk/ppb_graphics_3d_thunk.cc and make sure the caller passes
in all the necessary parameters? Or are we going to need a new version
of PPB_Graphics3D_1_0 interface and keep PPB_Graphics3D_1_0 with a
single-parameter SwapBuffers for backward-compatilibity? Sorry, I've
never touched PPAPI before, so I don't know what's the right thing to
do.


On Tue, Mar 8, 2016 at 5:57 PM, Raymes Khoury <raymes@chromium.org> wrote:
> The two real errors are because people changed the thunk without
> regenerating it. This should give a PRESUBMIT warning but quite possibly
> people ignored it.
>
> I'm not sure how the formatting changes happened though.
>
> On Wed, 9 Mar 2016 at 11:33 <servolk@chromium.org> wrote:
>>
>> On 2016/03/09 00:25:33, raymes wrote:
>> > On 2016/03/09 00:00:39, servolk wrote:
>> > > On 2016/03/08 23:58:17, servolk wrote:
>> > > > mailto:servolk@chromium.org changed reviewers:
>> > > > + mailto:raymes@chromium.org
>> > >
>> > > Most of the changes seem to be only in code formatting, but there are
>> > > also
>> > some
>> > > actual changes thrown in (e.g. in
>> > > ppapi/thunk/ppb_flash_font_file_thunk.cc)
>> so
>> > > please review carefully.
>> >
>> > Hmm do you think this is because someone ran git cl format on the
>> > thunks? What
>> > does this look like if you run git cl format?
>> >
>> > (it would be good to distinguish the functional changes here from the
>> formatting
>> > ones if possible).
>>
>> I'm not sure what happened exactly, but running 'git cl format ppapi' on a
>> clean
>> source tree produces no changes, and running 'git cl format ppapi' after
>> running
>> ppapi/generators/generator.py also doesn't revert any of the changes,
>> which
>> makes me think git cl format is somehow ignoring the generated files, but
>> generated files writted by generator.py are somehow still processed with
>> git cl
>> format.
>> I'm also surprised to see that there are build failures after running
>> generator.py. So far all the failures seem to be due to function signature
>> mismatch in ppapi/thunk/ppb_graphics_3d_api.h:
>> FAILED: cd ../../ppapi; python ../native_client/build/build_nexe.py --root
>> ..
>> --product-dir ../out/Release/xyz --config-name Release -t
>> ../native_client/toolchain/ --arch x86-64 --build newlib_nlib_clang --name
>> ../out/Release/gen/tc_irt/lib64/libppapi_shared_nacl.a --objdir
>>
>> ../out/Release/obj/ppapi/ppapi_shared_nacl.gen/irt-x86-64/ppapi_shared_nacl
>> "--include-dirs=../out/Release/gen/tc_newlib/include ..
>> \"../out/Release/gen\"
>> .. ../third_party/khronos ../gpu" "--compile_flags=-O2 -g -Wall
>> -fdiagnostics-show-option -Werror -Wno-unused-function
>> -Wno-char-subscripts
>> -Wno-c++11-extensions -Wno-unnamed-type-template-args -Wno-extra-semi
>> -Wno-unused-private-field -Wno-char-subscripts -Wno-unused-function
>> \"-std=gnu++11\" -Os -fno-exceptions -ffunction-sections -fdata-sections
>> -fasynchronous-unwind-tables -fomit-frame-pointer -integrated-as"
>> --gomadir
>> /b/build/slave/linux_chromeos/build/src/build/goma/client
>> "--defines=\"__STDC_LIMIT_MACROS=1\" \"__STDC_FORMAT_MACROS=1\"
>> \"_GNU_SOURCE=1\" \"_POSIX_C_SOURCE=199506\" \"_XOPEN_SOURCE=600\"
>> \"DYNAMIC_ANNOTATIONS_ENABLED=1\" \"DYNAMIC_ANNOTATIONS_PREFIX=NACL_\"
>> \"NACL_BUILD_ARCH=x86\" V8_DEPRECATION_WARNINGS \"CLD_VERSION=2\"
>> \"_FILE_OFFSET_BITS=64\" CHROMIUM_BUILD \"CR_CLANG_REVISION=261368-1\"
>> UI_COMPOSITOR_IMAGE_TRANSPORT \"USE_AURA=1\" \"USE_ASH=1\" \"USE_PANGO=1\"
>> \"USE_CAIRO=1\" \"USE_DEFAULT_RENDER_THEME=1\" \"USE_LIBJPEG_TURBO=1\"
>> \"USE_X11=1\" \"IMAGE_LOADER_EXTENSION=1\" \"ENABLE_WEBRTC=1\"
>> \"ENABLE_MEDIA_ROUTER=1\" USE_PROPRIETARY_CODECS ENABLE_PEPPER_CDMS
>> ENABLE_CONFIGURATION_POLICY ENABLE_NOTIFICATIONS \"ENABLE_TOPCHROME_MD=1\"
>> \"ENABLE_WAYLAND_SERVER=1\" USE_UDEV \"DCHECK_ALWAYS_ON=1\"
>> FIELDTRIAL_TESTING_ENABLED \"ENABLE_TASK_MANAGER=1\"
>> \"ENABLE_EXTENSIONS=1\"
>> \"ENABLE_PDF=1\" \"ENABLE_PLUGINS=1\" \"ENABLE_SESSION_SERVICE=1\"
>> \"ENABLE_THEMES=1\" \"ENABLE_AUTOFILL_DIALOG=1\" \"ENABLE_PRINTING=1\"
>> \"ENABLE_PRINT_PREVIEW=1\" \"ENABLE_SPELLCHECK=1\"
>> \"ENABLE_CAPTIVE_PORTAL_DETECTION=1\" \"ENABLE_APP_LIST=1\"
>> \"ENABLE_SUPERVISED_USERS=1\" \"ENABLE_MDNS=1\"
>> \"ENABLE_SERVICE_DISCOVERY=1\"
>> V8_USE_EXTERNAL_STARTUP_DATA FULL_SAFE_BROWSING SAFE_BROWSING_CSD
>> SAFE_BROWSING_DB_LOCAL \"USE_LIBPCI=1\" \"USE_OPENSSL=1\"
>> \"USE_OPENSSL_CERTS=1\" __STDC_CONSTANT_MACROS __STDC_FORMAT_MACROS
>> PPAPI_SHARED_IMPLEMENTATION PPAPI_THUNK_IMPLEMENTATION"
>> "--link_flags=-B../out/Release/gen/tc_irt/lib64 -Wl,--gc-sections
>> -nodefaultlibs -lc++ -lm -lirt_browser -Wl,--start-group -lc -lnacl -lgcc
>> -lgcc_eh -Wl,--end-group"
>>
>>
"--source-list=../out/gypfiles/ppapi/irt-x86-64.ppapi_shared_nacl.source_list.gypcmd"
>> thunk/ppb_graphics_3d_thunk.cc:85:70: error: too few arguments to function
>> call,
>> expected 2, have 1
>> return enter.SetResult(enter.object()->SwapBuffers(enter.callback()));
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
>> ../ppapi/thunk/ppb_graphics_3d_api.h:35:3: note: 'SwapBuffers' declared
>> here
>> virtual int32_t SwapBuffers(scoped_refptr<TrackedCallback> callback,
>> ^
>> 1 error generated.
>>
>> But I'm not sure what's the right thing to do here. Any ideas?
>>
>> https://codereview.chromium.org/1778683003/

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

[chromium-reviews, 2016-03-09 02:12:14]

Ok, yeah, that's what I did for my VP9 CL for now. I've created this
CL hoping it to be an easy fix, thinking I would only need to run
generator.py and everything will be fine. But we are running into
issues that are outside of my expertise area, so I'll leave it up to
you PPAPI experts to figure out. I'd be glad to help, but I just don't
know enough about PPAPI :)

On Tue, Mar 8, 2016 at 6:08 PM, Raymes Khoury <raymes@chromium.org> wrote:
> The formatting changes happened via a manual clang format:
> https://codereview.chromium.org/238923007
>
> I filed https://bugs.chromium.org/p/chromium/issues/detail?id=593205
>
> If you need to use the generator, I'd recommend just running it on the IDLs
> you're touching or reverting the files you're not interested in until we fix
> the bug (unless you want to help with that :)
>
> Thanks!
>
>
> On Wed, 9 Mar 2016 at 13:02 Raymes Khoury <raymes@chromium.org> wrote:
>>
>> The references to stdint.h were added manually to the thunks - that's why
>> they're being removed when regenerating. So we should look at why they were
>> added or add them to the generator.
>>
>> On Wed, 9 Mar 2016 at 12:57 Raymes Khoury <raymes@chromium.org> wrote:
>>>
>>> The two real errors are because people changed the thunk without
>>> regenerating it. This should give a PRESUBMIT warning but quite possibly
>>> people ignored it.
>>>
>>> I'm not sure how the formatting changes happened though.
>>>
>>> On Wed, 9 Mar 2016 at 11:33 <servolk@chromium.org> wrote:
>>>>
>>>> On 2016/03/09 00:25:33, raymes wrote:
>>>> > On 2016/03/09 00:00:39, servolk wrote:
>>>> > > On 2016/03/08 23:58:17, servolk wrote:
>>>> > > > mailto:servolk@chromium.org changed reviewers:
>>>> > > > + mailto:raymes@chromium.org
>>>> > >
>>>> > > Most of the changes seem to be only in code formatting, but there
>>>> > > are also
>>>> > some
>>>> > > actual changes thrown in (e.g. in
>>>> > > ppapi/thunk/ppb_flash_font_file_thunk.cc)
>>>> so
>>>> > > please review carefully.
>>>> >
>>>> > Hmm do you think this is because someone ran git cl format on the
>>>> > thunks? What
>>>> > does this look like if you run git cl format?
>>>> >
>>>> > (it would be good to distinguish the functional changes here from the
>>>> formatting
>>>> > ones if possible).
>>>>
>>>> I'm not sure what happened exactly, but running 'git cl format ppapi' on
>>>> a clean
>>>> source tree produces no changes, and running 'git cl format ppapi' after
>>>> running
>>>> ppapi/generators/generator.py also doesn't revert any of the changes,
>>>> which
>>>> makes me think git cl format is somehow ignoring the generated files,
>>>> but
>>>> generated files writted by generator.py are somehow still processed with
>>>> git cl
>>>> format.
>>>> I'm also surprised to see that there are build failures after running
>>>> generator.py. So far all the failures seem to be due to function
>>>> signature
>>>> mismatch in ppapi/thunk/ppb_graphics_3d_api.h:
>>>> FAILED: cd ../../ppapi; python ../native_client/build/build_nexe.py
>>>> --root ..
>>>> --product-dir ../out/Release/xyz --config-name Release -t
>>>> ../native_client/toolchain/ --arch x86-64 --build newlib_nlib_clang
>>>> --name
>>>> ../out/Release/gen/tc_irt/lib64/libppapi_shared_nacl.a --objdir
>>>>
>>>>
../out/Release/obj/ppapi/ppapi_shared_nacl.gen/irt-x86-64/ppapi_shared_nacl
>>>> "--include-dirs=../out/Release/gen/tc_newlib/include ..
>>>> \"../out/Release/gen\"
>>>> .. ../third_party/khronos ../gpu" "--compile_flags=-O2 -g -Wall
>>>> -fdiagnostics-show-option -Werror -Wno-unused-function
>>>> -Wno-char-subscripts
>>>> -Wno-c++11-extensions -Wno-unnamed-type-template-args -Wno-extra-semi
>>>> -Wno-unused-private-field -Wno-char-subscripts -Wno-unused-function
>>>> \"-std=gnu++11\" -Os -fno-exceptions -ffunction-sections -fdata-sections
>>>> -fasynchronous-unwind-tables -fomit-frame-pointer -integrated-as"
>>>> --gomadir
>>>> /b/build/slave/linux_chromeos/build/src/build/goma/client
>>>> "--defines=\"__STDC_LIMIT_MACROS=1\" \"__STDC_FORMAT_MACROS=1\"
>>>> \"_GNU_SOURCE=1\" \"_POSIX_C_SOURCE=199506\" \"_XOPEN_SOURCE=600\"
>>>> \"DYNAMIC_ANNOTATIONS_ENABLED=1\" \"DYNAMIC_ANNOTATIONS_PREFIX=NACL_\"
>>>> \"NACL_BUILD_ARCH=x86\" V8_DEPRECATION_WARNINGS \"CLD_VERSION=2\"
>>>> \"_FILE_OFFSET_BITS=64\" CHROMIUM_BUILD \"CR_CLANG_REVISION=261368-1\"
>>>> UI_COMPOSITOR_IMAGE_TRANSPORT \"USE_AURA=1\" \"USE_ASH=1\"
>>>> \"USE_PANGO=1\"
>>>> \"USE_CAIRO=1\" \"USE_DEFAULT_RENDER_THEME=1\" \"USE_LIBJPEG_TURBO=1\"
>>>> \"USE_X11=1\" \"IMAGE_LOADER_EXTENSION=1\" \"ENABLE_WEBRTC=1\"
>>>> \"ENABLE_MEDIA_ROUTER=1\" USE_PROPRIETARY_CODECS ENABLE_PEPPER_CDMS
>>>> ENABLE_CONFIGURATION_POLICY ENABLE_NOTIFICATIONS
>>>> \"ENABLE_TOPCHROME_MD=1\"
>>>> \"ENABLE_WAYLAND_SERVER=1\" USE_UDEV \"DCHECK_ALWAYS_ON=1\"
>>>> FIELDTRIAL_TESTING_ENABLED \"ENABLE_TASK_MANAGER=1\"
>>>> \"ENABLE_EXTENSIONS=1\"
>>>> \"ENABLE_PDF=1\" \"ENABLE_PLUGINS=1\" \"ENABLE_SESSION_SERVICE=1\"
>>>> \"ENABLE_THEMES=1\" \"ENABLE_AUTOFILL_DIALOG=1\" \"ENABLE_PRINTING=1\"
>>>> \"ENABLE_PRINT_PREVIEW=1\" \"ENABLE_SPELLCHECK=1\"
>>>> \"ENABLE_CAPTIVE_PORTAL_DETECTION=1\" \"ENABLE_APP_LIST=1\"
>>>> \"ENABLE_SUPERVISED_USERS=1\" \"ENABLE_MDNS=1\"
>>>> \"ENABLE_SERVICE_DISCOVERY=1\"
>>>> V8_USE_EXTERNAL_STARTUP_DATA FULL_SAFE_BROWSING SAFE_BROWSING_CSD
>>>> SAFE_BROWSING_DB_LOCAL \"USE_LIBPCI=1\" \"USE_OPENSSL=1\"
>>>> \"USE_OPENSSL_CERTS=1\" __STDC_CONSTANT_MACROS __STDC_FORMAT_MACROS
>>>> PPAPI_SHARED_IMPLEMENTATION PPAPI_THUNK_IMPLEMENTATION"
>>>> "--link_flags=-B../out/Release/gen/tc_irt/lib64 -Wl,--gc-sections
>>>> -nodefaultlibs -lc++ -lm -lirt_browser -Wl,--start-group -lc -lnacl
>>>> -lgcc
>>>> -lgcc_eh -Wl,--end-group"
>>>>
>>>>
"--source-list=../out/gypfiles/ppapi/irt-x86-64.ppapi_shared_nacl.source_list.gypcmd"
>>>> thunk/ppb_graphics_3d_thunk.cc:85:70: error: too few arguments to
>>>> function call,
>>>> expected 2, have 1
>>>> return enter.SetResult(enter.object()->SwapBuffers(enter.callback()));
>>>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
>>>> ../ppapi/thunk/ppb_graphics_3d_api.h:35:3: note: 'SwapBuffers' declared
>>>> here
>>>> virtual int32_t SwapBuffers(scoped_refptr<TrackedCallback> callback,
>>>> ^
>>>> 1 error generated.
>>>>
>>>> But I'm not sure what's the right thing to do here. Any ideas?
>>>>
>>>> https://codereview.chromium.org/1778683003/

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

[raymes, 2016-03-09 02:15:28]

That's ok I don't expect you to have to deal with this. Unfortunately this
code is all basically in maintenance mode which is why it has rotted a bit.

If you are interested in fixing the graphics 3d issue specifically (no
pressure), I think the right fix would be to add a new function
to PPB_Graphics3D_API so that we have:
  virtual int32_t SwapBuffers(scoped_refptr<TrackedCallback> callback) = 0;
  virtual int32_t SwapBuffersWithSyncToken(scoped_refptr<TrackedCallback>
callback,
                              const gpu::SyncToken& sync_token) = 0;

And then then fix up the non-thunk callsite to call the appriorate function.

Cheers,
Raymes

On Wed, 9 Mar 2016 at 13:09 Sergey Volk <servolk@google.com> wrote:

> Any ideas how to fix that issue with the SwapBuffers? Should we add
> the new parameter to the SwapBuffers method in
> ppapi/thunk/ppb_graphics_3d_thunk.cc and make sure the caller passes
> in all the necessary parameters? Or are we going to need a new version
> of PPB_Graphics3D_1_0 interface and keep PPB_Graphics3D_1_0 with a
> single-parameter SwapBuffers for backward-compatilibity? Sorry, I've
> never touched PPAPI before, so I don't know what's the right thing to
> do.
>
>
> On Tue, Mar 8, 2016 at 5:57 PM, Raymes Khoury <raymes@chromium.org> wrote:
> > The two real errors are because people changed the thunk without
> > regenerating it. This should give a PRESUBMIT warning but quite possibly
> > people ignored it.
> >
> > I'm not sure how the formatting changes happened though.
> >
> > On Wed, 9 Mar 2016 at 11:33 <servolk@chromium.org> wrote:
> >>
> >> On 2016/03/09 00:25:33, raymes wrote:
> >> > On 2016/03/09 00:00:39, servolk wrote:
> >> > > On 2016/03/08 23:58:17, servolk wrote:
> >> > > > mailto:servolk@chromium.org changed reviewers:
> >> > > > + mailto:raymes@chromium.org
> >> > >
> >> > > Most of the changes seem to be only in code formatting, but there
> are
> >> > > also
> >> > some
> >> > > actual changes thrown in (e.g. in
> >> > > ppapi/thunk/ppb_flash_font_file_thunk.cc)
> >> so
> >> > > please review carefully.
> >> >
> >> > Hmm do you think this is because someone ran git cl format on the
> >> > thunks? What
> >> > does this look like if you run git cl format?
> >> >
> >> > (it would be good to distinguish the functional changes here from the
> >> formatting
> >> > ones if possible).
> >>
> >> I'm not sure what happened exactly, but running 'git cl format ppapi'
> on a
> >> clean
> >> source tree produces no changes, and running 'git cl format ppapi' after
> >> running
> >> ppapi/generators/generator.py also doesn't revert any of the changes,
> >> which
> >> makes me think git cl format is somehow ignoring the generated files,
> but
> >> generated files writted by generator.py are somehow still processed with
> >> git cl
> >> format.
> >> I'm also surprised to see that there are build failures after running
> >> generator.py. So far all the failures seem to be due to function
> signature
> >> mismatch in ppapi/thunk/ppb_graphics_3d_api.h:
> >> FAILED: cd ../../ppapi; python ../native_client/build/build_nexe.py
> --root
> >> ..
> >> --product-dir ../out/Release/xyz --config-name Release -t
> >> ../native_client/toolchain/ --arch x86-64 --build newlib_nlib_clang
> --name
> >> ../out/Release/gen/tc_irt/lib64/libppapi_shared_nacl.a --objdir
> >>
> >>
> ../out/Release/obj/ppapi/ppapi_shared_nacl.gen/irt-x86-64/ppapi_shared_nacl
> >> "--include-dirs=../out/Release/gen/tc_newlib/include ..
> >> \"../out/Release/gen\"
> >> .. ../third_party/khronos ../gpu" "--compile_flags=-O2 -g -Wall
> >> -fdiagnostics-show-option -Werror -Wno-unused-function
> >> -Wno-char-subscripts
> >> -Wno-c++11-extensions -Wno-unnamed-type-template-args -Wno-extra-semi
> >> -Wno-unused-private-field -Wno-char-subscripts -Wno-unused-function
> >> \"-std=gnu++11\" -Os -fno-exceptions -ffunction-sections -fdata-sections
> >> -fasynchronous-unwind-tables -fomit-frame-pointer -integrated-as"
> >> --gomadir
> >> /b/build/slave/linux_chromeos/build/src/build/goma/client
> >> "--defines=\"__STDC_LIMIT_MACROS=1\" \"__STDC_FORMAT_MACROS=1\"
> >> \"_GNU_SOURCE=1\" \"_POSIX_C_SOURCE=199506\" \"_XOPEN_SOURCE=600\"
> >> \"DYNAMIC_ANNOTATIONS_ENABLED=1\" \"DYNAMIC_ANNOTATIONS_PREFIX=NACL_\"
> >> \"NACL_BUILD_ARCH=x86\" V8_DEPRECATION_WARNINGS \"CLD_VERSION=2\"
> >> \"_FILE_OFFSET_BITS=64\" CHROMIUM_BUILD \"CR_CLANG_REVISION=261368-1\"
> >> UI_COMPOSITOR_IMAGE_TRANSPORT \"USE_AURA=1\" \"USE_ASH=1\"
> \"USE_PANGO=1\"
> >> \"USE_CAIRO=1\" \"USE_DEFAULT_RENDER_THEME=1\" \"USE_LIBJPEG_TURBO=1\"
> >> \"USE_X11=1\" \"IMAGE_LOADER_EXTENSION=1\" \"ENABLE_WEBRTC=1\"
> >> \"ENABLE_MEDIA_ROUTER=1\" USE_PROPRIETARY_CODECS ENABLE_PEPPER_CDMS
> >> ENABLE_CONFIGURATION_POLICY ENABLE_NOTIFICATIONS
> \"ENABLE_TOPCHROME_MD=1\"
> >> \"ENABLE_WAYLAND_SERVER=1\" USE_UDEV \"DCHECK_ALWAYS_ON=1\"
> >> FIELDTRIAL_TESTING_ENABLED \"ENABLE_TASK_MANAGER=1\"
> >> \"ENABLE_EXTENSIONS=1\"
> >> \"ENABLE_PDF=1\" \"ENABLE_PLUGINS=1\" \"ENABLE_SESSION_SERVICE=1\"
> >> \"ENABLE_THEMES=1\" \"ENABLE_AUTOFILL_DIALOG=1\" \"ENABLE_PRINTING=1\"
> >> \"ENABLE_PRINT_PREVIEW=1\" \"ENABLE_SPELLCHECK=1\"
> >> \"ENABLE_CAPTIVE_PORTAL_DETECTION=1\" \"ENABLE_APP_LIST=1\"
> >> \"ENABLE_SUPERVISED_USERS=1\" \"ENABLE_MDNS=1\"
> >> \"ENABLE_SERVICE_DISCOVERY=1\"
> >> V8_USE_EXTERNAL_STARTUP_DATA FULL_SAFE_BROWSING SAFE_BROWSING_CSD
> >> SAFE_BROWSING_DB_LOCAL \"USE_LIBPCI=1\" \"USE_OPENSSL=1\"
> >> \"USE_OPENSSL_CERTS=1\" __STDC_CONSTANT_MACROS __STDC_FORMAT_MACROS
> >> PPAPI_SHARED_IMPLEMENTATION PPAPI_THUNK_IMPLEMENTATION"
> >> "--link_flags=-B../out/Release/gen/tc_irt/lib64 -Wl,--gc-sections
> >> -nodefaultlibs -lc++ -lm -lirt_browser -Wl,--start-group -lc -lnacl
> -lgcc
> >> -lgcc_eh -Wl,--end-group"
> >>
> >>
>
"--source-list=../out/gypfiles/ppapi/irt-x86-64.ppapi_shared_nacl.source_list.gypcmd"
> >> thunk/ppb_graphics_3d_thunk.cc:85:70: error: too few arguments to
> function
> >> call,
> >> expected 2, have 1
> >> return enter.SetResult(enter.object()->SwapBuffers(enter.callback()));
> >> ~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
> >> ../ppapi/thunk/ppb_graphics_3d_api.h:35:3: note: 'SwapBuffers' declared
> >> here
> >> virtual int32_t SwapBuffers(scoped_refptr<TrackedCallback> callback,
> >> ^
> >> 1 error generated.
> >>
> >> But I'm not sure what's the right thing to do here. Any ideas?
> >>
> >> https://codereview.chromium.org/1778683003/
>

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

[chromium-reviews, 2016-03-09 02:19:22]

Ok, I think I can give it a try tomorrow, but my question is still: is
it ok to add a new function to an already existing/published PPAPI
interface or are we going to need to introduce a new version/revisiong
of PPB_Graphics3D_API interface? Since IIUC strictly speaking adding a
new function might be considered a breaking change for the interface.

On Tue, Mar 8, 2016 at 6:15 PM, Raymes Khoury <raymes@chromium.org> wrote:
> That's ok I don't expect you to have to deal with this. Unfortunately this
> code is all basically in maintenance mode which is why it has rotted a bit.
>
> If you are interested in fixing the graphics 3d issue specifically (no
> pressure), I think the right fix would be to add a new function to
> PPB_Graphics3D_API so that we have:
>   virtual int32_t SwapBuffers(scoped_refptr<TrackedCallback> callback) = 0;
>   virtual int32_t SwapBuffersWithSyncToken(scoped_refptr<TrackedCallback>
> callback,
>                               const gpu::SyncToken& sync_token) = 0;
>
> And then then fix up the non-thunk callsite to call the appriorate function.
>
> Cheers,
> Raymes
>
> On Wed, 9 Mar 2016 at 13:09 Sergey Volk <servolk@google.com> wrote:
>>
>> Any ideas how to fix that issue with the SwapBuffers? Should we add
>> the new parameter to the SwapBuffers method in
>> ppapi/thunk/ppb_graphics_3d_thunk.cc and make sure the caller passes
>> in all the necessary parameters? Or are we going to need a new version
>> of PPB_Graphics3D_1_0 interface and keep PPB_Graphics3D_1_0 with a
>> single-parameter SwapBuffers for backward-compatilibity? Sorry, I've
>> never touched PPAPI before, so I don't know what's the right thing to
>> do.
>>
>>
>> On Tue, Mar 8, 2016 at 5:57 PM, Raymes Khoury <raymes@chromium.org> wrote:
>> > The two real errors are because people changed the thunk without
>> > regenerating it. This should give a PRESUBMIT warning but quite possibly
>> > people ignored it.
>> >
>> > I'm not sure how the formatting changes happened though.
>> >
>> > On Wed, 9 Mar 2016 at 11:33 <servolk@chromium.org> wrote:
>> >>
>> >> On 2016/03/09 00:25:33, raymes wrote:
>> >> > On 2016/03/09 00:00:39, servolk wrote:
>> >> > > On 2016/03/08 23:58:17, servolk wrote:
>> >> > > > mailto:servolk@chromium.org changed reviewers:
>> >> > > > + mailto:raymes@chromium.org
>> >> > >
>> >> > > Most of the changes seem to be only in code formatting, but there
>> >> > > are
>> >> > > also
>> >> > some
>> >> > > actual changes thrown in (e.g. in
>> >> > > ppapi/thunk/ppb_flash_font_file_thunk.cc)
>> >> so
>> >> > > please review carefully.
>> >> >
>> >> > Hmm do you think this is because someone ran git cl format on the
>> >> > thunks? What
>> >> > does this look like if you run git cl format?
>> >> >
>> >> > (it would be good to distinguish the functional changes here from the
>> >> formatting
>> >> > ones if possible).
>> >>
>> >> I'm not sure what happened exactly, but running 'git cl format ppapi'
>> >> on a
>> >> clean
>> >> source tree produces no changes, and running 'git cl format ppapi'
>> >> after
>> >> running
>> >> ppapi/generators/generator.py also doesn't revert any of the changes,
>> >> which
>> >> makes me think git cl format is somehow ignoring the generated files,
>> >> but
>> >> generated files writted by generator.py are somehow still processed
>> >> with
>> >> git cl
>> >> format.
>> >> I'm also surprised to see that there are build failures after running
>> >> generator.py. So far all the failures seem to be due to function
>> >> signature
>> >> mismatch in ppapi/thunk/ppb_graphics_3d_api.h:
>> >> FAILED: cd ../../ppapi; python ../native_client/build/build_nexe.py
>> >> --root
>> >> ..
>> >> --product-dir ../out/Release/xyz --config-name Release -t
>> >> ../native_client/toolchain/ --arch x86-64 --build newlib_nlib_clang
>> >> --name
>> >> ../out/Release/gen/tc_irt/lib64/libppapi_shared_nacl.a --objdir
>> >>
>> >>
>> >>
../out/Release/obj/ppapi/ppapi_shared_nacl.gen/irt-x86-64/ppapi_shared_nacl
>> >> "--include-dirs=../out/Release/gen/tc_newlib/include ..
>> >> \"../out/Release/gen\"
>> >> .. ../third_party/khronos ../gpu" "--compile_flags=-O2 -g -Wall
>> >> -fdiagnostics-show-option -Werror -Wno-unused-function
>> >> -Wno-char-subscripts
>> >> -Wno-c++11-extensions -Wno-unnamed-type-template-args -Wno-extra-semi
>> >> -Wno-unused-private-field -Wno-char-subscripts -Wno-unused-function
>> >> \"-std=gnu++11\" -Os -fno-exceptions -ffunction-sections
>> >> -fdata-sections
>> >> -fasynchronous-unwind-tables -fomit-frame-pointer -integrated-as"
>> >> --gomadir
>> >> /b/build/slave/linux_chromeos/build/src/build/goma/client
>> >> "--defines=\"__STDC_LIMIT_MACROS=1\" \"__STDC_FORMAT_MACROS=1\"
>> >> \"_GNU_SOURCE=1\" \"_POSIX_C_SOURCE=199506\" \"_XOPEN_SOURCE=600\"
>> >> \"DYNAMIC_ANNOTATIONS_ENABLED=1\" \"DYNAMIC_ANNOTATIONS_PREFIX=NACL_\"
>> >> \"NACL_BUILD_ARCH=x86\" V8_DEPRECATION_WARNINGS \"CLD_VERSION=2\"
>> >> \"_FILE_OFFSET_BITS=64\" CHROMIUM_BUILD \"CR_CLANG_REVISION=261368-1\"
>> >> UI_COMPOSITOR_IMAGE_TRANSPORT \"USE_AURA=1\" \"USE_ASH=1\"
>> >> \"USE_PANGO=1\"
>> >> \"USE_CAIRO=1\" \"USE_DEFAULT_RENDER_THEME=1\" \"USE_LIBJPEG_TURBO=1\"
>> >> \"USE_X11=1\" \"IMAGE_LOADER_EXTENSION=1\" \"ENABLE_WEBRTC=1\"
>> >> \"ENABLE_MEDIA_ROUTER=1\" USE_PROPRIETARY_CODECS ENABLE_PEPPER_CDMS
>> >> ENABLE_CONFIGURATION_POLICY ENABLE_NOTIFICATIONS
>> >> \"ENABLE_TOPCHROME_MD=1\"
>> >> \"ENABLE_WAYLAND_SERVER=1\" USE_UDEV \"DCHECK_ALWAYS_ON=1\"
>> >> FIELDTRIAL_TESTING_ENABLED \"ENABLE_TASK_MANAGER=1\"
>> >> \"ENABLE_EXTENSIONS=1\"
>> >> \"ENABLE_PDF=1\" \"ENABLE_PLUGINS=1\" \"ENABLE_SESSION_SERVICE=1\"
>> >> \"ENABLE_THEMES=1\" \"ENABLE_AUTOFILL_DIALOG=1\" \"ENABLE_PRINTING=1\"
>> >> \"ENABLE_PRINT_PREVIEW=1\" \"ENABLE_SPELLCHECK=1\"
>> >> \"ENABLE_CAPTIVE_PORTAL_DETECTION=1\" \"ENABLE_APP_LIST=1\"
>> >> \"ENABLE_SUPERVISED_USERS=1\" \"ENABLE_MDNS=1\"
>> >> \"ENABLE_SERVICE_DISCOVERY=1\"
>> >> V8_USE_EXTERNAL_STARTUP_DATA FULL_SAFE_BROWSING SAFE_BROWSING_CSD
>> >> SAFE_BROWSING_DB_LOCAL \"USE_LIBPCI=1\" \"USE_OPENSSL=1\"
>> >> \"USE_OPENSSL_CERTS=1\" __STDC_CONSTANT_MACROS __STDC_FORMAT_MACROS
>> >> PPAPI_SHARED_IMPLEMENTATION PPAPI_THUNK_IMPLEMENTATION"
>> >> "--link_flags=-B../out/Release/gen/tc_irt/lib64 -Wl,--gc-sections
>> >> -nodefaultlibs -lc++ -lm -lirt_browser -Wl,--start-group -lc -lnacl
>> >> -lgcc
>> >> -lgcc_eh -Wl,--end-group"
>> >>
>> >>
>> >>
"--source-list=../out/gypfiles/ppapi/irt-x86-64.ppapi_shared_nacl.source_list.gypcmd"
>> >> thunk/ppb_graphics_3d_thunk.cc:85:70: error: too few arguments to
>> >> function
>> >> call,
>> >> expected 2, have 1
>> >> return enter.SetResult(enter.object()->SwapBuffers(enter.callback()));
>> >> ~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
>> >> ../ppapi/thunk/ppb_graphics_3d_api.h:35:3: note: 'SwapBuffers' declared
>> >> here
>> >> virtual int32_t SwapBuffers(scoped_refptr<TrackedCallback> callback,
>> >> ^
>> >> 1 error generated.
>> >>
>> >> But I'm not sure what's the right thing to do here. Any ideas?
>> >>
>> >> https://codereview.chromium.org/1778683003/

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