GeckoView was created to switch between normal and private themes. And later functionality for LWTheme was added. But GeckoView was never made to listen for LWTheme messages by itself. It's better to make GeckoView listen to LWTheme and update their the theme, instead of a parent layout containing GeckoView doing it.

Phew! I did it! Killed all possible regressions! Basically GeckoView is made to support LWTheme. However, there are few things like Awesomebar Tab indicator, the URL EditText (CustomEditText) and so on, that shouldn't change where there is a theme change. Instead, it should be changed by the app, as needed. Hence I introduced a "gecko:autoUpdateTheme" attribute that will block any unnecessary theme updates. Checked this patch with normal, private tabs, installing a persona.

Comment on attachment 718717 [details] [diff] [review] Patch I like

(In reply to Mark Finkle (:mfinkle) from comment #2) > Comment on attachment 718717 [details] [diff] [review] > Patch > > I like I stole wesj's refactor :P

Comment on attachment 718717 [details] [diff] [review] Patch Review of attachment 718717 [details] [diff] [review]: ----------------------------------------------------------------- I still dont' love the auto-update bit, but its a good optimization probably. Nice! ::: mobile/android/base/AboutHomeContent.java @@ -759,5 @@ > - mLastTabs.setTheme(isLight); > - mRemoteTabs.setTheme(isLight); > - ((GeckoImageView) findViewById(R.id.abouthome_logo)).setTheme(isLight); > - ((GeckoTextView) findViewById(R.id.top_sites_title)).setTheme(isLight); > - } Yay!

The only other call to setTheme I see is in: http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/AwesomeBarTabs.java#231 Can we kill it too?

(In reply to Wesley Johnston (:wesj) from comment #5) > The only other call to setTheme I see is in: > > http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/ > AwesomeBarTabs.java#231 > > Can we kill it too? Sadly no. This is one of the cases where we cannot "auto update" the theme. The theme has to be set in a weird fashion depending on if the tab is selected or not. (Selected tab should always have black text, no matter what theme it is).

(In reply to Sriram Ramasubramanian [:sriram] from comment #6) > (In reply to Wesley Johnston (:wesj) from comment #5) > > The only other call to setTheme I see is in: > > > > http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/ > > AwesomeBarTabs.java#231 > > > > Can we kill it too? > > Sadly no. This is one of the cases where we cannot "auto update" the theme. > The theme has to be set in a weird fashion depending on if the tab is > selected or not. (Selected tab should always have black text, no matter what > theme it is). Oh, and.. we don't want a race condition between Gecko telling the view to change the theme, and the setTarget used for setting private/normal tabs. Basically that part has to be handled by us. I'll try to optimize that piece of code using states in a separate patch.