903174 - Move GonkNativeWindow to /widget/gonk/nativewindow

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Sotaro Ikeda [:sotaro]]

+++ This bug was initially created as a clone of Bug #871364 +++

GonkNativeWindow is present under /dom/camera. It's usage is not only for camera, but also used for video decoding. And in future, it might be used for other usages. It is better to move GonkNativeWindow to more correct place. From Bug 871364 comment #28, /widget/gonk/nativewindow was choosed.

This bug handle only current GonkNativeWindow and do not handle android 4.3's one.

Comment 1

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 1 - Move GonkNativeWindow and GonkNativeWindowClient to /widget/gonk/nativewindow/

Comment 2

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 2 - Modify /dom/camera/moz.build

Comment 3

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 3 - Modify /widget/gonk/moz.build

Comment 4

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 4 - Modify /widget/gonk/nativewindow/Makefile.in and moz.build

Comment 5

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 5 - /toolkit/library/Makefile.in

Comment 6

[Sotaro Ikeda [:sotaro]]

Comment on attachment 787883 [details] [diff] [review]
patch part 1 - Move GonkNativeWindow and GonkNativeWindowClient to /widget/gonk/nativewindow/

:mwu, can you reivew the patch?

Comment 7

[Sotaro Ikeda [:sotaro]]

Comment on attachment 787883 [details] [diff] [review]
patch part 1 - Move GonkNativeWindow and GonkNativeWindowClient to /widget/gonk/nativewindow/

Change reviewer to vlad, mwu is PTO this week.

Comment 8

[Sotaro Ikeda [:sotaro]]

nical, can I commit the patches in this bug before Bug 858914 fixed?

Comment 9

[Sotaro Ikeda [:sotaro]]

Attachment: patch - roll up patch

roll up patch. Carry "vladimir: review+".
And Fix a nit(revert AudioParameter.cpp).

Comment 10

[Sotaro Ikeda [:sotaro]]

https://tbpl.mozilla.org/?tree=Try&rev=cb109636efe9

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/0249777a97f6

Comment 12

[Nicolas Silva [:nical]]

(In reply to Sotaro Ikeda [:sotaro] from comment #8)
> nical, can I commit the patches in this bug before Bug 858914 fixed?

Sure! thanks for asking

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0249777a97f6

Comment 14

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

This needed review from a build peer. Please take note that the policy is

> if a non-list change in a Makefile.in isn't reviewed by a build peer,
> it doesn't land or gets backed out

Second, why did this mess up the blame by not using hg rename? That's a pretty annoying thing to do.

Comment 15

[Sotaro Ikeda [:sotaro]]

(In reply to :Ms2ger from comment #14)
> This needed review from a build peer. Please take note that the policy is
> 
> > if a non-list change in a Makefile.in isn't reviewed by a build peer,
> > it doesn't land or gets backed out

Thanks for the advice. I will ask a review for them.

> Second, why did this mess up the blame by not using hg rename? That's a
> pretty annoying thing to do.

Sorry, next time I use hg rename.

Comment 16

[Sotaro Ikeda [:sotaro]]

Comment on attachment 787886 [details] [diff] [review]
patch part 4 - Modify /widget/gonk/nativewindow/Makefile.in and moz.build

Mike, can you review the patch?

Comment 17

[Sotaro Ikeda [:sotaro]]

Comment on attachment 787888 [details] [diff] [review]
patch part 5 - /toolkit/library/Makefile.in

Mike, can you review the patch? Sorry for the post review request.

Comment 18

[Mike Hommey [:glandium]]

Comment on attachment 787886 [details] [diff] [review]
patch part 4 - Modify /widget/gonk/nativewindow/Makefile.in and moz.build

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

::: widget/gonk/nativewindow/Makefile.in
@@ +9,5 @@
> +# Unless required by applicable law or agreed to in writing, software
> +# distributed under the License is distributed on an "AS IS" BASIS,
> +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> +# See the License for the specific language governing permissions and
> +# limitations under the License.

Build files should probably stay MPL2.

@@ +23,5 @@
> +FAIL_ON_WARNINGS := 1
> +STL_FLAGS=
> +
> +include $(topsrcdir)/config/rules.mk
> +include $(topsrcdir)/ipc/chromium/chromium-config.mk

You probably don't need chromium-config.mk.

::: widget/gonk/nativewindow/moz.build
@@ +20,5 @@
> +    'GonkNativeWindow.h',
> +    'GonkNativeWindowClient.h',
> +]
> +
> +EXPORTS += [

You can merge the two lists.

Comment 19

[Mike Hommey [:glandium]]

Comment on attachment 787888 [details] [diff] [review]
patch part 5 - /toolkit/library/Makefile.in

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

::: toolkit/library/Makefile.in
@@ +453,5 @@
>    -lstagefright_foundation \
>    -lstagefright_omx \
>    -lbinder \
>    -lgui \
> +  $(DEPTH)/widget/gonk/nativewindow/$(LIB_PREFIX)nativewindow.$(LIB_SUFFIX) \

That should be in SHARED_LIBRARY_LIBS, not OS_LIBS.

Comment 20

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Change to /toolkit/library/Makefile.in

Apply comment #19.

Comment 21

[Sotaro Ikeda [:sotaro]]

Comment on attachment 798898 [details] [diff] [review]
patch - Change to /toolkit/library/Makefile.in

Mike, can you review the patch again?

Comment 22

[bhavana bajaj [:bajaj]]

Sotaro, removing the blocker nomination on this as this seems to have landed in 1.2 and as per our discussion last week the patch in comment#21 is not needed.  Feel free to renom if this does not align with you.