From f936e8bb1b13dd14af8bccf90b9809398a3ba96e Mon Sep 17 00:00:00 2001 From: Margaret Leibovic Date: Sat, 4 Aug 2012 16:34:42 -0700 Subject: [PATCH 01/22] Bug 732336 - (Part 1) Move doorhanger-related event listeners to DoorHangerPopup. r=wesj --- mobile/android/base/BrowserApp.java | 17 +--- mobile/android/base/DoorHangerPopup.java | 104 +++++++++++++++-------- mobile/android/base/GeckoApp.java | 55 ++---------- 3 files changed, 76 insertions(+), 100 deletions(-) diff --git a/mobile/android/base/BrowserApp.java b/mobile/android/base/BrowserApp.java index 3210e0855552..e301934ffba1 100644 --- a/mobile/android/base/BrowserApp.java +++ b/mobile/android/base/BrowserApp.java @@ -212,9 +212,9 @@ abstract public class BrowserApp extends GeckoApp super.finishProfileMigration(); } + // We don't want to call super.initializeChrome in here because we don't + // want to create two DoorHangerPopup instances. @Override void initializeChrome(String uri, Boolean isExternalURL) { - super.initializeChrome(uri, isExternalURL); - mBrowserToolbar.updateBackButton(false); mBrowserToolbar.updateForwardButton(false); @@ -244,6 +244,8 @@ abstract public class BrowserApp extends GeckoApp } mBrowserToolbar.setProgressVisibility(isExternalURL || (mRestoreMode != GeckoAppShell.RESTORE_NONE)); + + mDoorHangerPopup = new DoorHangerPopup(this, mBrowserToolbar.mFavicon); } void toggleChrome(final boolean aShow) { @@ -436,17 +438,6 @@ abstract public class BrowserApp extends GeckoApp }); } - /* Doorhanger notification methods */ - @Override - void updatePopups(final Tab tab) { - mDoorHangerPopup.updatePopup(mBrowserToolbar.mFavicon); - } - - @Override - void addDoorHanger(String message, String value, JSONArray buttons, Tab tab, JSONObject options) { - mDoorHangerPopup.addDoorHanger(message, value, buttons, tab, options, mBrowserToolbar.mFavicon); - } - /* Favicon methods */ private void loadFavicon(final Tab tab) { maybeCancelFaviconLoad(tab); diff --git a/mobile/android/base/DoorHangerPopup.java b/mobile/android/base/DoorHangerPopup.java index 3f3e86021b2f..4c22c30658bc 100644 --- a/mobile/android/base/DoorHangerPopup.java +++ b/mobile/android/base/DoorHangerPopup.java @@ -23,32 +23,77 @@ import android.widget.RelativeLayout; import java.util.HashMap; -public class DoorHangerPopup extends PopupWindow { +public class DoorHangerPopup extends PopupWindow implements GeckoEventListener { private static final String LOGTAG = "GeckoDoorHangerPopup"; - private Context mContext; + private GeckoApp mActivity; + private View mAnchor; private LinearLayout mContent; private boolean mInflated; private ImageView mArrow; private int mArrowWidth; - public DoorHangerPopup(Context aContext) { - super(aContext); - mContext = aContext; + DoorHangerPopup(GeckoApp aActivity, View aAnchor) { + super(aActivity); + mActivity = aActivity; + mAnchor = aAnchor; mInflated = false; - mArrowWidth = aContext.getResources().getDimensionPixelSize(R.dimen.doorhanger_arrow_width); - } + mArrowWidth = aActivity.getResources().getDimensionPixelSize(R.dimen.doorhanger_arrow_width); + + GeckoAppShell.registerGeckoEventListener("Doorhanger:Add", this); + GeckoAppShell.registerGeckoEventListener("Doorhanger:Remove", this); + } + + void destroy() { + GeckoAppShell.unregisterGeckoEventListener("Doorhanger:Add", this); + GeckoAppShell.unregisterGeckoEventListener("Doorhanger:Remove", this); + } + + public void handleMessage(String event, JSONObject geckoObject) { + try { + if (event.equals("Doorhanger:Add")) { + final String message = geckoObject.getString("message"); + final String value = geckoObject.getString("value"); + final JSONArray buttons = geckoObject.getJSONArray("buttons"); + final int tabId = geckoObject.getInt("tabID"); + final JSONObject options = geckoObject.getJSONObject("options"); + + mActivity.runOnUiThread(new Runnable() { + public void run() { + Tab tab = Tabs.getInstance().getTab(tabId); + if (tab != null) + addDoorHanger(message, value, buttons, tab, options); + } + }); + } else if (event.equals("Doorhanger:Remove")) { + final String value = geckoObject.getString("value"); + final int tabId = geckoObject.getInt("tabID"); + + mActivity.runOnUiThread(new Runnable() { + public void run() { + Tab tab = Tabs.getInstance().getTab(tabId); + if (tab == null) + return; + tab.removeDoorHanger(value); + updatePopup(); + } + }); + } + } catch (Exception e) { + Log.e(LOGTAG, "Exception handling message \"" + event + "\":", e); + } + } private void init() { setBackgroundDrawable(new BitmapDrawable()); setOutsideTouchable(true); setFocusable(true); - setWindowLayoutMode(GeckoApp.mAppContext.isTablet() ? ViewGroup.LayoutParams.WRAP_CONTENT : ViewGroup.LayoutParams.FILL_PARENT, + setWindowLayoutMode(mActivity.isTablet() ? ViewGroup.LayoutParams.WRAP_CONTENT : ViewGroup.LayoutParams.FILL_PARENT, ViewGroup.LayoutParams.WRAP_CONTENT); - LayoutInflater inflater = LayoutInflater.from(mContext); + LayoutInflater inflater = LayoutInflater.from(mActivity); RelativeLayout layout = (RelativeLayout) inflater.inflate(R.layout.doorhangerpopup, null); mArrow = (ImageView) layout.findViewById(R.id.doorhanger_arrow); mContent = (LinearLayout) layout.findViewById(R.id.doorhanger_container); @@ -57,10 +102,8 @@ public class DoorHangerPopup extends PopupWindow { mInflated = true; } - public void addDoorHanger(String message, String value, JSONArray buttons, - Tab tab, JSONObject options, View v) { - Log.i(LOGTAG, "Adding a DoorHanger to Tab: " + tab.getId()); - + private void addDoorHanger(String message, String value, JSONArray buttons, + Tab tab, JSONObject options) { if (!mInflated) init(); @@ -91,18 +134,13 @@ public class DoorHangerPopup extends PopupWindow { // Only update the popup if we're adding a notifcation to the selected tab if (tab.equals(Tabs.getInstance().getSelectedTab())) - updatePopup(v); + updatePopup(); } - // Updates popup contents to show doorhangers for the selected tab public void updatePopup() { - updatePopup(null); - } - - public void updatePopup(View v) { Tab tab = Tabs.getInstance().getSelectedTab(); if (tab == null) { - hidePopup(); + dismiss(); return; } @@ -111,7 +149,7 @@ public class DoorHangerPopup extends PopupWindow { HashMap doorHangers = tab.getDoorHangers(); // Hide the popup if there aren't any doorhangers to show if (doorHangers == null || doorHangers.size() == 0) { - hidePopup(); + dismiss(); return; } @@ -129,33 +167,25 @@ public class DoorHangerPopup extends PopupWindow { dh.show(); } - if (v == null) - showAtLocation(((GeckoApp)mContext).getView(), Gravity.TOP, 0, 0); - else - showPopup(v); - } - public void hidePopup() { - if (isShowing()) { - dismiss(); - } - } - - public void showPopup(View v) { fixBackgroundForFirst(); - if (isShowing()) { update(); return; } + // If there's no anchor, just show the popup at the top of the gecko app view. + if (mAnchor == null) { + showAtLocation(mActivity.getView(), Gravity.TOP, 0, 0); + return; + } + // On tablets, we need to position the popup so that the center of the arrow points to the // center of the anchor view. On phones the popup stretches across the entire screen, so the // arrow position is determined by its left margin. - int offset = GeckoApp.mAppContext.isTablet() ? v.getWidth()/2 - mArrowWidth/2 - + int offset = mActivity.isTablet() ? mAnchor.getWidth()/2 - mArrowWidth/2 - ((RelativeLayout.LayoutParams) mArrow.getLayoutParams()).leftMargin : 0; - - showAsDropDown(v, offset, 0); + showAsDropDown(mAnchor, offset, 0); } private void fixBackgroundForFirst() { diff --git a/mobile/android/base/GeckoApp.java b/mobile/android/base/GeckoApp.java index dea4d4132923..2911608337ac 100644 --- a/mobile/android/base/GeckoApp.java +++ b/mobile/android/base/GeckoApp.java @@ -669,10 +669,6 @@ abstract public class GeckoApp mDoorHangerPopup.updatePopup(); } - void addDoorHanger(String message, String value, JSONArray buttons, Tab tab, JSONObject options) { - mDoorHangerPopup.addDoorHanger(message, value, buttons, tab, options, null); - } - void handleLocationChange(final int tabId, final String uri, final String documentURI, final String contentType, final boolean sameDocument) { @@ -937,10 +933,6 @@ abstract public class GeckoApp } else if (event.equals("Content:PageShow")) { final int tabId = message.getInt("tabID"); handlePageShow(tabId); - } else if (event.equals("Doorhanger:Add")) { - handleDoorHanger(message); - } else if (event.equals("Doorhanger:Remove")) { - handleDoorHangerRemove(message); } else if (event.equals("Gecko:Ready")) { sIsGeckoReady = true; final Menu menu = mMenu; @@ -1218,41 +1210,6 @@ abstract public class GeckoApp }); } - void handleDoorHanger(JSONObject geckoObject) throws JSONException { - final String message = geckoObject.getString("message"); - final String value = geckoObject.getString("value"); - final JSONArray buttons = geckoObject.getJSONArray("buttons"); - final int tabId = geckoObject.getInt("tabID"); - final JSONObject options = geckoObject.getJSONObject("options"); - - Log.i(LOGTAG, "DoorHanger received for tab " + tabId + ", msg:" + message); - - mMainHandler.post(new Runnable() { - public void run() { - Tab tab = Tabs.getInstance().getTab(tabId); - if (tab != null) - addDoorHanger(message, value, buttons, tab, options); - } - }); - } - - void handleDoorHangerRemove(JSONObject geckoObject) throws JSONException { - final String value = geckoObject.getString("value"); - final int tabId = geckoObject.getInt("tabID"); - - Log.i(LOGTAG, "Doorhanger:Remove received for tab " + tabId); - - mMainHandler.post(new Runnable() { - public void run() { - Tab tab = Tabs.getInstance().getTab(tabId); - if (tab == null) - return; - tab.removeDoorHanger(value); - updatePopups(tab); - } - }); - } - void handleDocumentStart(int tabId, final boolean showProgress, String uri) { final Tab tab = Tabs.getInstance().getTab(tabId); if (tab == null) @@ -1610,7 +1567,9 @@ abstract public class GeckoApp ((GeckoApplication) getApplication()).addApplicationLifecycleCallbacks(this); } - void initializeChrome(String uri, Boolean isExternalURL) { } + void initializeChrome(String uri, Boolean isExternalURL) { + mDoorHangerPopup = new DoorHangerPopup(this, null); + } private void initialize() { mInitialized = true; @@ -1729,8 +1688,6 @@ abstract public class GeckoApp } mPluginContainer = (AbsoluteLayout) findViewById(R.id.plugin_container); - - mDoorHangerPopup = new DoorHangerPopup(this); mFormAssistPopup = (FormAssistPopup) findViewById(R.id.form_assist_popup); Log.w(LOGTAG, "zerdatime " + SystemClock.uptimeMillis() + " - UI almost up"); @@ -1749,8 +1706,6 @@ abstract public class GeckoApp GeckoAppShell.registerGeckoEventListener("Content:PageShow", this); GeckoAppShell.registerGeckoEventListener("Reader:FaviconRequest", this); GeckoAppShell.registerGeckoEventListener("onCameraCapture", this); - GeckoAppShell.registerGeckoEventListener("Doorhanger:Add", this); - GeckoAppShell.registerGeckoEventListener("Doorhanger:Remove", this); GeckoAppShell.registerGeckoEventListener("Menu:Add", this); GeckoAppShell.registerGeckoEventListener("Menu:Remove", this); GeckoAppShell.registerGeckoEventListener("Gecko:Ready", this); @@ -2114,8 +2069,6 @@ abstract public class GeckoApp GeckoAppShell.unregisterGeckoEventListener("Content:PageShow", this); GeckoAppShell.unregisterGeckoEventListener("Reader:FaviconRequest", this); GeckoAppShell.unregisterGeckoEventListener("onCameraCapture", this); - GeckoAppShell.unregisterGeckoEventListener("Doorhanger:Add", this); - GeckoAppShell.unregisterGeckoEventListener("Doorhanger:Remove", this); GeckoAppShell.unregisterGeckoEventListener("Menu:Add", this); GeckoAppShell.unregisterGeckoEventListener("Menu:Remove", this); GeckoAppShell.unregisterGeckoEventListener("Gecko:Ready", this); @@ -2150,6 +2103,8 @@ abstract public class GeckoApp mLayerController.destroy(); if (mLayerClient != null) mLayerClient.destroy(); + if (mDoorHangerPopup != null) + mDoorHangerPopup.destroy(); if (mFormAssistPopup != null) mFormAssistPopup.destroy(); if (mPromptService != null) From 091ee72c05f8bf0654a39e51a39b3a88b64b0a79 Mon Sep 17 00:00:00 2001 From: Margaret Leibovic Date: Sat, 4 Aug 2012 16:34:44 -0700 Subject: [PATCH 02/22] Bug 732336 - (Part 2) Refactor DoorHanger/Tab to centralize logic in DoorHangerPopup. r=wesj --- mobile/android/base/DoorHanger.java | 109 +++++++------- mobile/android/base/DoorHangerPopup.java | 172 +++++++++++++---------- mobile/android/base/GeckoApp.java | 2 +- mobile/android/base/Tab.java | 40 ------ 4 files changed, 155 insertions(+), 168 deletions(-) diff --git a/mobile/android/base/DoorHanger.java b/mobile/android/base/DoorHanger.java index 6779b9844872..da7cf5725aec 100644 --- a/mobile/android/base/DoorHanger.java +++ b/mobile/android/base/DoorHanger.java @@ -5,6 +5,7 @@ package org.mozilla.gecko; +import org.json.JSONArray; import org.json.JSONException; import org.json.JSONObject; @@ -22,51 +23,79 @@ import android.widget.LinearLayout; import android.widget.TextView; public class DoorHanger extends LinearLayout implements Button.OnClickListener { - private static final String LOGTAG = "DoorHanger"; + private static final String LOGTAG = "GeckoDoorHanger"; - private Context mContext; + private GeckoApp mActivity; + // The popup that holds this doorhanger + private DoorHangerPopup mPopup; private LinearLayout mChoicesLayout; private TextView mTextView; + + // LayoutParams used for adding button layouts static private LayoutParams mLayoutParams; - public Tab mTab; - // value used to identify the notification - private String mValue; + private final int mTabId; + // Value used to identify the notification + private final String mValue; // Optional checkbox added underneath message text private CheckBox mCheckBox; - static private LayoutInflater mInflater; - private int mPersistence = 0; private boolean mPersistWhileVisible = false; private long mTimeout = 0; - public DoorHanger(Context aContext, String aValue) { - super(aContext); + DoorHanger(GeckoApp aActivity, DoorHangerPopup aPopup, int aTabId, String aValue) { + super(aActivity); - mContext = aContext; + mActivity = aActivity; + mPopup = aPopup; + mTabId = aTabId; mValue = aValue; + } + + int getTabId() { + return mTabId; + } + String getValue() { + return mValue; + } + + // Postpone stuff that needs to be done on the main thread + void init(String message, JSONArray buttons, JSONObject options) { setOrientation(VERTICAL); setBackgroundResource(R.drawable.doorhanger_shadow_bg); - if (mInflater == null) - mInflater = LayoutInflater.from(mContext); - - mInflater.inflate(R.layout.doorhanger, this); - hide(); + LayoutInflater.from(mActivity).inflate(R.layout.doorhanger, this); + setVisibility(View.GONE); mTextView = (TextView) findViewById(R.id.doorhanger_title); + mTextView.setText(message); + mChoicesLayout = (LinearLayout) findViewById(R.id.doorhanger_choices); + // Set the doorhanger text and buttons + for (int i = 0; i < buttons.length(); i++) { + try { + JSONObject buttonObject = buttons.getJSONObject(i); + String label = buttonObject.getString("label"); + int callBackId = buttonObject.getInt("callback"); + addButton(label, callBackId); + } catch (JSONException e) { + Log.e(LOGTAG, "Error creating doorhanger button", e); + } + } + + setOptions(options); + } + + private void addButton(String aText, int aCallback) { if (mLayoutParams == null) mLayoutParams = new LayoutParams(LayoutParams.FILL_PARENT, LayoutParams.FILL_PARENT, 1.0f); - } - public void addButton(String aText, int aCallback) { - Button mButton = new Button(mContext); + Button mButton = new Button(mActivity); mButton.setText(aText); mButton.setTag(Integer.toString(aCallback)); mButton.setOnClickListener(this); @@ -81,48 +110,20 @@ public class DoorHanger extends LinearLayout implements Button.OnClickListener { // If the checkbox is being used, pass its value if (mCheckBox != null) response.put("checked", mCheckBox.isChecked()); - } catch (JSONException ex) { - Log.e(LOGTAG, "Error creating onClick response: " + ex); + } catch (JSONException e) { + Log.e(LOGTAG, "Error creating onClick response", e); } GeckoEvent e = GeckoEvent.createBroadcastEvent("Doorhanger:Reply", response.toString()); GeckoAppShell.sendEventToGecko(e); - mTab.removeDoorHanger(mValue); + mPopup.removeDoorHanger(this); // This will hide the doorhanger (and hide the popup if there are no // more doorhangers to show) - ((GeckoApp)mContext).updatePopups(mTab); + mPopup.updatePopup(); } - public void show() { - setVisibility(View.VISIBLE); - } - - public void hide() { - setVisibility(View.GONE); - } - - public boolean isVisible() { - return getVisibility() == View.VISIBLE; - } - - public String getValue() { - return mValue; - } - - public void setText(String aText) { - mTextView.setText(aText); - } - - public Tab getTab() { - return mTab; - } - - public void setTab(Tab tab) { - mTab = tab; - } - - public void setOptions(JSONObject options) { + private void setOptions(JSONObject options) { try { mPersistence = options.getInt("persistence"); } catch (JSONException e) { } @@ -144,7 +145,7 @@ public class DoorHanger extends LinearLayout implements Button.OnClickListener { URLSpan linkSpan = new URLSpan(linkUrl) { @Override public void onClick(View view) { - GeckoApp.mAppContext.loadUrlInTab(this.getURL()); + mActivity.loadUrlInTab(this.getURL()); } }; @@ -167,8 +168,8 @@ public class DoorHanger extends LinearLayout implements Button.OnClickListener { // This method checks with persistence and timeout options to see if // it's okay to remove a doorhanger. - public boolean shouldRemove() { - if (mPersistWhileVisible && GeckoApp.mAppContext.mDoorHangerPopup.isShowing()) { + boolean shouldRemove() { + if (mPersistWhileVisible && mPopup.isShowing()) { // We still want to decrement mPersistence, even if the popup is showing if (mPersistence != 0) mPersistence--; diff --git a/mobile/android/base/DoorHangerPopup.java b/mobile/android/base/DoorHangerPopup.java index 4c22c30658bc..119304fc4f7f 100644 --- a/mobile/android/base/DoorHangerPopup.java +++ b/mobile/android/base/DoorHangerPopup.java @@ -21,7 +21,7 @@ import android.widget.LinearLayout; import android.widget.PopupWindow; import android.widget.RelativeLayout; -import java.util.HashMap; +import java.util.HashSet; public class DoorHangerPopup extends PopupWindow implements GeckoEventListener { private static final String LOGTAG = "GeckoDoorHangerPopup"; @@ -34,6 +34,10 @@ public class DoorHangerPopup extends PopupWindow implements GeckoEventListener { private ImageView mArrow; private int mArrowWidth; + // Stores a set of all active DoorHanger notifications. A DoorHanger is + // uniquely identified by its tabId and value. + private HashSet mDoorHangers; + DoorHangerPopup(GeckoApp aActivity, View aAnchor) { super(aActivity); mActivity = aActivity; @@ -41,6 +45,7 @@ public class DoorHangerPopup extends PopupWindow implements GeckoEventListener { mInflated = false; mArrowWidth = aActivity.getResources().getDimensionPixelSize(R.dimen.doorhanger_arrow_width); + mDoorHangers = new HashSet(); GeckoAppShell.registerGeckoEventListener("Doorhanger:Add", this); GeckoAppShell.registerGeckoEventListener("Doorhanger:Remove", this); @@ -54,29 +59,23 @@ public class DoorHangerPopup extends PopupWindow implements GeckoEventListener { public void handleMessage(String event, JSONObject geckoObject) { try { if (event.equals("Doorhanger:Add")) { - final String message = geckoObject.getString("message"); - final String value = geckoObject.getString("value"); - final JSONArray buttons = geckoObject.getJSONArray("buttons"); - final int tabId = geckoObject.getInt("tabID"); - final JSONObject options = geckoObject.getJSONObject("options"); + int tabId = geckoObject.getInt("tabID"); + String value = geckoObject.getString("value"); + String message = geckoObject.getString("message"); + JSONArray buttons = geckoObject.getJSONArray("buttons"); + JSONObject options = geckoObject.getJSONObject("options"); - mActivity.runOnUiThread(new Runnable() { - public void run() { - Tab tab = Tabs.getInstance().getTab(tabId); - if (tab != null) - addDoorHanger(message, value, buttons, tab, options); - } - }); + addDoorHanger(tabId, value, message, buttons, options); } else if (event.equals("Doorhanger:Remove")) { - final String value = geckoObject.getString("value"); - final int tabId = geckoObject.getInt("tabID"); + int tabId = geckoObject.getInt("tabID"); + String value = geckoObject.getString("value"); + DoorHanger doorHanger = getDoorHanger(tabId, value); + if (doorHanger == null) + return; + removeDoorHanger(doorHanger); mActivity.runOnUiThread(new Runnable() { public void run() { - Tab tab = Tabs.getInstance().getTab(tabId); - if (tab == null) - return; - tab.removeDoorHanger(value); updatePopup(); } }); @@ -102,72 +101,99 @@ public class DoorHangerPopup extends PopupWindow implements GeckoEventListener { mInflated = true; } - private void addDoorHanger(String message, String value, JSONArray buttons, - Tab tab, JSONObject options) { - if (!mInflated) - init(); + void addDoorHanger(final int tabId, final String value, final String message, + final JSONArray buttons, final JSONObject options) { + // Don't add a doorhanger for a tab that doesn't exist + if (Tabs.getInstance().getTab(tabId) == null) + return; // Replace the doorhanger if it already exists - DoorHanger dh = tab.getDoorHanger(value); - if (dh != null) { - tab.removeDoorHanger(value); - } - dh = new DoorHanger(mContent.getContext(), value); - - // Set the doorhanger text and buttons - dh.setText(message); - for (int i = 0; i < buttons.length(); i++) { - try { - JSONObject buttonObject = buttons.getJSONObject(i); - String label = buttonObject.getString("label"); - int callBackId = buttonObject.getInt("callback"); - dh.addButton(label, callBackId); - } catch (JSONException e) { - Log.i(LOGTAG, "JSON throws", e); + DoorHanger oldDoorHanger = getDoorHanger(tabId, value); + if (oldDoorHanger != null) + removeDoorHanger(oldDoorHanger); + + final DoorHanger newDoorHanger = new DoorHanger(mActivity, this, tabId, value); + mDoorHangers.add(newDoorHanger); + + // Update the UI bits on the main thread + mActivity.runOnUiThread(new Runnable() { + public void run() { + if (!mInflated) + init(); + + newDoorHanger.init(message, buttons, options); + mContent.addView(newDoorHanger); + + // Only update the popup if we're adding a notifcation to the selected tab + if (tabId == Tabs.getInstance().getSelectedTab().getId()) + updatePopup(); } - } - dh.setOptions(options); - - dh.setTab(tab); - tab.addDoorHanger(value, dh); - mContent.addView(dh); - - // Only update the popup if we're adding a notifcation to the selected tab - if (tab.equals(Tabs.getInstance().getSelectedTab())) - updatePopup(); + }); } - public void updatePopup() { + DoorHanger getDoorHanger(int tabId, String value) { + for (DoorHanger dh : mDoorHangers) { + if (dh.getTabId() == tabId && dh.getValue().equals(value)) + return dh; + } + + // If there's no doorhanger for the given tabId and value, return null + return null; + } + + void removeDoorHanger(final DoorHanger doorHanger) { + mDoorHangers.remove(doorHanger); + + // Update the UI on the main thread + mActivity.runOnUiThread(new Runnable() { + public void run() { + mContent.removeView(doorHanger); + } + }); + } + + void removeTransientDoorHangers(int tabId) { + // Make a temporary set to avoid a ConcurrentModificationException + HashSet doorHangersToRemove = new HashSet(); + for (DoorHanger dh : mDoorHangers) { + // Only remove transient doorhangers for the given tab + if (dh.getTabId() == tabId && dh.shouldRemove()) + doorHangersToRemove.add(dh); + } + + for (DoorHanger dh : doorHangersToRemove) { + removeDoorHanger(dh); + } + } + + void updatePopup() { + // Bail if the selected tab is null, if there are no active doorhangers, + // or if we haven't inflated the layout yet (this can happen if updatePopup() + // is called before the runnable from addDoorHanger() runs). Tab tab = Tabs.getInstance().getSelectedTab(); - if (tab == null) { + if (tab == null || mDoorHangers.size() == 0 || !mInflated) { dismiss(); return; } - - Log.i(LOGTAG, "Showing all doorhangers for tab: " + tab.getId()); + + // Show doorhangers for the selected tab + int tabId = tab.getId(); + boolean shouldShowPopup = false; + for (DoorHanger dh : mDoorHangers) { + if (dh.getTabId() == tabId) { + dh.setVisibility(View.VISIBLE); + shouldShowPopup = true; + } else { + dh.setVisibility(View.GONE); + } + } - HashMap doorHangers = tab.getDoorHangers(); - // Hide the popup if there aren't any doorhangers to show - if (doorHangers == null || doorHangers.size() == 0) { + // Dismiss the popup if there are no doorhangers to show for this tab + if (!shouldShowPopup) { dismiss(); return; } - if (!mInflated) - init(); - - // Hide old doorhangers - for (int i = 0; i < mContent.getChildCount(); i++) { - DoorHanger dh = (DoorHanger) mContent.getChildAt(i); - dh.hide(); - } - - // Show the doorhangers for the tab - for (DoorHanger dh : doorHangers.values()) { - dh.show(); - } - - fixBackgroundForFirst(); if (isShowing()) { update(); @@ -189,9 +215,9 @@ public class DoorHangerPopup extends PopupWindow implements GeckoEventListener { } private void fixBackgroundForFirst() { - for (int i=0; i < mContent.getChildCount(); i++) { + for (int i = 0; i < mContent.getChildCount(); i++) { DoorHanger dh = (DoorHanger) mContent.getChildAt(i); - if (dh.isVisible()) { + if (dh.getVisibility() == View.VISIBLE) { dh.setBackgroundResource(R.drawable.doorhanger_bg); break; } diff --git a/mobile/android/base/GeckoApp.java b/mobile/android/base/GeckoApp.java index 2911608337ac..8570e61dfd97 100644 --- a/mobile/android/base/GeckoApp.java +++ b/mobile/android/base/GeckoApp.java @@ -678,7 +678,7 @@ abstract public class GeckoApp // Only remove doorhangers if the popup is hidden or if we're navigating to a new URL if (!mDoorHangerPopup.isShowing() || !uri.equals(tab.getURL())) - tab.removeTransientDoorHangers(); + mDoorHangerPopup.removeTransientDoorHangers(tabId); tab.updateURL(uri); tab.setDocumentURI(documentURI); diff --git a/mobile/android/base/Tab.java b/mobile/android/base/Tab.java index c388e03cf184..0080f7351462 100644 --- a/mobile/android/base/Tab.java +++ b/mobile/android/base/Tab.java @@ -49,7 +49,6 @@ public final class Tab { private boolean mExternal; private boolean mBookmark; private boolean mReadingListItem; - private HashMap mDoorHangers; private long mFaviconLoadId; private String mDocumentURI; private String mContentType; @@ -89,7 +88,6 @@ public final class Tab { mHistorySize = 0; mBookmark = false; mReadingListItem = false; - mDoorHangers = new HashMap(); mFaviconLoadId = 0; mDocumentURI = ""; mContentType = ""; @@ -106,7 +104,6 @@ public final class Tab { } public void onDestroy() { - mDoorHangers = new HashMap(); BrowserDB.unregisterContentObserver(mContentResolver, mContentObserver); } @@ -492,43 +489,6 @@ public final class Tab { return true; } - public void addDoorHanger(String value, DoorHanger dh) { - mDoorHangers.put(value, dh); - } - - public void removeDoorHanger(String value) { - mDoorHangers.remove(value); - } - - public void removeTransientDoorHangers() { - // Make a temporary set to avoid a ConcurrentModificationException - final HashSet valuesToRemove = new HashSet(); - - for (String value : mDoorHangers.keySet()) { - DoorHanger dh = mDoorHangers.get(value); - if (dh.shouldRemove()) - valuesToRemove.add(value); - } - - for (String value : valuesToRemove) { - mDoorHangers.remove(value); - } - } - - public DoorHanger getDoorHanger(String value) { - if (mDoorHangers == null) - return null; - - if (mDoorHangers.containsKey(value)) - return mDoorHangers.get(value); - - return null; - } - - public HashMap getDoorHangers() { - return mDoorHangers; - } - void handleSessionHistoryMessage(String event, JSONObject message) throws JSONException { if (event.equals("New")) { final String url = message.getString("url"); From aef69410ac0d30888c048af0f31f35f22db44370 Mon Sep 17 00:00:00 2001 From: Margaret Leibovic Date: Sat, 4 Aug 2012 16:34:47 -0700 Subject: [PATCH 03/22] Bug 732336 - (Part 3) Listen for tab events in doorhanger popup. r=wesj --- mobile/android/base/DoorHangerPopup.java | 33 +++++++++++++++++++++++- mobile/android/base/GeckoApp.java | 11 +------- 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/mobile/android/base/DoorHangerPopup.java b/mobile/android/base/DoorHangerPopup.java index 119304fc4f7f..662f82942364 100644 --- a/mobile/android/base/DoorHangerPopup.java +++ b/mobile/android/base/DoorHangerPopup.java @@ -23,7 +23,8 @@ import android.widget.RelativeLayout; import java.util.HashSet; -public class DoorHangerPopup extends PopupWindow implements GeckoEventListener { +public class DoorHangerPopup extends PopupWindow + implements GeckoEventListener, Tabs.OnTabsChangedListener { private static final String LOGTAG = "GeckoDoorHangerPopup"; private GeckoApp mActivity; @@ -49,11 +50,13 @@ public class DoorHangerPopup extends PopupWindow implements GeckoEventListener { GeckoAppShell.registerGeckoEventListener("Doorhanger:Add", this); GeckoAppShell.registerGeckoEventListener("Doorhanger:Remove", this); + Tabs.registerOnTabsChangedListener(this); } void destroy() { GeckoAppShell.unregisterGeckoEventListener("Doorhanger:Add", this); GeckoAppShell.unregisterGeckoEventListener("Doorhanger:Remove", this); + Tabs.unregisterOnTabsChangedListener(this); } public void handleMessage(String event, JSONObject geckoObject) { @@ -85,6 +88,34 @@ public class DoorHangerPopup extends PopupWindow implements GeckoEventListener { } } + public void onTabChanged(Tab tab, Tabs.TabEvents msg, Object data) { + switch(msg) { + case CLOSED: + // Remove any doorhangers for a tab when it's closed + for (DoorHanger dh : mDoorHangers) { + if (dh.getTabId() == tab.getId()) + removeDoorHanger(dh); + } + break; + + case LOCATION_CHANGE: + // Only remove doorhangers if the popup is hidden or if we're navigating to a new URL + if (!isShowing() || !data.equals(tab.getURL())) + removeTransientDoorHangers(tab.getId()); + + // Update the popup if the location change was on the current tab + if (Tabs.getInstance().isSelectedTab(tab)) + updatePopup(); + break; + + case SELECTED: + // Always update the popup when a new tab is selected. This will cover cases + // where a different tab was closed, since we always need to select a new tab. + updatePopup(); + break; + } + } + private void init() { setBackgroundDrawable(new BitmapDrawable()); setOutsideTouchable(true); diff --git a/mobile/android/base/GeckoApp.java b/mobile/android/base/GeckoApp.java index 8570e61dfd97..1ae7ef52e195 100644 --- a/mobile/android/base/GeckoApp.java +++ b/mobile/android/base/GeckoApp.java @@ -213,7 +213,6 @@ abstract public class GeckoApp break; // Fall through... case SELECTED: - updatePopups(tab); invalidateOptionsMenu(); break; } @@ -663,10 +662,6 @@ abstract public class GeckoApp public void hideFormAssistPopup() { if (mFormAssistPopup != null) mFormAssistPopup.hide(); - } - - void updatePopups(final Tab tab) { - mDoorHangerPopup.updatePopup(); } void handleLocationChange(final int tabId, final String uri, @@ -676,10 +671,6 @@ abstract public class GeckoApp if (tab == null) return; - // Only remove doorhangers if the popup is hidden or if we're navigating to a new URL - if (!mDoorHangerPopup.isShowing() || !uri.equals(tab.getURL())) - mDoorHangerPopup.removeTransientDoorHangers(tabId); - tab.updateURL(uri); tab.setDocumentURI(documentURI); @@ -701,7 +692,7 @@ abstract public class GeckoApp mMainHandler.post(new Runnable() { public void run() { - Tabs.getInstance().notifyListeners(tab, Tabs.TabEvents.LOCATION_CHANGE); + Tabs.getInstance().notifyListeners(tab, Tabs.TabEvents.LOCATION_CHANGE, uri); } }); } From db1f06ec89532357e34bc44a982aafe5d1a8def2 Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Sun, 5 Aug 2012 11:01:18 +1000 Subject: [PATCH 04/22] Bug 655877 - Part 17: Ensure non-SVG child elements of SVG text elements do not get frames. r=bz --- layout/base/nsCSSFrameConstructor.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/layout/base/nsCSSFrameConstructor.cpp b/layout/base/nsCSSFrameConstructor.cpp index fea72e703767..e82a6fe851b5 100644 --- a/layout/base/nsCSSFrameConstructor.cpp +++ b/layout/base/nsCSSFrameConstructor.cpp @@ -375,6 +375,17 @@ IsInlineFrame(const nsIFrame* aFrame) return aFrame->IsFrameOfType(nsIFrame::eLineParticipant); } +/** + * True if aFrame is an instance of an SVG frame class or is an inline/block + * frame being used for SVG text. + */ +static bool +IsFrameForSVG(const nsIFrame* aFrame) +{ + return aFrame->IsFrameOfType(nsIFrame::eSVG) || + aFrame->IsSVGText(); +} + /** * Returns true iff aFrame explicitly prevents its descendants from floating * (at least, down to the level of descendants which themselves are @@ -5233,7 +5244,7 @@ nsCSSFrameConstructor::AddFrameConstructionItemsInternal(nsFrameConstructorState // Don't create frames for non-SVG element children of SVG elements. if (aNameSpaceID != kNameSpaceID_SVG && aParentFrame && - aParentFrame->IsFrameOfType(nsIFrame::eSVG) && + IsFrameForSVG(aParentFrame) && !aParentFrame->IsFrameOfType(nsIFrame::eSVGForeignObject) ) { SetAsUndisplayedContent(aItems, element, styleContext, From ca73141a7dbc0c1244cab4172b5d5de71b8caf54 Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Sun, 5 Aug 2012 11:01:18 +1000 Subject: [PATCH 05/22] Bug 655877 - Part 18: Ensure even line-ending white space SVG text frames are created. r=bz --- layout/base/nsCSSFrameConstructor.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/layout/base/nsCSSFrameConstructor.cpp b/layout/base/nsCSSFrameConstructor.cpp index e82a6fe851b5..750373a8c0a6 100644 --- a/layout/base/nsCSSFrameConstructor.cpp +++ b/layout/base/nsCSSFrameConstructor.cpp @@ -5509,12 +5509,14 @@ nsCSSFrameConstructor::ConstructFramesFromItem(nsFrameConstructorState& aState, // We don't do it for content that may have XBL anonymous siblings, // because they make it difficult to correctly create the frame // due to dynamic changes. - // We don't do it for text that's not a line participant (i.e. SVG text). + // We don't do it for SVG text, since we might need to position and + // measure the white space glyphs due to x/y/dx/dy attributes. if (AtLineBoundary(aIter) && !styleContext->GetStyleText()->NewlineIsSignificant() && aIter.List()->ParentHasNoXBLChildren() && !(aState.mAdditionalStateBits & NS_FRAME_GENERATED_CONTENT) && (item.mFCData->mBits & FCDATA_IS_LINE_PARTICIPANT) && + !(item.mFCData->mBits & FCDATA_IS_SVG_TEXT) && item.IsWhitespace(aState)) return NS_OK; From 7404892c563eb2eeca2c06a5d40921f69d28c017 Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Sun, 5 Aug 2012 11:01:19 +1000 Subject: [PATCH 06/22] Bug 655877 - Part 20: Make nsTextFrame QueryFrame-able. r=roc --- layout/generic/nsTextFrame.h | 10 ++++++++-- layout/generic/nsTextFrameThebes.cpp | 4 ++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/layout/generic/nsTextFrame.h b/layout/generic/nsTextFrame.h index d667a53ad5ee..3a352909e649 100644 --- a/layout/generic/nsTextFrame.h +++ b/layout/generic/nsTextFrame.h @@ -23,18 +23,24 @@ class PropertyProvider; #define TEXT_HAS_FONT_INFLATION NS_FRAME_STATE_BIT(61) -class nsTextFrame : public nsFrame { +typedef nsFrame nsTextFrameBase; + +class nsTextFrame : public nsTextFrameBase { public: + NS_DECL_QUERYFRAME_TARGET(nsTextFrame) NS_DECL_FRAMEARENA_HELPERS friend class nsContinuingTextFrame; nsTextFrame(nsStyleContext* aContext) - : nsFrame(aContext) + : nsTextFrameBase(aContext) { NS_ASSERTION(mContentOffset == 0, "Bogus content offset"); } + // nsQueryFrame + NS_DECL_QUERYFRAME + // nsIFrame NS_IMETHOD BuildDisplayList(nsDisplayListBuilder* aBuilder, const nsRect& aDirtyRect, diff --git a/layout/generic/nsTextFrameThebes.cpp b/layout/generic/nsTextFrameThebes.cpp index 311348221743..da9831d5c639 100644 --- a/layout/generic/nsTextFrameThebes.cpp +++ b/layout/generic/nsTextFrameThebes.cpp @@ -2404,6 +2404,10 @@ BuildTextRunsScanner::AssignTextRun(gfxTextRun* aTextRun, float aInflation) } } +NS_QUERYFRAME_HEAD(nsTextFrame) + NS_QUERYFRAME_ENTRY(nsTextFrame) +NS_QUERYFRAME_TAIL_INHERITING(nsTextFrameBase) + gfxSkipCharsIterator nsTextFrame::EnsureTextRun(TextRunType aWhichTextRun, gfxContext* aReferenceContext, From 209745005deab80cc70dd985bc5bf4b2fad408b3 Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Sun, 5 Aug 2012 11:01:19 +1000 Subject: [PATCH 07/22] Bug 655877 - Part 21: Avoid assertions when nsStyleContext::GetVisitedDependentColor is called for an SVG paint property. r=dbaron --- layout/style/nsStyleContext.cpp | 45 ++++++++++++++++++++++++++------- 1 file changed, 36 insertions(+), 9 deletions(-) diff --git a/layout/style/nsStyleContext.cpp b/layout/style/nsStyleContext.cpp index afeee9ef4fa7..0256499ab391 100644 --- a/layout/style/nsStyleContext.cpp +++ b/layout/style/nsStyleContext.cpp @@ -18,11 +18,14 @@ #include "nsStyleContext.h" #include "prlog.h" #include "nsStyleAnimation.h" +#include "mozilla/Util.h" #ifdef DEBUG // #define NOISY_DEBUG #endif +using namespace mozilla; + //---------------------------------------------------------------------- @@ -699,19 +702,38 @@ NS_NewStyleContext(nsStyleContext* aParentContext, return context; } -static nscolor ExtractColor(nsCSSProperty aProperty, - nsStyleContext *aStyleContext) +static inline void +ExtractAnimationValue(nsCSSProperty aProperty, + nsStyleContext* aStyleContext, + nsStyleAnimation::Value& aResult) { - nsStyleAnimation::Value val; -#ifdef DEBUG - bool success = -#endif - nsStyleAnimation::ExtractComputedValue(aProperty, aStyleContext, val); + DebugOnly success = + nsStyleAnimation::ExtractComputedValue(aProperty, aStyleContext, aResult); NS_ABORT_IF_FALSE(success, "aProperty must be extractable by nsStyleAnimation"); +} + +static nscolor +ExtractColor(nsCSSProperty aProperty, + nsStyleContext *aStyleContext) +{ + nsStyleAnimation::Value val; + ExtractAnimationValue(aProperty, aStyleContext, val); return val.GetColorValue(); } +static nscolor +ExtractColorLenient(nsCSSProperty aProperty, + nsStyleContext *aStyleContext) +{ + nsStyleAnimation::Value val; + ExtractAnimationValue(aProperty, aStyleContext, val); + if (val.GetUnit() == nsStyleAnimation::eUnit_Color) { + return val.GetColorValue(); + } + return NS_RGBA(0, 0, 0, 0); +} + struct ColorIndexSet { PRUint8 colorIndex, alphaIndex; }; @@ -734,15 +756,20 @@ nsStyleContext::GetVisitedDependentColor(nsCSSProperty aProperty) aProperty == eCSSProperty_stroke, "we need to add to nsStyleContext::CalcStyleDifference"); + bool isPaintProperty = aProperty == eCSSProperty_fill || + aProperty == eCSSProperty_stroke; + nscolor colors[2]; - colors[0] = ExtractColor(aProperty, this); + colors[0] = isPaintProperty ? ExtractColorLenient(aProperty, this) + : ExtractColor(aProperty, this); nsStyleContext *visitedStyle = this->GetStyleIfVisited(); if (!visitedStyle) { return colors[0]; } - colors[1] = ExtractColor(aProperty, visitedStyle); + colors[1] = isPaintProperty ? ExtractColorLenient(aProperty, visitedStyle) + : ExtractColor(aProperty, visitedStyle); return nsStyleContext::CombineVisitedColors(colors, this->RelevantLinkVisited()); From 31d50d770d3d9af0db5cfa534717a8b035c6c693 Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Sun, 5 Aug 2012 11:01:19 +1000 Subject: [PATCH 08/22] Bug 655877 - Part 29: Don't underline links within SVG text by default. r=dbaron --- layout/base/nsPresShell.cpp | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/layout/base/nsPresShell.cpp b/layout/base/nsPresShell.cpp index 6a9380b8a099..2cd590c29182 100644 --- a/layout/base/nsPresShell.cpp +++ b/layout/base/nsPresShell.cpp @@ -1195,6 +1195,13 @@ PresShell::CreatePreferenceStyleSheet() mPrefStyleSheet->SetURIs(uri, uri, uri); mPrefStyleSheet->SetComplete(); PRUint32 index; + rv = + mPrefStyleSheet->InsertRuleInternal(NS_LITERAL_STRING("@namespace svg url(http://www.w3.org/2000/svg);"), + 0, &index); + if (NS_FAILED(rv)) { + mPrefStyleSheet = nullptr; + return rv; + } rv = mPrefStyleSheet->InsertRuleInternal(NS_LITERAL_STRING("@namespace url(http://www.w3.org/1999/xhtml);"), 0, &index); @@ -1207,10 +1214,10 @@ PresShell::CreatePreferenceStyleSheet() return NS_OK; } -// XXX We want these after the @namespace rule. Does order matter +// XXX We want these after the @namespace rules. Does order matter // for these rules, or can we call StyleRule::StyleRuleCount() // and just "append"? -static PRUint32 sInsertPrefSheetRulesAt = 1; +static PRUint32 sInsertPrefSheetRulesAt = 2; nsresult PresShell::SetPrefNoScriptRule() @@ -1345,7 +1352,7 @@ nsresult PresShell::SetPrefLinkRules(void) // NOTE: these must go in the agent stylesheet or they cannot be // overridden by authors rv = mPrefStyleSheet-> - InsertRuleInternal(NS_LITERAL_STRING("*|*:-moz-any-link{text-decoration:underline}"), + InsertRuleInternal(NS_LITERAL_STRING("*|*:-moz-any-link:not(svg|a){text-decoration:underline}"), sInsertPrefSheetRulesAt, &index); } else { rv = mPrefStyleSheet-> From f54d116fea11770cf674b90cb59173b1ca9c25e8 Mon Sep 17 00:00:00 2001 From: Justin Lebar Date: Sun, 5 Aug 2012 01:09:39 -0400 Subject: [PATCH 09/22] Bug 768832 - Set OOM adjust for background windows. r=bz,cjones --- b2g/app/b2g.js | 12 + dom/ipc/Makefile.in | 2 + dom/ipc/ProcessPriorityManager.cpp | 336 +++++++++++++++++++++++ dom/ipc/ProcessPriorityManager.h | 33 +++ hal/Hal.cpp | 16 ++ hal/Hal.h | 9 + hal/HalTypes.h | 21 +- hal/Makefile.in | 1 + hal/fallback/FallbackProcessPriority.cpp | 19 ++ hal/gonk/GonkHal.cpp | 51 +++- hal/sandbox/PHal.ipdl | 6 +- hal/sandbox/SandboxHal.cpp | 15 + layout/build/nsLayoutStatics.cpp | 4 + 13 files changed, 517 insertions(+), 8 deletions(-) create mode 100644 dom/ipc/ProcessPriorityManager.cpp create mode 100644 dom/ipc/ProcessPriorityManager.h create mode 100644 hal/fallback/FallbackProcessPriority.cpp diff --git a/b2g/app/b2g.js b/b2g/app/b2g.js index e91de9312fba..7e33bcaa1cf7 100644 --- a/b2g/app/b2g.js +++ b/b2g/app/b2g.js @@ -483,3 +483,15 @@ pref("javascript.options.mem.gc_incremental_slice_ms", 30); // Show/Hide scrollbars when active/inactive pref("ui.showHideScrollbars", 1); + +// Enable the ProcessPriorityManager, and give processes with no visible +// documents a 1s grace period before they're eligible to be marked as +// background. +pref("dom.ipc.processPriorityManager.enabled", true); +pref("dom.ipc.processPriorityManager.gracePeriodMS", 1000); +pref("hal.processPriorityManager.gonk.masterOomAdjust", 0); +pref("hal.processPriorityManager.gonk.foregroundOomAdjust", 1); +pref("hal.processPriorityManager.gonk.backgroundOomAdjust", 2); +pref("hal.processPriorityManager.gonk.masterNice", -1); +pref("hal.processPriorityManager.gonk.foregroundNice", 0); +pref("hal.processPriorityManager.gonk.backgroundNice", 10); diff --git a/dom/ipc/Makefile.in b/dom/ipc/Makefile.in index a07bd9ad6ef1..d1a96fe9e730 100644 --- a/dom/ipc/Makefile.in +++ b/dom/ipc/Makefile.in @@ -40,6 +40,7 @@ EXPORTS_mozilla/dom = \ EXPORTS_mozilla/dom/ipc = \ Blob.h \ + ProcessPriorityManager.h \ nsIRemoteBlob.h \ $(NULL) @@ -50,6 +51,7 @@ CPPSRCS = \ ContentChild.cpp \ CrashReporterParent.cpp \ CrashReporterChild.cpp \ + ProcessPriorityManager.cpp \ StructuredCloneUtils.cpp \ TabParent.cpp \ TabChild.cpp \ diff --git a/dom/ipc/ProcessPriorityManager.cpp b/dom/ipc/ProcessPriorityManager.cpp new file mode 100644 index 000000000000..9be87ebd149e --- /dev/null +++ b/dom/ipc/ProcessPriorityManager.cpp @@ -0,0 +1,336 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim: set sw=2 ts=8 et ft=cpp : */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include "mozilla/dom/ipc/ProcessPriorityManager.h" +#include "mozilla/Hal.h" +#include "mozilla/Preferences.h" +#include "mozilla/Services.h" +#include "mozilla/HalTypes.h" +#include "mozilla/TimeStamp.h" +#include "prlog.h" +#include "nsWeakPtr.h" +#include "nsXULAppAPI.h" +#include "nsIInterfaceRequestorUtils.h" +#include "nsITimer.h" +#include "nsIObserver.h" +#include "nsIObserverService.h" +#include "nsIDocument.h" +#include "nsIDOMEventListener.h" +#include "nsIDOMWindow.h" +#include "nsIDOMEvent.h" +#include "nsIDOMEventTarget.h" +#include "nsIDOMDocument.h" +#include "nsPIDOMWindow.h" + +#ifdef XP_WIN +#include +#define getpid _getpid +#else +#include +#endif + +using namespace mozilla::hal; + +namespace mozilla { +namespace dom { +namespace ipc { + +namespace { +static bool sInitialized = false; + +// Some header defines a LOG macro, but we don't want it here. +#ifdef LOG +#undef LOG +#endif + +// Enable logging by setting +// +// NSPR_LOG_MODULES=ProcessPriorityManager:5 +// +// in your environment. + +#ifdef PR_LOGGING +static PRLogModuleInfo* logModule = PR_NewLogModule("ProcessPriorityManager"); +#define LOG(fmt, ...) \ + PR_LOG(logModule, PR_LOG_DEBUG, \ + ("[%d] ProcessPriorityManager - " fmt, getpid(), ##__VA_ARGS__)) +#else +#define LOG(fmt, ...) +#endif + +/** + * This class listens to window creation and visibilitychange events and + * informs the hal back-end when this process transitions between having no + * visible top-level windows, and when it has at least one visible top-level + * window. + * + * + * An important heuristic here is that we don't mark a process as background + * until it's had no visible top-level windows for some amount of time. + * + * We do this because the notion of visibility is tied to inner windows + * (actually, to documents). When we navigate a page with outer window W, we + * first destroy W's inner window and document, then insert a new inner window + * and document into W. If not for our grace period, this transition could + * cause us to inform hal that this process quickly transitioned from + * foreground to background to foreground again. + * + */ +class ProcessPriorityManager MOZ_FINAL + : public nsIObserver + , public nsIDOMEventListener +{ +public: + ProcessPriorityManager(); + void Init(); + + NS_DECL_ISUPPORTS + NS_DECL_NSIOBSERVER + NS_DECL_NSIDOMEVENTLISTENER + +private: + void SetPriority(ProcessPriority aPriority); + void OnContentDocumentGlobalCreated(nsISupports* aOuterWindow); + void OnInnerWindowDestroyed(); + void OnGracePeriodTimerFired(); + void RecomputeNumVisibleWindows(); + + // mProcessPriority tracks the priority we've given this process in hal, + // except that, when the grace period timer is active, + // mProcessPriority == BACKGROUND even though hal still thinks we're a + // foreground process. + ProcessPriority mProcessPriority; + + nsTArray mWindows; + nsCOMPtr mGracePeriodTimer; + TimeStamp mStartupTime; +}; + +NS_IMPL_ISUPPORTS2(ProcessPriorityManager, nsIObserver, nsIDOMEventListener); + +ProcessPriorityManager::ProcessPriorityManager() + : mProcessPriority(PROCESS_PRIORITY_FOREGROUND) + , mStartupTime(TimeStamp::Now()) +{ +} + +void +ProcessPriorityManager::Init() +{ + LOG("Starting up."); + + // We can't do this in the constructor because we need to hold a strong ref + // to |this| before calling these methods. + nsCOMPtr os = services::GetObserverService(); + os->AddObserver(this, "content-document-global-created", /* ownsWeak = */ false); + os->AddObserver(this, "inner-window-destroyed", /* ownsWeak = */ false); + + SetPriority(PROCESS_PRIORITY_FOREGROUND); +} + +NS_IMETHODIMP +ProcessPriorityManager::Observe( + nsISupports* aSubject, + const char* aTopic, + const PRUnichar* aData) +{ + if (!strcmp(aTopic, "content-document-global-created")) { + OnContentDocumentGlobalCreated(aSubject); + } else if (!strcmp(aTopic, "inner-window-destroyed")) { + OnInnerWindowDestroyed(); + } else if (!strcmp(aTopic, "timer-callback")) { + OnGracePeriodTimerFired(); + } else { + MOZ_ASSERT(false); + } + return NS_OK; +} + +NS_IMETHODIMP +ProcessPriorityManager::HandleEvent( + nsIDOMEvent* aEvent) +{ + LOG("Got visibilitychange."); + RecomputeNumVisibleWindows(); + return NS_OK; +} + +void +ProcessPriorityManager::OnContentDocumentGlobalCreated( + nsISupports* aOuterWindow) +{ + // Get the inner window (the topic of content-document-global-created is + // the /outer/ window!). + nsCOMPtr outerWindow = do_QueryInterface(aOuterWindow); + NS_ENSURE_TRUE(outerWindow, ); + nsCOMPtr innerWindow = outerWindow->GetCurrentInnerWindow(); + NS_ENSURE_TRUE(innerWindow, ); + + // We're only interested in top-level windows. + nsCOMPtr parentOuterWindow; + innerWindow->GetScriptableParent(getter_AddRefs(parentOuterWindow)); + NS_ENSURE_TRUE(parentOuterWindow, ); + if (parentOuterWindow != outerWindow) { + return; + } + + nsCOMPtr target = do_QueryInterface(innerWindow); + NS_ENSURE_TRUE(target, ); + + nsWeakPtr weakWin = do_GetWeakReference(innerWindow); + NS_ENSURE_TRUE(weakWin, ); + + if (mWindows.Contains(weakWin)) { + return; + } + + target->AddSystemEventListener(NS_LITERAL_STRING("mozvisibilitychange"), + this, + /* useCapture = */ false, + /* wantsUntrusted = */ false); + + mWindows.AppendElement(weakWin); + RecomputeNumVisibleWindows(); +} + +void +ProcessPriorityManager::OnInnerWindowDestroyed() +{ + RecomputeNumVisibleWindows(); +} + +void +ProcessPriorityManager::RecomputeNumVisibleWindows() +{ + // We could try to be clever and count the number of visible windows, instead + // of iterating over mWindows every time one window's visibility state changes. + // But experience suggests that iterating over the windows is prone to fewer + // errors (and one mistake doesn't mess you up for the entire session). + // Moreover, mWindows should be a very short list, since it contains only + // top-level content windows. + + bool allHidden = true; + for (PRUint32 i = 0; i < mWindows.Length(); i++) { + nsCOMPtr window = do_QueryReferent(mWindows[i]); + if (!window) { + mWindows.RemoveElementAt(i); + i--; + continue; + } + + nsCOMPtr doc; + window->GetDocument(getter_AddRefs(doc)); + if (!doc) { + continue; + } + + bool hidden = false; + doc->GetMozHidden(&hidden); +#ifdef DEBUG + nsAutoString spec; + doc->GetDocumentURI(spec); + LOG("Document at %s has visibility %d.", NS_ConvertUTF16toUTF8(spec).get(), !hidden); +#endif + + allHidden = allHidden && hidden; + + // We could break out early from this loop if + // !hidden && mProcessPriority == BACKGROUND, + // but then we might not clean up all the weak refs. + } + + SetPriority(allHidden ? + PROCESS_PRIORITY_BACKGROUND : + PROCESS_PRIORITY_FOREGROUND); +} + +void +ProcessPriorityManager::SetPriority(ProcessPriority aPriority) +{ + if (aPriority == mProcessPriority) { + return; + } + + if (aPriority == PROCESS_PRIORITY_BACKGROUND) { + // If this is a foreground --> background transition, give ourselves a + // grace period before informing hal. + PRUint32 gracePeriodMS = Preferences::GetUint("dom.ipc.processPriorityManager.gracePeriodMS", 1000); + if (mGracePeriodTimer) { + LOG("Grace period timer already active."); + return; + } + + LOG("Initializing grace period timer."); + mProcessPriority = aPriority; + mGracePeriodTimer = do_CreateInstance("@mozilla.org/timer;1"); + mGracePeriodTimer->Init(this, gracePeriodMS, nsITimer::TYPE_ONE_SHOT); + + } else if (aPriority == PROCESS_PRIORITY_FOREGROUND) { + // If this is a background --> foreground transition, do it immediately, and + // cancel the outstanding grace period timer, if there is one. + if (mGracePeriodTimer) { + mGracePeriodTimer->Cancel(); + mGracePeriodTimer = nullptr; + } + + LOG("Setting priority to %d.", aPriority); + mProcessPriority = aPriority; + hal::SetProcessPriority(getpid(), aPriority); + + } else { + MOZ_ASSERT(false); + } +} + +void +ProcessPriorityManager::OnGracePeriodTimerFired() +{ + LOG("Grace period timer fired; setting priority to %d.", + PROCESS_PRIORITY_BACKGROUND); + + // mProcessPriority should already be BACKGROUND: We set it in + // SetPriority(BACKGROUND), and we canceled this timer if there was an + // intervening SetPriority(FOREGROUND) call. + MOZ_ASSERT(mProcessPriority == PROCESS_PRIORITY_BACKGROUND); + + mGracePeriodTimer = nullptr; + hal::SetProcessPriority(getpid(), PROCESS_PRIORITY_BACKGROUND); +} + +} // anonymous namespace + +void +InitProcessPriorityManager() +{ + if (sInitialized) { + return; + } + + // If IPC tabs aren't enabled at startup, don't bother with any of this. + if (!Preferences::GetBool("dom.ipc.processPriorityManager.enabled") || + Preferences::GetBool("dom.ipc.tabs.disabled")) { + return; + } + + sInitialized = true; + + // If we're the master process, mark ourselves as such and don't create a + // ProcessPriorityManager (we never want to mark the master process as + // backgrounded). + if (XRE_GetProcessType() == GeckoProcessType_Default) { + LOG("This is the master process."); + hal::SetProcessPriority(getpid(), PROCESS_PRIORITY_MASTER); + return; + } + + // This object is held alive by the observer service. + nsRefPtr mgr = new ProcessPriorityManager(); + mgr->Init(); +} + +} // namespace ipc +} // namespace dom +} // namespace mozilla diff --git a/dom/ipc/ProcessPriorityManager.h b/dom/ipc/ProcessPriorityManager.h new file mode 100644 index 000000000000..c320611bbbff --- /dev/null +++ b/dom/ipc/ProcessPriorityManager.h @@ -0,0 +1,33 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim: set sw=2 ts=8 et ft=cpp : */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#ifndef mozilla_ProcessPriorityManager_h_ +#define mozilla_ProcessPriorityManager_h_ + +namespace mozilla { +namespace dom { +namespace ipc { + +/** + * Initialize the ProcessPriorityManager. + * + * The ProcessPriorityManager informs the hal back-end whether this is the root + * Gecko process, and, if we're not the root, informs hal when this process + * transitions between having no visible top-level windows, and having at least + * one visible top-level window. + * + * Hal may adjust this process's operating system priority (e.g. niceness, on + * *nix) according to these notificaitons. + * + * This function call does nothing if the pref for OOP tabs is not set. + */ +void InitProcessPriorityManager(); + +} // namespace ipc +} // namespace dom +} // namespace mozilla + +#endif diff --git a/hal/Hal.cpp b/hal/Hal.cpp index 468536d671d0..a5b6d732b142 100644 --- a/hal/Hal.cpp +++ b/hal/Hal.cpp @@ -22,6 +22,11 @@ #include "WindowIdentifier.h" #include "mozilla/dom/ScreenOrientation.h" +#ifdef XP_WIN +#include +#define getpid _getpid +#endif + using namespace mozilla::services; #define PROXY_IF_SANDBOXED(_call) \ @@ -728,5 +733,16 @@ SetAlarm(PRInt32 aSeconds, PRInt32 aNanoseconds) RETURN_PROXY_IF_SANDBOXED(SetAlarm(aSeconds, aNanoseconds)); } +void +SetProcessPriority(int aPid, ProcessPriority aPriority) +{ + if (InSandbox()) { + hal_sandbox::SetProcessPriority(aPid, aPriority); + } + else { + hal_impl::SetProcessPriority(aPid, aPriority); + } +} + } // namespace hal } // namespace mozilla diff --git a/hal/Hal.h b/hal/Hal.h index a2d84c9990c3..84d96241a456 100644 --- a/hal/Hal.h +++ b/hal/Hal.h @@ -402,6 +402,15 @@ void NotifyAlarmFired(); */ bool SetAlarm(PRInt32 aSeconds, PRInt32 aNanoseconds); +/** + * Set the priority of the given process. + * + * Exactly what this does will vary between platforms. On *nix we might give + * background processes higher nice values. On other platforms, we might + * ignore this call entirely. + */ +void SetProcessPriority(int aPid, hal::ProcessPriority aPriority); + } // namespace MOZ_HAL_NAMESPACE } // namespace mozilla diff --git a/hal/HalTypes.h b/hal/HalTypes.h index 8687d3710c0c..1833297655a3 100644 --- a/hal/HalTypes.h +++ b/hal/HalTypes.h @@ -54,11 +54,13 @@ enum SwitchState { }; typedef Observer SwitchObserver; -} // namespace hal -} // namespace mozilla -namespace mozilla { -namespace hal { +enum ProcessPriority { + PROCESS_PRIORITY_BACKGROUND, + PROCESS_PRIORITY_FOREGROUND, + PROCESS_PRIORITY_MASTER, + NUM_PROCESS_PRIORITY +}; /** * Used by ModifyWakeLock @@ -69,8 +71,8 @@ enum WakeLockControl { WAKE_LOCK_ADD_ONE = 1, }; -} -} +} // namespace hal +} // namespace mozilla namespace IPC { @@ -134,6 +136,13 @@ struct ParamTraits: mozilla::hal::NUM_SWITCH_DEVICE> { }; +template <> +struct ParamTraits: + public EnumSerializer { +}; + } // namespace IPC diff --git a/hal/Makefile.in b/hal/Makefile.in index 10b09033d98b..5c37019aea90 100644 --- a/hal/Makefile.in +++ b/hal/Makefile.in @@ -124,6 +124,7 @@ CPPSRCS += \ FallbackWakeLocks.cpp \ FallbackSwitch.cpp \ FallbackScreenPower.cpp \ + FallbackProcessPriority.cpp \ $(NULL) endif #} diff --git a/hal/fallback/FallbackProcessPriority.cpp b/hal/fallback/FallbackProcessPriority.cpp new file mode 100644 index 000000000000..f7a528592b61 --- /dev/null +++ b/hal/fallback/FallbackProcessPriority.cpp @@ -0,0 +1,19 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include "Hal.h" + +using namespace mozilla::hal; + +namespace mozilla { +namespace hal_impl { + +void +SetProcessPriority(int aPid, ProcessPriority aPriority) +{ + HAL_LOG(("FallbackProcessPriority - SetProcessPriority(%d, %d)\n", aPid, aPriority)); +} + +} // hal_impl +} // namespace mozilla diff --git a/hal/gonk/GonkHal.cpp b/hal/gonk/GonkHal.cpp index 50aba0fa531d..9ac8fc97bf4d 100644 --- a/hal/gonk/GonkHal.cpp +++ b/hal/gonk/GonkHal.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include "android/log.h" @@ -39,7 +40,9 @@ #include "mozilla/FileUtils.h" #include "mozilla/Monitor.h" #include "mozilla/Services.h" +#include "mozilla/Preferences.h" #include "nsAlgorithm.h" +#include "nsPrintfCString.h" #include "nsIObserver.h" #include "nsIObserverService.h" #include "nsIRunnable.h" @@ -56,7 +59,8 @@ #define NsecPerSec 1000000000 -using mozilla::hal::WindowIdentifier; +using namespace mozilla; +using namespace mozilla::hal; namespace mozilla { namespace hal_impl { @@ -800,5 +804,50 @@ SetAlarm(PRInt32 aSeconds, PRInt32 aNanoseconds) return true; } +void +SetProcessPriority(int aPid, ProcessPriority aPriority) +{ + HAL_LOG(("SetProcessPriority(pid=%d, priority=%d)", aPid, aPriority)); + + const char* priorityStr = NULL; + switch (aPriority) { + case PROCESS_PRIORITY_BACKGROUND: + priorityStr = "background"; + break; + case PROCESS_PRIORITY_FOREGROUND: + priorityStr = "foreground"; + break; + case PROCESS_PRIORITY_MASTER: + priorityStr = "master"; + break; + default: + MOZ_NOT_REACHED(); + } + + // Notice that you can disable oom_adj and renice by deleting the prefs + // hal.processPriorityManager{foreground,background,master}{OomAdjust,Nice}. + + PRInt32 oomAdj = 0; + nsresult rv = Preferences::GetInt(nsPrintfCString( + "hal.processPriorityManager.gonk.%sOomAdjust", priorityStr).get(), &oomAdj); + if (NS_SUCCEEDED(rv)) { + HAL_LOG(("Setting oom_adj for pid %d to %d", aPid, oomAdj)); + WriteToFile(nsPrintfCString("/proc/%d/oom_adj", aPid).get(), + nsPrintfCString("%d", oomAdj).get()); + } + + PRInt32 nice = 0; + rv = Preferences::GetInt(nsPrintfCString( + "hal.processPriorityManager.gonk.%sNice", priorityStr).get(), &nice); + if (NS_SUCCEEDED(rv)) { + HAL_LOG(("Setting nice for pid %d to %d", aPid, nice)); + + int success = setpriority(PRIO_PROCESS, aPid, nice); + if (success != 0) { + HAL_LOG(("Failed to set nice for pid %d to %d", aPid, nice)); + } + } +} + } // hal_impl } // mozilla diff --git a/hal/sandbox/PHal.ipdl b/hal/sandbox/PHal.ipdl index 79b4621f0ea4..0e35bf2efaad 100644 --- a/hal/sandbox/PHal.ipdl +++ b/hal/sandbox/PHal.ipdl @@ -21,6 +21,7 @@ using mozilla::hal::SensorAccuracyType; using mozilla::hal::WakeLockControl; using mozilla::hal::SwitchState; using mozilla::hal::SwitchDevice; +using mozilla::hal::ProcessPriority; using nsIntRect; using PRTime; @@ -71,7 +72,8 @@ struct ScreenConfiguration { uint32_t colorDepth; uint32_t pixelDepth; }; -} + +} // namespace hal namespace hal_sandbox { @@ -138,6 +140,8 @@ parent: sync GetCurrentSwitchState(SwitchDevice aDevice) returns (SwitchState aState); + SetProcessPriority(int aPid, ProcessPriority aPriority); + child: NotifySensorChange(SensorData aSensorData); diff --git a/hal/sandbox/SandboxHal.cpp b/hal/sandbox/SandboxHal.cpp index 756bce7b0597..ed6bdfb63955 100644 --- a/hal/sandbox/SandboxHal.cpp +++ b/hal/sandbox/SandboxHal.cpp @@ -280,6 +280,12 @@ SetAlarm(PRInt32 aSeconds, PRInt32 aNanoseconds) return false; } +void +SetProcessPriority(int aPid, ProcessPriority aPriority) +{ + Hal()->SendSetProcessPriority(aPid, aPriority); +} + class HalParent : public PHalParent , public BatteryObserver , public NetworkObserver @@ -568,6 +574,15 @@ public: *aState = hal::GetCurrentSwitchState(aDevice); return true; } + + virtual bool + RecvSetProcessPriority(const int& aPid, const ProcessPriority& aPriority) + { + // TODO As a security check, we should ensure that aPid is either the pid + // of our child, or the pid of one of the child's children. + hal::SetProcessPriority(aPid, aPriority); + return true; + } }; class HalChild : public PHalChild { diff --git a/layout/build/nsLayoutStatics.cpp b/layout/build/nsLayoutStatics.cpp index f935f4fae9f4..767ec622b9f0 100644 --- a/layout/build/nsLayoutStatics.cpp +++ b/layout/build/nsLayoutStatics.cpp @@ -94,11 +94,13 @@ #include "nsHyphenationManager.h" #include "nsEditorSpellCheck.h" #include "nsWindowMemoryReporter.h" +#include "mozilla/dom/ipc/ProcessPriorityManager.h" extern void NS_ShutdownChainItemPool(); using namespace mozilla; using namespace mozilla::dom; +using namespace mozilla::dom::ipc; nsrefcnt nsLayoutStatics::sLayoutStaticRefcnt = 0; @@ -243,6 +245,8 @@ nsLayoutStatics::Initialize() nsSVGUtils::Init(); + InitProcessPriorityManager(); + return NS_OK; } From cab8001bc3add17de5191f5a547d0a08b4d6a1a9 Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Sun, 5 Aug 2012 00:26:38 +1200 Subject: [PATCH 10/22] Bug 772679. RestrictToLayerPixels needs to accurately convert between appunits scroll offsets and ThebesLayer pixel coordinates. r=tnikkel Change GetThebesLayerResolutionForFrame to GetThebesLayerScaleForFrame, which just returns a scale. Ensure that the scale is as accurate as possible even if dedicated layers for scrolled content (or any layers at all) have not been created yet, by taking into account transforms that have not yet generated layers. This makes the decisions made by nsGfxScrollFrameInner::ScrollToImpl independent of whether there is currently an active layer for the scrolled content (or much more nearly so). In nsGfxScrollFrameInner::ScrollToImpl, do not use the current internal fractional offset of the ThebesLayer, which is in a mostly unrelated coordinate space to our scroll positions. Instead, just try to make sure that the previous and next scroll position differ by a whole number of layer pixels. --- layout/base/FrameLayerBuilder.cpp | 72 +++++++++++---------- layout/base/FrameLayerBuilder.h | 14 ++--- layout/base/nsIPresShell.h | 1 + layout/generic/nsGfxScrollFrame.cpp | 97 +++++++++++++++-------------- 4 files changed, 97 insertions(+), 87 deletions(-) diff --git a/layout/base/FrameLayerBuilder.cpp b/layout/base/FrameLayerBuilder.cpp index a2c91740ab2f..e54c2ed8f355 100644 --- a/layout/base/FrameLayerBuilder.cpp +++ b/layout/base/FrameLayerBuilder.cpp @@ -2505,44 +2505,50 @@ FrameLayerBuilder::GetDedicatedLayer(nsIFrame* aFrame, PRUint32 aDisplayItemKey) return nullptr; } -bool -FrameLayerBuilder::GetThebesLayerResolutionForFrame(nsIFrame* aFrame, - double* aXres, double* aYres, - gfxPoint* aPoint) +static gfxSize +PredictScaleForContent(nsIFrame* aFrame, nsIFrame* aAncestorWithScale, + const gfxSize& aScale) { - nsTArray *array = GetDisplayItemDataArrayForFrame(aFrame); - if (array) { - for (PRUint32 i = 0; i < array->Length(); ++i) { - Layer* layer = array->ElementAt(i).mLayer; - if (layer->HasUserData(&gThebesDisplayItemLayerUserData)) { - ThebesDisplayItemLayerUserData* data = - static_cast - (layer->GetUserData(&gThebesDisplayItemLayerUserData)); - *aXres = data->mXScale; - *aYres = data->mYScale; - *aPoint = data->mActiveScrolledRootPosition; - return true; + gfx3DMatrix transform = + gfx3DMatrix::ScalingMatrix(aScale.width, aScale.height, 1.0); + // aTransform is applied first, then the scale is applied to the result + transform = nsLayoutUtils::GetTransformToAncestor(aFrame, aAncestorWithScale)*transform; + gfxMatrix transform2d; + if (transform.CanDraw2D(&transform2d)) { + return transform2d.ScaleFactors(true); + } + return gfxSize(1.0, 1.0); +} + +gfxSize +FrameLayerBuilder::GetThebesLayerScaleForFrame(nsIFrame* aFrame) +{ + nsIFrame* last; + for (nsIFrame* f = aFrame; f; f = nsLayoutUtils::GetCrossDocParentFrame(f)) { + last = f; + if (f->GetStateBits() & NS_FRAME_HAS_CONTAINER_LAYER) { + nsTArray* array = GetDisplayItemDataArrayForFrame(f); + NS_ASSERTION(array, "Must have display item data for container"); + for (PRUint32 i = 0; i < array->Length(); ++i) { + Layer* layer = array->ElementAt(i).mLayer; + ContainerLayer* container = layer->AsContainerLayer(); + if (!container) { + continue; + } + for (Layer* l = container->GetFirstChild(); l; l = l->GetNextSibling()) { + ThebesDisplayItemLayerUserData* data = + static_cast + (l->GetUserData(&gThebesDisplayItemLayerUserData)); + if (data) { + return PredictScaleForContent(aFrame, f, gfxSize(data->mXScale, data->mYScale)); + } + } } } } - nsIFrame::ChildListIterator lists(aFrame); - for (; !lists.IsDone(); lists.Next()) { - if (lists.CurrentID() == nsIFrame::kPopupList || - lists.CurrentID() == nsIFrame::kSelectPopupList) { - continue; - } - - nsFrameList::Enumerator childFrames(lists.CurrentList()); - for (; !childFrames.AtEnd(); childFrames.Next()) { - if (GetThebesLayerResolutionForFrame(childFrames.get(), - aXres, aYres, aPoint)) { - return true; - } - } - } - - return false; + return PredictScaleForContent(aFrame, last, + last->PresContext()->PresShell()->GetResolution()); } #ifdef MOZ_DUMP_PAINTING diff --git a/layout/base/FrameLayerBuilder.h b/layout/base/FrameLayerBuilder.h index 085dfc512b52..9513460235bc 100644 --- a/layout/base/FrameLayerBuilder.h +++ b/layout/base/FrameLayerBuilder.h @@ -369,15 +369,13 @@ public: nsIntPoint GetLastPaintOffset(ThebesLayer* aLayer); /** - * Return resolution and scroll offset of ThebesLayer content associated - * with aFrame's subtree. - * Returns true if some ThebesLayer was found. - * This just looks for the first ThebesLayer and returns its data. There - * could be other ThebesLayers with different resolution and offsets. + * Return the resolution at which we expect to render aFrame's contents, + * assuming they are being painted to retained layers. This takes into account + * the resolution the contents of the ContainerLayer containing aFrame are + * being rendered at, as well as any currently-inactive transforms between + * aFrame and that container layer. */ - static bool GetThebesLayerResolutionForFrame(nsIFrame* aFrame, - double* aXRes, double* aYRes, - gfxPoint* aPoint); + static gfxSize GetThebesLayerScaleForFrame(nsIFrame* aFrame); /** * Clip represents the intersection of an optional rectangle with a diff --git a/layout/base/nsIPresShell.h b/layout/base/nsIPresShell.h index 9bd77a2e12cf..79169524b073 100644 --- a/layout/base/nsIPresShell.h +++ b/layout/base/nsIPresShell.h @@ -1196,6 +1196,7 @@ public: * The resolution defaults to 1.0. */ virtual nsresult SetResolution(float aXResolution, float aYResolution) = 0; + gfxSize GetResolution() { return gfxSize(mXResolution, mYResolution); } float GetXResolution() { return mXResolution; } float GetYResolution() { return mYResolution; } diff --git a/layout/generic/nsGfxScrollFrame.cpp b/layout/generic/nsGfxScrollFrame.cpp index 7d1b88af66c5..cead6404ff25 100644 --- a/layout/generic/nsGfxScrollFrame.cpp +++ b/layout/generic/nsGfxScrollFrame.cpp @@ -1967,38 +1967,34 @@ void nsGfxScrollFrameInner::ScrollVisual(nsPoint aOldScrolledFramePos) } /** - * Adjust the desired scroll value in given range - * in order to get resulting scroll by whole amount of layer pixels. - * Current implementation is not checking that result value is the best. - * Ideally it's would be possible to find best value by implementing - * test function which is repeating last part of CreateOrRecycleThebesLayer, - * and checking other points in allowed range, but that may cause another perf hit. - * Let's keep it as TODO. + * Return an appunit value close to aDesired and between aLower and aUpper + * such that (aDesired - aCurrent)*aRes/aAppUnitsPerPixel is an integer (or + * as close as we can get modulo rounding to appunits). If that + * can't be done, just returns aDesired. */ static nscoord -RestrictToLayerPixels(nscoord aDesired, nscoord aLower, - nscoord aUpper, nscoord aAppUnitsPerPixel, - double aRes, double aCurrentLayerOffset) +AlignWithLayerPixels(nscoord aDesired, nscoord aLower, + nscoord aUpper, nscoord aAppUnitsPerPixel, + double aRes, nscoord aCurrent) { - // convert the result to layer pixels - double layerVal = aRes * double(aDesired) / aAppUnitsPerPixel; + double currentLayerVal = (aRes*aCurrent)/aAppUnitsPerPixel; + double desiredLayerVal = (aRes*aDesired)/aAppUnitsPerPixel; + double delta = desiredLayerVal - currentLayerVal; + double nearestVal = NS_round(delta) + currentLayerVal; - // Correct value using current layer offset - layerVal -= aCurrentLayerOffset; - - // Try nearest pixel bound first - double nearestVal = NS_round(layerVal); + // Convert back from ThebesLayer space to appunits relative to the top-left + // of the scrolled frame. nscoord nearestAppUnitVal = - NSToCoordRoundWithClamp(nearestVal * aAppUnitsPerPixel / aRes); + NSToCoordRoundWithClamp(nearestVal*aAppUnitsPerPixel/aRes); // Check if nearest layer pixel result fit into allowed and scroll range if (nearestAppUnitVal >= aLower && nearestAppUnitVal <= aUpper) { return nearestAppUnitVal; - } else if (nearestVal != layerVal) { + } else if (nearestVal != desiredLayerVal) { // Check if opposite pixel boundary fit into scroll range - double oppositeVal = nearestVal + ((nearestVal < layerVal) ? 1 : -1); + double oppositeVal = nearestVal + ((nearestVal < desiredLayerVal) ? 1 : -1); nscoord oppositeAppUnitVal = - NSToCoordRoundWithClamp(oppositeVal * aAppUnitsPerPixel / aRes); + NSToCoordRoundWithClamp(oppositeVal*aAppUnitsPerPixel/aRes); if (oppositeAppUnitVal >= aLower && oppositeAppUnitVal <= aUpper) { return oppositeAppUnitVal; } @@ -2007,17 +2003,17 @@ RestrictToLayerPixels(nscoord aDesired, nscoord aLower, } /** - * Clamp desired scroll position aPt to aBounds (if aBounds is non-null) and then snap - * it to the nearest layer pixel edges, keeping it within aRange during snapping - * (if aRange is non-null). aCurrScroll is the current scroll position. + * Clamp desired scroll position aPt to aBounds and then snap + * it to the same layer pixel edges as aCurrent, keeping it within aRange + * during snapping. aCurrent is the current scroll position. */ static nsPoint -ClampAndRestrictToLayerPixels(const nsPoint& aPt, - const nsRect& aBounds, - nscoord aAppUnitsPerPixel, - const nsRect& aRange, - double aXRes, double aYRes, - const gfxPoint& aCurrScroll) +ClampAndAlignWithLayerPixels(const nsPoint& aPt, + const nsRect& aBounds, + const nsRect& aRange, + const nsPoint& aCurrent, + nscoord aAppUnitsPerPixel, + const gfxSize& aScale) { nsPoint pt = aBounds.ClampPoint(aPt); // Intersect scroll range with allowed range, by clamping the corners @@ -2025,10 +2021,10 @@ ClampAndRestrictToLayerPixels(const nsPoint& aPt, nsPoint rangeTopLeft = aBounds.ClampPoint(aRange.TopLeft()); nsPoint rangeBottomRight = aBounds.ClampPoint(aRange.BottomRight()); - return nsPoint(RestrictToLayerPixels(pt.x, rangeTopLeft.x, rangeBottomRight.x, - aAppUnitsPerPixel, aXRes, aCurrScroll.x), - RestrictToLayerPixels(pt.y, rangeTopLeft.y, rangeBottomRight.y, - aAppUnitsPerPixel, aYRes, aCurrScroll.y)); + return nsPoint(AlignWithLayerPixels(pt.x, rangeTopLeft.x, rangeBottomRight.x, + aAppUnitsPerPixel, aScale.width, aCurrent.x), + AlignWithLayerPixels(pt.y, rangeTopLeft.y, rangeBottomRight.y, + aAppUnitsPerPixel, aScale.height, aCurrent.y)); } /* static */ void @@ -2061,19 +2057,28 @@ nsGfxScrollFrameInner::ScrollToImpl(nsPoint aPt, const nsRect& aRange) { nsPresContext* presContext = mOuter->PresContext(); nscoord appUnitsPerDevPixel = presContext->AppUnitsPerDevPixel(); - - double xres = 1.0, yres = 1.0; - gfxPoint activeScrolledRootPosition; - FrameLayerBuilder::GetThebesLayerResolutionForFrame(mScrolledFrame, &xres, &yres, - &activeScrolledRootPosition); - nsPoint pt = - ClampAndRestrictToLayerPixels(aPt, - GetScrollRangeForClamping(), - appUnitsPerDevPixel, - aRange, xres, yres, - activeScrolledRootPosition); - + // 'scale' is our estimate of the scale factor that will be applied + // when rendering the scrolled content to its own ThebesLayer. + gfxSize scale = FrameLayerBuilder::GetThebesLayerScaleForFrame(mScrolledFrame); nsPoint curPos = GetScrollPosition(); + // Try to align aPt with curPos so they have an integer number of layer + // pixels between them. This gives us the best chance of scrolling without + // having to invalidate due to changes in subpixel rendering. + // Note that when we actually draw into a ThebesLayer, the coordinates + // that get mapped onto the layer buffer pixels are from the display list, + // which are relative to the display root frame's top-left increasing down, + // whereas here our coordinates are scroll positions which increase upward + // and are relative to the scrollport top-left. This difference doesn't actually + // matter since all we are about is that there be an integer number of + // layer pixels between pt and curPos. + nsPoint pt = + ClampAndAlignWithLayerPixels(aPt, + GetScrollRangeForClamping(), + aRange, + curPos, + appUnitsPerDevPixel, + scale); + if (pt == curPos) { return; } From f7b6f9a9520f8e6c50a4ce607e8f6ea8cb863f10 Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Sun, 5 Aug 2012 00:26:58 +1200 Subject: [PATCH 11/22] Bug 772679. Testcase. --- layout/base/tests/chrome/Makefile.in | 1 + .../tests/chrome/test_scrolling_repaints.html | 49 +++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 layout/base/tests/chrome/test_scrolling_repaints.html diff --git a/layout/base/tests/chrome/Makefile.in b/layout/base/tests/chrome/Makefile.in index 2f0f59bb2c19..bcb21fe533c4 100644 --- a/layout/base/tests/chrome/Makefile.in +++ b/layout/base/tests/chrome/Makefile.in @@ -40,6 +40,7 @@ MOCHITEST_CHROME_FILES = \ printpreview_bug396024_helper.xul \ test_printpreview_bug482976.xul \ printpreview_bug482976_helper.xul \ + test_scrolling_repaints.html \ test_transformed_scrolling_repaints.html \ test_transformed_scrolling_repaints_2.html \ $(NULL) diff --git a/layout/base/tests/chrome/test_scrolling_repaints.html b/layout/base/tests/chrome/test_scrolling_repaints.html new file mode 100644 index 000000000000..db163207eee3 --- /dev/null +++ b/layout/base/tests/chrome/test_scrolling_repaints.html @@ -0,0 +1,49 @@ + + + + Test that we don't get unnecessary repaints due to subpixel shifts + + + + + + +
+
+
+
+
+
+
+
+ + From 21bdb5dd1eaf7a80fad6076129adc374dd559718 Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Sun, 5 Aug 2012 00:27:07 +1200 Subject: [PATCH 12/22] Bug 772679. Mark test as passing on Mac. --- .../tests/chrome/test_transformed_scrolling_repaints_2.html | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/layout/base/tests/chrome/test_transformed_scrolling_repaints_2.html b/layout/base/tests/chrome/test_transformed_scrolling_repaints_2.html index 0651e165eeae..45eaac49c28b 100644 --- a/layout/base/tests/chrome/test_transformed_scrolling_repaints_2.html +++ b/layout/base/tests/chrome/test_transformed_scrolling_repaints_2.html @@ -34,11 +34,7 @@ function startTest() { t.scrollTop = 33; waitForAllPaintsFlushed(function () { var painted = utils.checkAndClearPaintedState(e); - if (navigator.platform.indexOf("Mac") >= 0) { - todo_is(painted, false, "Fully-visible scrolled element should not have been painted (disabled on Mac, see bug 753497)"); - } else { - is(painted, false, "Fully-visible scrolled element should not have been painted"); - } + is(painted, false, "Fully-visible scrolled element should not have been painted"); SimpleTest.finish(); }); }); From b1946a2297fdb8a002f39576be77c54bf510cb11 Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Sun, 5 Aug 2012 20:59:06 +1200 Subject: [PATCH 13/22] Bug 772679. Handle frame with NS_HAS_CONTAINER_LAYER having no display items. r=tnikkel --- layout/base/FrameLayerBuilder.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/layout/base/FrameLayerBuilder.cpp b/layout/base/FrameLayerBuilder.cpp index e54c2ed8f355..e6c608089b22 100644 --- a/layout/base/FrameLayerBuilder.cpp +++ b/layout/base/FrameLayerBuilder.cpp @@ -2528,7 +2528,10 @@ FrameLayerBuilder::GetThebesLayerScaleForFrame(nsIFrame* aFrame) last = f; if (f->GetStateBits() & NS_FRAME_HAS_CONTAINER_LAYER) { nsTArray* array = GetDisplayItemDataArrayForFrame(f); - NS_ASSERTION(array, "Must have display item data for container"); + // Some frames with NS_FRAME_HAS_CONTAINER_LAYER may not have display items. + // In particular the root frame probably doesn't! + if (!array) + continue; for (PRUint32 i = 0; i < array->Length(); ++i) { Layer* layer = array->ElementAt(i).mLayer; ContainerLayer* container = layer->AsContainerLayer(); From 612bd2f8d912d47e01948426f2f59bffb35c5456 Mon Sep 17 00:00:00 2001 From: Aryeh Gregor Date: Thu, 5 Jul 2012 10:45:09 +0300 Subject: [PATCH 14/22] Bug 732153 part 1 - Computed transform should be 'none' if there's no transform, even if backface-visibility or transform-style is set; r=dbaron --- layout/style/nsComputedDOMStyle.cpp | 6 +++--- layout/style/test/Makefile.in | 1 + layout/style/test/test_bug732153.html | 22 ++++++++++++++++++++++ 3 files changed, 26 insertions(+), 3 deletions(-) create mode 100644 layout/style/test/test_bug732153.html diff --git a/layout/style/nsComputedDOMStyle.cpp b/layout/style/nsComputedDOMStyle.cpp index 6facddef449c..b5269a3bc8a0 100644 --- a/layout/style/nsComputedDOMStyle.cpp +++ b/layout/style/nsComputedDOMStyle.cpp @@ -993,10 +993,10 @@ nsComputedDOMStyle::DoGetTransform() /* First, get the display data. We'll need it. */ const nsStyleDisplay* display = GetStyleDisplay(); - /* If the "no transforms" flag is set, then we should construct a - * single-element entry and hand it back. + /* If there are no transforms, then we should construct a single-element + * entry and hand it back. */ - if (!display->HasTransform()) { + if (!display->mSpecifiedTransform) { nsROCSSPrimitiveValue* val = GetROCSSPrimitiveValue(); /* Set it to "none." */ diff --git a/layout/style/test/Makefile.in b/layout/style/test/Makefile.in index c9429f324d0e..7a2d69a4db19 100644 --- a/layout/style/test/Makefile.in +++ b/layout/style/test/Makefile.in @@ -185,6 +185,7 @@ MOCHITEST_FILES = test_acid3_test46.html \ visited_image_loading_frame_empty.html \ test_load_events_on_stylesheets.html \ test_bug721136.html \ + test_bug732153.html \ $(NULL) ifdef MOZ_FLEXBOX diff --git a/layout/style/test/test_bug732153.html b/layout/style/test/test_bug732153.html new file mode 100644 index 000000000000..892029886097 --- /dev/null +++ b/layout/style/test/test_bug732153.html @@ -0,0 +1,22 @@ + + +Test for Bug 732153 + + +Mozilla Bug 732153 +
+ From 91d92e952eb7bf11ad8045a665c518c7dcd81e79 Mon Sep 17 00:00:00 2001 From: Siddharth Agarwal Date: Sun, 5 Aug 2012 12:42:57 +0200 Subject: [PATCH 15/22] Bug 780421 - Use $(PYTHON) to run config.status. r=glandium --- client.mk | 2 +- config/rules.mk | 8 ++++---- gfx/cairo/cairo/src/Makefile.in | 2 +- js/src/config/rules.mk | 8 ++++---- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/client.mk b/client.mk index eba2a73e3948..1ba3cac8f723 100644 --- a/client.mk +++ b/client.mk @@ -320,7 +320,7 @@ endif ifneq (,$(CONFIG_STATUS)) $(OBJDIR)/config/autoconf.mk: $(TOPSRCDIR)/config/autoconf.mk.in - $(OBJDIR)/config.status -n --file=$(OBJDIR)/config/autoconf.mk + $(PYTHON) $(OBJDIR)/config.status -n --file=$(OBJDIR)/config/autoconf.mk endif diff --git a/config/rules.mk b/config/rules.mk index 4636724057f1..4579a1548e25 100644 --- a/config/rules.mk +++ b/config/rules.mk @@ -1154,24 +1154,24 @@ GARBAGE_DIRS += $(_JAVA_DIR) ifndef NO_MAKEFILE_RULE Makefile: Makefile.in - @$(DEPTH)/config.status -n --file=Makefile + @$(PYTHON) $(DEPTH)/config.status -n --file=Makefile endif ifndef NO_SUBMAKEFILES_RULE ifdef SUBMAKEFILES # VPATH does not work on some machines in this case, so add $(srcdir) $(SUBMAKEFILES): % : $(srcdir)/%.in - $(DEPTH)$(addprefix /,$(subsrcdir))/config.status -n --file=$@ + $(PYTHON) $(DEPTH)$(addprefix /,$(subsrcdir))/config.status -n --file=$@ endif endif ifdef AUTOUPDATE_CONFIGURE $(topsrcdir)/configure: $(topsrcdir)/configure.in - (cd $(topsrcdir) && $(AUTOCONF)) && $(DEPTH)/config.status -n --recheck) + (cd $(topsrcdir) && $(AUTOCONF)) && $(PYTHON) $(DEPTH)/config.status -n --recheck) endif $(DEPTH)/config/autoconf.mk: $(topsrcdir)/config/autoconf.mk.in - $(DEPTH)/config.status -n --file=$(DEPTH)/config/autoconf.mk + $(PYTHON) $(DEPTH)/config.status -n --file=$(DEPTH)/config/autoconf.mk $(TOUCH) $@ ############################################################################### diff --git a/gfx/cairo/cairo/src/Makefile.in b/gfx/cairo/cairo/src/Makefile.in index 249d8f955372..83a309b27fc7 100644 --- a/gfx/cairo/cairo/src/Makefile.in +++ b/gfx/cairo/cairo/src/Makefile.in @@ -233,4 +233,4 @@ DEFINES += -DMOZ_TREE_PIXMAN endif cairo-features.h: $(srcdir)/cairo-features.h.in $(GLOBAL_DEPS) - $(DEPTH)/config.status -n --file=$@ + $(PYTHON) $(DEPTH)/config.status -n --file=$@ diff --git a/js/src/config/rules.mk b/js/src/config/rules.mk index 4636724057f1..4579a1548e25 100644 --- a/js/src/config/rules.mk +++ b/js/src/config/rules.mk @@ -1154,24 +1154,24 @@ GARBAGE_DIRS += $(_JAVA_DIR) ifndef NO_MAKEFILE_RULE Makefile: Makefile.in - @$(DEPTH)/config.status -n --file=Makefile + @$(PYTHON) $(DEPTH)/config.status -n --file=Makefile endif ifndef NO_SUBMAKEFILES_RULE ifdef SUBMAKEFILES # VPATH does not work on some machines in this case, so add $(srcdir) $(SUBMAKEFILES): % : $(srcdir)/%.in - $(DEPTH)$(addprefix /,$(subsrcdir))/config.status -n --file=$@ + $(PYTHON) $(DEPTH)$(addprefix /,$(subsrcdir))/config.status -n --file=$@ endif endif ifdef AUTOUPDATE_CONFIGURE $(topsrcdir)/configure: $(topsrcdir)/configure.in - (cd $(topsrcdir) && $(AUTOCONF)) && $(DEPTH)/config.status -n --recheck) + (cd $(topsrcdir) && $(AUTOCONF)) && $(PYTHON) $(DEPTH)/config.status -n --recheck) endif $(DEPTH)/config/autoconf.mk: $(topsrcdir)/config/autoconf.mk.in - $(DEPTH)/config.status -n --file=$(DEPTH)/config/autoconf.mk + $(PYTHON) $(DEPTH)/config.status -n --file=$(DEPTH)/config/autoconf.mk $(TOUCH) $@ ############################################################################### From 4d843c664585ecddb09b73791fd1b112a0855f8b Mon Sep 17 00:00:00 2001 From: Masatoshi Kimura Date: Sun, 5 Aug 2012 12:42:59 +0200 Subject: [PATCH 16/22] Bug 780430 - Declare encoding in config.status. r=glandium --- build/autoconf/config.status.m4 | 3 +++ js/src/build/autoconf/config.status.m4 | 3 +++ 2 files changed, 6 insertions(+) diff --git a/build/autoconf/config.status.m4 b/build/autoconf/config.status.m4 index dbc0d1e97d14..deec976f4b5b 100644 --- a/build/autoconf/config.status.m4 +++ b/build/autoconf/config.status.m4 @@ -45,9 +45,11 @@ dnl Replace AC_OUTPUT to create and call a python config.status define([AC_OUTPUT], [dnl Top source directory in Windows format (as opposed to msys format). WIN_TOP_SRC= +encoding=utf-8 case "$host_os" in mingw*) WIN_TOP_SRC=`cd $srcdir; pwd -W` + encoding=mbcs ;; esac AC_SUBST(WIN_TOP_SRC) @@ -74,6 +76,7 @@ echo creating $CONFIG_STATUS cat > $CONFIG_STATUS < $CONFIG_STATUS < Date: Sun, 5 Aug 2012 12:43:00 +0200 Subject: [PATCH 17/22] Bug 780446 - touch Makefile after invoking config.status. r=glandium --- config/rules.mk | 2 ++ js/src/config/rules.mk | 2 ++ 2 files changed, 4 insertions(+) diff --git a/config/rules.mk b/config/rules.mk index 4579a1548e25..28508b7a43b0 100644 --- a/config/rules.mk +++ b/config/rules.mk @@ -1155,6 +1155,7 @@ GARBAGE_DIRS += $(_JAVA_DIR) ifndef NO_MAKEFILE_RULE Makefile: Makefile.in @$(PYTHON) $(DEPTH)/config.status -n --file=Makefile + @$(TOUCH) $@ endif ifndef NO_SUBMAKEFILES_RULE @@ -1162,6 +1163,7 @@ ifdef SUBMAKEFILES # VPATH does not work on some machines in this case, so add $(srcdir) $(SUBMAKEFILES): % : $(srcdir)/%.in $(PYTHON) $(DEPTH)$(addprefix /,$(subsrcdir))/config.status -n --file=$@ + @$(TOUCH) $@ endif endif diff --git a/js/src/config/rules.mk b/js/src/config/rules.mk index 4579a1548e25..28508b7a43b0 100644 --- a/js/src/config/rules.mk +++ b/js/src/config/rules.mk @@ -1155,6 +1155,7 @@ GARBAGE_DIRS += $(_JAVA_DIR) ifndef NO_MAKEFILE_RULE Makefile: Makefile.in @$(PYTHON) $(DEPTH)/config.status -n --file=Makefile + @$(TOUCH) $@ endif ifndef NO_SUBMAKEFILES_RULE @@ -1162,6 +1163,7 @@ ifdef SUBMAKEFILES # VPATH does not work on some machines in this case, so add $(srcdir) $(SUBMAKEFILES): % : $(srcdir)/%.in $(PYTHON) $(DEPTH)$(addprefix /,$(subsrcdir))/config.status -n --file=$@ + @$(TOUCH) $@ endif endif From 553aa166175ea4cc8b0bbe9a9ba2db31386cf14e Mon Sep 17 00:00:00 2001 From: Joel Maher Date: Sun, 5 Aug 2012 07:36:35 -0400 Subject: [PATCH 18/22] Bug 687032 - Intermittent test_bug590812.html | noxul domain same as ref and test_bug590812.html | xul supporting domain same as ref. r=edmorley --- testing/mochitest/android.json | 1 + 1 file changed, 1 insertion(+) diff --git a/testing/mochitest/android.json b/testing/mochitest/android.json index bd741e75b490..6f827b0f28ea 100644 --- a/testing/mochitest/android.json +++ b/testing/mochitest/android.json @@ -1,6 +1,7 @@ { "runtests": {}, "excludetests": { + "content/base/test/test_bug590812.html": "bug 687032", "content/base/test/test_CSP.html": "TIMED_OUT", "content/base/test/test_CSP_frameancestors.html": "", "content/base/test/test_CSP_inlinescript.html": "", From d0459ad8bad91611c8af3e2c85c13883c5ccdc47 Mon Sep 17 00:00:00 2001 From: Geoff Brown Date: Sun, 5 Aug 2012 13:09:44 +0100 Subject: [PATCH 19/22] Bug 775227: tweak set of Android M3 tests disabled for OOM; r=jmaher --- testing/mochitest/android.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/testing/mochitest/android.json b/testing/mochitest/android.json index 6f827b0f28ea..5af97f394050 100644 --- a/testing/mochitest/android.json +++ b/testing/mochitest/android.json @@ -125,7 +125,6 @@ "docshell/test/test_bug637644.html": "", "docshell/test/test_bug668513.html": "RANDOM", "dom/browser-element/mochitest/test_browserElement_oop_SecurityChange.html": "TIMED_OUT, bug 766586", - "dom/browser-element/mochitest/test_browserElement_inproc_Iconchange.html": "bug 775227", "dom/browser-element/mochitest/test_browserElement_inproc_SecurityChange.html": "TIMED_OUT, bug 766586", "dom/imptests/editing/conformancetest/test_event.html": "", "dom/imptests/editing/conformancetest/test_runtest.html": "", @@ -141,7 +140,7 @@ "dom/imptests/webapps/DOMCore/tests/approved/test_Range-mutations.html": "bug 775227", "dom/imptests/webapps/DOMCore/tests/approved/test_Range-set.html": "bug 775227", "dom/imptests/webapps/DOMCore/tests/approved/test_Range-surroundContents.html": "bug 775227", - "dom/imptests/webapps/WebStorage/tests/submissions/Infraware/test_event_session_key.html": "bug 775227", + "dom/imptests/webapps/WebStorage/tests/submissions/Infraware/test_storage_local_key.html": "bug 775227", "dom/indexedDB/test/test_third_party.html": "TIMED_OUT", "dom/network/tests/test_network_basics.html": "", "dom/settings/tests/test_settings_events.html": "", From 846d3da1b5a2bca1d6b70a575d304dc9effbd114 Mon Sep 17 00:00:00 2001 From: Matt Brubeck Date: Sun, 5 Aug 2012 13:20:23 +0100 Subject: [PATCH 20/22] Bug 779551 - Mark spellcheck-superscript-2.html as failing on native Android, so reftest-3 can be unhidden; rs=edmorley --- editor/reftests/reftest.list | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/editor/reftests/reftest.list b/editor/reftests/reftest.list index 62a179837318..15555821efc0 100644 --- a/editor/reftests/reftest.list +++ b/editor/reftests/reftest.list @@ -97,4 +97,4 @@ skip-if(Android) == 462758-grabbers-resizers.html 462758-grabbers-resizers-ref.h == 694880-3.html 694880-ref.html == 388980-1.html 388980-1-ref.html needs-focus == spellcheck-superscript-1.html spellcheck-superscript-1-ref.html -needs-focus != spellcheck-superscript-2.html spellcheck-superscript-2-ref.html +fails-if(Android) needs-focus != spellcheck-superscript-2.html spellcheck-superscript-2-ref.html From d3209cdecebd56a2f47ccdefaad7807d6dcd828a Mon Sep 17 00:00:00 2001 From: Henri Sivonen Date: Sun, 5 Aug 2012 16:28:48 +0300 Subject: [PATCH 21/22] Bug 768138 - s/must to be/must be/ in a parser error message. r=smaug. --- dom/locales/en-US/chrome/layout/htmlparser.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dom/locales/en-US/chrome/layout/htmlparser.properties b/dom/locales/en-US/chrome/layout/htmlparser.properties index 9210b6b95402..d83e7798becb 100644 --- a/dom/locales/en-US/chrome/layout/htmlparser.properties +++ b/dom/locales/en-US/chrome/layout/htmlparser.properties @@ -5,7 +5,7 @@ # Encoding warnings and errors EncNoDeclarationFrame=The character encoding of a framed document was not declared. The document may appear different if viewed without the document framing it. EncNoDeclarationPlain=The character encoding of the plain text document was not declared. The document will render with garbled text in some browser configurations if the document contains characters from outside the US-ASCII range. The character encoding of the file needs to be declared in the transfer protocol or file needs to use a byte order mark as an encoding signature. -EncNoDeclaration=The character encoding of the HTML document was not declared. The document will render with garbled text in some browser configurations if the document contains characters from outside the US-ASCII range. The character encoding of the page must to be declared in the document or in the transfer protocol. +EncNoDeclaration=The character encoding of the HTML document was not declared. The document will render with garbled text in some browser configurations if the document contains characters from outside the US-ASCII range. The character encoding of the page must be declared in the document or in the transfer protocol. EncLateMetaFrame=The character encoding declaration of the framed HTML document was not found when prescanning the first 1024 bytes of the file. When viewed without the document framing it, the page will reload automatically. The encoding declaration needs to be moved to be within the first 1024 bytes of the file. EncLateMeta=The character encoding declaration of the HTML document was not found when prescanning the first 1024 bytes of the file. When viewed in a differently-configured browser, this page will reload automatically. The encoding declaration needs to be moved to be within the first 1024 bytes of the file. EncLateMetaReload=The page was reloaded, because the character encoding declaration of the HTML document was not found when prescanning the first 1024 bytes of the file. The encoding declaration needs to be moved to be within the first 1024 bytes of the file. From 5be29bb8fd688d9ecf2613e623ce133b2d88c48f Mon Sep 17 00:00:00 2001 From: Ed Morley Date: Sun, 5 Aug 2012 14:35:08 +0100 Subject: [PATCH 22/22] Backout 6ea008b301da, 14d17919e235, b15fb3603bfe & f89ae41eed63 (bug 772679) for turning test_offsets.html perma-orange on native Android mochitest-7 --- layout/base/FrameLayerBuilder.cpp | 75 +++++++------- layout/base/FrameLayerBuilder.h | 14 +-- layout/base/nsIPresShell.h | 1 - layout/base/tests/chrome/Makefile.in | 1 - .../tests/chrome/test_scrolling_repaints.html | 49 ---------- ...test_transformed_scrolling_repaints_2.html | 6 +- layout/generic/nsGfxScrollFrame.cpp | 97 +++++++++---------- 7 files changed, 92 insertions(+), 151 deletions(-) delete mode 100644 layout/base/tests/chrome/test_scrolling_repaints.html diff --git a/layout/base/FrameLayerBuilder.cpp b/layout/base/FrameLayerBuilder.cpp index e6c608089b22..a2c91740ab2f 100644 --- a/layout/base/FrameLayerBuilder.cpp +++ b/layout/base/FrameLayerBuilder.cpp @@ -2505,53 +2505,44 @@ FrameLayerBuilder::GetDedicatedLayer(nsIFrame* aFrame, PRUint32 aDisplayItemKey) return nullptr; } -static gfxSize -PredictScaleForContent(nsIFrame* aFrame, nsIFrame* aAncestorWithScale, - const gfxSize& aScale) +bool +FrameLayerBuilder::GetThebesLayerResolutionForFrame(nsIFrame* aFrame, + double* aXres, double* aYres, + gfxPoint* aPoint) { - gfx3DMatrix transform = - gfx3DMatrix::ScalingMatrix(aScale.width, aScale.height, 1.0); - // aTransform is applied first, then the scale is applied to the result - transform = nsLayoutUtils::GetTransformToAncestor(aFrame, aAncestorWithScale)*transform; - gfxMatrix transform2d; - if (transform.CanDraw2D(&transform2d)) { - return transform2d.ScaleFactors(true); - } - return gfxSize(1.0, 1.0); -} - -gfxSize -FrameLayerBuilder::GetThebesLayerScaleForFrame(nsIFrame* aFrame) -{ - nsIFrame* last; - for (nsIFrame* f = aFrame; f; f = nsLayoutUtils::GetCrossDocParentFrame(f)) { - last = f; - if (f->GetStateBits() & NS_FRAME_HAS_CONTAINER_LAYER) { - nsTArray* array = GetDisplayItemDataArrayForFrame(f); - // Some frames with NS_FRAME_HAS_CONTAINER_LAYER may not have display items. - // In particular the root frame probably doesn't! - if (!array) - continue; - for (PRUint32 i = 0; i < array->Length(); ++i) { - Layer* layer = array->ElementAt(i).mLayer; - ContainerLayer* container = layer->AsContainerLayer(); - if (!container) { - continue; - } - for (Layer* l = container->GetFirstChild(); l; l = l->GetNextSibling()) { - ThebesDisplayItemLayerUserData* data = - static_cast - (l->GetUserData(&gThebesDisplayItemLayerUserData)); - if (data) { - return PredictScaleForContent(aFrame, f, gfxSize(data->mXScale, data->mYScale)); - } - } + nsTArray *array = GetDisplayItemDataArrayForFrame(aFrame); + if (array) { + for (PRUint32 i = 0; i < array->Length(); ++i) { + Layer* layer = array->ElementAt(i).mLayer; + if (layer->HasUserData(&gThebesDisplayItemLayerUserData)) { + ThebesDisplayItemLayerUserData* data = + static_cast + (layer->GetUserData(&gThebesDisplayItemLayerUserData)); + *aXres = data->mXScale; + *aYres = data->mYScale; + *aPoint = data->mActiveScrolledRootPosition; + return true; } } } - return PredictScaleForContent(aFrame, last, - last->PresContext()->PresShell()->GetResolution()); + nsIFrame::ChildListIterator lists(aFrame); + for (; !lists.IsDone(); lists.Next()) { + if (lists.CurrentID() == nsIFrame::kPopupList || + lists.CurrentID() == nsIFrame::kSelectPopupList) { + continue; + } + + nsFrameList::Enumerator childFrames(lists.CurrentList()); + for (; !childFrames.AtEnd(); childFrames.Next()) { + if (GetThebesLayerResolutionForFrame(childFrames.get(), + aXres, aYres, aPoint)) { + return true; + } + } + } + + return false; } #ifdef MOZ_DUMP_PAINTING diff --git a/layout/base/FrameLayerBuilder.h b/layout/base/FrameLayerBuilder.h index 9513460235bc..085dfc512b52 100644 --- a/layout/base/FrameLayerBuilder.h +++ b/layout/base/FrameLayerBuilder.h @@ -369,13 +369,15 @@ public: nsIntPoint GetLastPaintOffset(ThebesLayer* aLayer); /** - * Return the resolution at which we expect to render aFrame's contents, - * assuming they are being painted to retained layers. This takes into account - * the resolution the contents of the ContainerLayer containing aFrame are - * being rendered at, as well as any currently-inactive transforms between - * aFrame and that container layer. + * Return resolution and scroll offset of ThebesLayer content associated + * with aFrame's subtree. + * Returns true if some ThebesLayer was found. + * This just looks for the first ThebesLayer and returns its data. There + * could be other ThebesLayers with different resolution and offsets. */ - static gfxSize GetThebesLayerScaleForFrame(nsIFrame* aFrame); + static bool GetThebesLayerResolutionForFrame(nsIFrame* aFrame, + double* aXRes, double* aYRes, + gfxPoint* aPoint); /** * Clip represents the intersection of an optional rectangle with a diff --git a/layout/base/nsIPresShell.h b/layout/base/nsIPresShell.h index 79169524b073..9bd77a2e12cf 100644 --- a/layout/base/nsIPresShell.h +++ b/layout/base/nsIPresShell.h @@ -1196,7 +1196,6 @@ public: * The resolution defaults to 1.0. */ virtual nsresult SetResolution(float aXResolution, float aYResolution) = 0; - gfxSize GetResolution() { return gfxSize(mXResolution, mYResolution); } float GetXResolution() { return mXResolution; } float GetYResolution() { return mYResolution; } diff --git a/layout/base/tests/chrome/Makefile.in b/layout/base/tests/chrome/Makefile.in index bcb21fe533c4..2f0f59bb2c19 100644 --- a/layout/base/tests/chrome/Makefile.in +++ b/layout/base/tests/chrome/Makefile.in @@ -40,7 +40,6 @@ MOCHITEST_CHROME_FILES = \ printpreview_bug396024_helper.xul \ test_printpreview_bug482976.xul \ printpreview_bug482976_helper.xul \ - test_scrolling_repaints.html \ test_transformed_scrolling_repaints.html \ test_transformed_scrolling_repaints_2.html \ $(NULL) diff --git a/layout/base/tests/chrome/test_scrolling_repaints.html b/layout/base/tests/chrome/test_scrolling_repaints.html deleted file mode 100644 index db163207eee3..000000000000 --- a/layout/base/tests/chrome/test_scrolling_repaints.html +++ /dev/null @@ -1,49 +0,0 @@ - - - - Test that we don't get unnecessary repaints due to subpixel shifts - - - - - - -
-
-
-
-
-
-
-
- - diff --git a/layout/base/tests/chrome/test_transformed_scrolling_repaints_2.html b/layout/base/tests/chrome/test_transformed_scrolling_repaints_2.html index 45eaac49c28b..0651e165eeae 100644 --- a/layout/base/tests/chrome/test_transformed_scrolling_repaints_2.html +++ b/layout/base/tests/chrome/test_transformed_scrolling_repaints_2.html @@ -34,7 +34,11 @@ function startTest() { t.scrollTop = 33; waitForAllPaintsFlushed(function () { var painted = utils.checkAndClearPaintedState(e); - is(painted, false, "Fully-visible scrolled element should not have been painted"); + if (navigator.platform.indexOf("Mac") >= 0) { + todo_is(painted, false, "Fully-visible scrolled element should not have been painted (disabled on Mac, see bug 753497)"); + } else { + is(painted, false, "Fully-visible scrolled element should not have been painted"); + } SimpleTest.finish(); }); }); diff --git a/layout/generic/nsGfxScrollFrame.cpp b/layout/generic/nsGfxScrollFrame.cpp index cead6404ff25..7d1b88af66c5 100644 --- a/layout/generic/nsGfxScrollFrame.cpp +++ b/layout/generic/nsGfxScrollFrame.cpp @@ -1967,34 +1967,38 @@ void nsGfxScrollFrameInner::ScrollVisual(nsPoint aOldScrolledFramePos) } /** - * Return an appunit value close to aDesired and between aLower and aUpper - * such that (aDesired - aCurrent)*aRes/aAppUnitsPerPixel is an integer (or - * as close as we can get modulo rounding to appunits). If that - * can't be done, just returns aDesired. + * Adjust the desired scroll value in given range + * in order to get resulting scroll by whole amount of layer pixels. + * Current implementation is not checking that result value is the best. + * Ideally it's would be possible to find best value by implementing + * test function which is repeating last part of CreateOrRecycleThebesLayer, + * and checking other points in allowed range, but that may cause another perf hit. + * Let's keep it as TODO. */ static nscoord -AlignWithLayerPixels(nscoord aDesired, nscoord aLower, - nscoord aUpper, nscoord aAppUnitsPerPixel, - double aRes, nscoord aCurrent) +RestrictToLayerPixels(nscoord aDesired, nscoord aLower, + nscoord aUpper, nscoord aAppUnitsPerPixel, + double aRes, double aCurrentLayerOffset) { - double currentLayerVal = (aRes*aCurrent)/aAppUnitsPerPixel; - double desiredLayerVal = (aRes*aDesired)/aAppUnitsPerPixel; - double delta = desiredLayerVal - currentLayerVal; - double nearestVal = NS_round(delta) + currentLayerVal; + // convert the result to layer pixels + double layerVal = aRes * double(aDesired) / aAppUnitsPerPixel; - // Convert back from ThebesLayer space to appunits relative to the top-left - // of the scrolled frame. + // Correct value using current layer offset + layerVal -= aCurrentLayerOffset; + + // Try nearest pixel bound first + double nearestVal = NS_round(layerVal); nscoord nearestAppUnitVal = - NSToCoordRoundWithClamp(nearestVal*aAppUnitsPerPixel/aRes); + NSToCoordRoundWithClamp(nearestVal * aAppUnitsPerPixel / aRes); // Check if nearest layer pixel result fit into allowed and scroll range if (nearestAppUnitVal >= aLower && nearestAppUnitVal <= aUpper) { return nearestAppUnitVal; - } else if (nearestVal != desiredLayerVal) { + } else if (nearestVal != layerVal) { // Check if opposite pixel boundary fit into scroll range - double oppositeVal = nearestVal + ((nearestVal < desiredLayerVal) ? 1 : -1); + double oppositeVal = nearestVal + ((nearestVal < layerVal) ? 1 : -1); nscoord oppositeAppUnitVal = - NSToCoordRoundWithClamp(oppositeVal*aAppUnitsPerPixel/aRes); + NSToCoordRoundWithClamp(oppositeVal * aAppUnitsPerPixel / aRes); if (oppositeAppUnitVal >= aLower && oppositeAppUnitVal <= aUpper) { return oppositeAppUnitVal; } @@ -2003,17 +2007,17 @@ AlignWithLayerPixels(nscoord aDesired, nscoord aLower, } /** - * Clamp desired scroll position aPt to aBounds and then snap - * it to the same layer pixel edges as aCurrent, keeping it within aRange - * during snapping. aCurrent is the current scroll position. + * Clamp desired scroll position aPt to aBounds (if aBounds is non-null) and then snap + * it to the nearest layer pixel edges, keeping it within aRange during snapping + * (if aRange is non-null). aCurrScroll is the current scroll position. */ static nsPoint -ClampAndAlignWithLayerPixels(const nsPoint& aPt, - const nsRect& aBounds, - const nsRect& aRange, - const nsPoint& aCurrent, - nscoord aAppUnitsPerPixel, - const gfxSize& aScale) +ClampAndRestrictToLayerPixels(const nsPoint& aPt, + const nsRect& aBounds, + nscoord aAppUnitsPerPixel, + const nsRect& aRange, + double aXRes, double aYRes, + const gfxPoint& aCurrScroll) { nsPoint pt = aBounds.ClampPoint(aPt); // Intersect scroll range with allowed range, by clamping the corners @@ -2021,10 +2025,10 @@ ClampAndAlignWithLayerPixels(const nsPoint& aPt, nsPoint rangeTopLeft = aBounds.ClampPoint(aRange.TopLeft()); nsPoint rangeBottomRight = aBounds.ClampPoint(aRange.BottomRight()); - return nsPoint(AlignWithLayerPixels(pt.x, rangeTopLeft.x, rangeBottomRight.x, - aAppUnitsPerPixel, aScale.width, aCurrent.x), - AlignWithLayerPixels(pt.y, rangeTopLeft.y, rangeBottomRight.y, - aAppUnitsPerPixel, aScale.height, aCurrent.y)); + return nsPoint(RestrictToLayerPixels(pt.x, rangeTopLeft.x, rangeBottomRight.x, + aAppUnitsPerPixel, aXRes, aCurrScroll.x), + RestrictToLayerPixels(pt.y, rangeTopLeft.y, rangeBottomRight.y, + aAppUnitsPerPixel, aYRes, aCurrScroll.y)); } /* static */ void @@ -2057,28 +2061,19 @@ nsGfxScrollFrameInner::ScrollToImpl(nsPoint aPt, const nsRect& aRange) { nsPresContext* presContext = mOuter->PresContext(); nscoord appUnitsPerDevPixel = presContext->AppUnitsPerDevPixel(); - // 'scale' is our estimate of the scale factor that will be applied - // when rendering the scrolled content to its own ThebesLayer. - gfxSize scale = FrameLayerBuilder::GetThebesLayerScaleForFrame(mScrolledFrame); - nsPoint curPos = GetScrollPosition(); - // Try to align aPt with curPos so they have an integer number of layer - // pixels between them. This gives us the best chance of scrolling without - // having to invalidate due to changes in subpixel rendering. - // Note that when we actually draw into a ThebesLayer, the coordinates - // that get mapped onto the layer buffer pixels are from the display list, - // which are relative to the display root frame's top-left increasing down, - // whereas here our coordinates are scroll positions which increase upward - // and are relative to the scrollport top-left. This difference doesn't actually - // matter since all we are about is that there be an integer number of - // layer pixels between pt and curPos. - nsPoint pt = - ClampAndAlignWithLayerPixels(aPt, - GetScrollRangeForClamping(), - aRange, - curPos, - appUnitsPerDevPixel, - scale); + double xres = 1.0, yres = 1.0; + gfxPoint activeScrolledRootPosition; + FrameLayerBuilder::GetThebesLayerResolutionForFrame(mScrolledFrame, &xres, &yres, + &activeScrolledRootPosition); + nsPoint pt = + ClampAndRestrictToLayerPixels(aPt, + GetScrollRangeForClamping(), + appUnitsPerDevPixel, + aRange, xres, yres, + activeScrolledRootPosition); + + nsPoint curPos = GetScrollPosition(); if (pt == curPos) { return; }